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(blackbox_exporter): make capability setting idempotent #438

Merged

Conversation

sverrehu
Copy link
Contributor

@sverrehu sverrehu commented Oct 23, 2024

Recently, the "Ensure blackbox exporter binary has cap_net_raw capability" task has reported a change for every pipeline run. This PR contains two changes to fix this behavior.

  • An old bug in community.general.capabilities, which is caused by naive comparison between requested state and observed state, will indicate a change if the operation is anything besides =. This PR changes from cap_net_raw+ep to cap_net_raw=ep to fix this.

  • A recent PR set changed_when: "'molecule-idempotence-notest' not in ansible_skip_tags", which translates to changed_when: true, causing this task to appear as changed no matter what. This PR removes this line.

With these changes, our pipeline reports no change if the cap_net_raw is as wanted, and reports change otherwise.

@erikgb
Copy link

erikgb commented Oct 23, 2024

@gardar Could this be the culprit task causing Prometheus always to restart, ref. #431 (comment)?

@SuperQ
Copy link
Contributor

SuperQ commented Oct 23, 2024

This wouldn't case Prometheus to restart, only the blackbox_exporter.

@SuperQ
Copy link
Contributor

SuperQ commented Oct 23, 2024

This needs a DCO sign-off. You can use git commit -s --amend to add it.

@sverrehu sverrehu force-pushed the blackbox-capability-idempotence branch from d4950a9 to a9bc6d7 Compare October 23, 2024 10:21
@github-actions github-actions bot added bugfix and removed bugfix labels Oct 23, 2024
@github-actions github-actions bot added bugfix and removed bugfix labels Oct 23, 2024
@sverrehu sverrehu force-pushed the blackbox-capability-idempotence branch from 171c1a5 to da3b577 Compare October 23, 2024 11:19
@github-actions github-actions bot added bugfix and removed bugfix labels Oct 23, 2024
@sverrehu
Copy link
Contributor Author

sverrehu commented Oct 23, 2024

I noticed some idempotence checks failed. Before this PR, those tests would in effect have changed_when: false, as molecule-idempotence-notest was part of ansible_skip_tags in Molecule's idempotence checks.

I just added molecule-idempotence-notest as part of the tags, in order to skip this task completely during Molecule's idempotence checks. Hopefully, the effect should be the same as before, but without having any effect when running outside of Molecule.

PS: The reason it fails on Ubuntu 20.04 is that getcap on that version returns cap_net_raw+ep no matter how we set it. On other platforms, getcap returns cap_net_raw=ep.

@sverrehu sverrehu requested a review from gardar October 23, 2024 12:07
Copy link
Member

@gardar gardar left a comment

Choose a reason for hiding this comment

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

LGTM!

@gardar
Copy link
Member

gardar commented Oct 23, 2024

The failing CI tests will be fixed with #440

@gardar gardar changed the title fix(blackbox): make capability setting idempotent fix(blackbox_exporter): make capability setting idempotent Oct 23, 2024
@github-actions github-actions bot added bugfix and removed bugfix labels Oct 23, 2024
@github-actions github-actions bot added bugfix and removed bugfix labels Oct 23, 2024
@sverrehu sverrehu requested a review from gardar October 23, 2024 16:42
@gardar gardar merged commit c81f7d9 into prometheus-community:main Oct 23, 2024
41 checks passed
@sverrehu sverrehu deleted the blackbox-capability-idempotence branch October 23, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants