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

If a sentinel command is configured, it runs every minute. #853

Open
haraldkoch opened this issue Nov 22, 2023 · 7 comments
Open

If a sentinel command is configured, it runs every minute. #853

haraldkoch opened this issue Nov 22, 2023 · 7 comments

Comments

@haraldkoch
Copy link

The configured sentinel command runs every minute, instead of once every period. The reason for this is appears to be the Prometheus metrics updater maintainRebootRequiredMetric() here:

https://github.com/kubereboot/kured/blob/9e4b69f8189cbf685e459ad1be2eb15f4dfd0cd2/cmd/kured/main.go#L566C1-L575C2

This is arguably incorrect. The metric should be updated only when the sentinel command is run in the main code (i.e. once every period interval).

A compromise would be to have the function maintainRebootRequiredMetric() sleep for period instead of the hard-coded time.Minute that is there now.

Copy link

This issue was automatically considered stale due to lack of activity. Please update it and/or join our slack channels to promote it, before it automatically closes (in 7 days).

@miheinr
Copy link

miheinr commented Jan 30, 2024

The configured sentinel command runs every minute, instead of once every period.

I can confirm this behaviour for version 1.15.0. Although I set period to 30min the sentinel command runs every 1min.

Copy link

This issue was automatically considered stale due to lack of activity. Please update it and/or join our slack channels to promote it, before it automatically closes (in 7 days).

@kingdonb
Copy link
Contributor

kingdonb commented Apr 1, 2024

Since no one has turned up to defend the code's position, I have to infer that we'd be open to some kind of PR to fix this.

(Un-labeled as inactive)

@jackfrancis
Copy link
Collaborator

@haraldkoch @kingdonb is this as simple as this? #919

Note the potential downside for folks who want to have a separate periodicity for metrics data.

@haraldkoch
Copy link
Author

Huzzah!

Note the potential downside for folks who want to have a separate periodicity for metrics data.

The Prometheus library caches the metric values passed to metric.Set(), and returns the cached value to anyone who queries the metrics endpoint, so this shouldn't affect any metrics data.

Copy link

github-actions bot commented Jun 2, 2024

This issue was automatically considered stale due to lack of activity. Please update it and/or join our slack channels to promote it, before it automatically closes (in 7 days).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants