Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: adapt to the new state of insights-client.service on failure #80

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

ptoscano
Copy link
Collaborator

There were recent changes to the insights-client.service systemd service of insights-client:
RedHatInsights/insights-client#240
This in practice means that insights-client.service now automatically restarts itself in case of failure. Because of that, that the status of the service on failure now is not "failed" but "activating", since it will enabled again the next time insights-client.timer is triggered; the effect in the cockpit plugin is that the detection for a failed upload now does not work.

To adapt to the new situation, tweak the detection a bit: in addition to the old logic (left there to support both old and new versions of insights-client), a new logic is added to check that the systemd service is "starting" (the cockpit mapping to the "activating" status) and its last result is different than "success" (hence it failed somehow).

Also adapt the test a bit: the service needs explicit restarting now, rather than a "simple" start.

Card ID: CCT-702

There were recent changes to the insights-client.service systemd service
of insights-client:
RedHatInsights/insights-client#240
This in practice means that insights-client.service now automatically
restarts itself in case of failure. Because of that, that the status of
the service on failure now is not "failed" but "activating", since it
will enabled again the next time insights-client.timer is triggered;
the effect in the cockpit plugin is that the detection for a failed
upload now does not work.

To adapt to the new situation, tweak the detection a bit: in addition to
the old logic (left there to support both old and new versions of
insights-client), a new logic is added to check that the systemd service
is "starting" (the cockpit mapping to the "activating" status) and its
last result is different than "success" (hence it failed somehow).

Also adapt the test a bit: the service needs explicit restarting now,
rather than a "simple" start.

Card ID: CCT-702
@martinpitt
Copy link
Contributor

This completely makes sense to me, thanks! I triggered a run against the new image in cockpit-project/bots#6775 as well.

Copy link
Contributor

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works!

Copy link
Contributor

@m-horky m-horky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@m-horky m-horky merged commit 77151a2 into main Aug 29, 2024
9 checks passed
@m-horky m-horky deleted the ptoscano/new-insights-client-service branch August 29, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants