-
Notifications
You must be signed in to change notification settings - Fork 200
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
Comments
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). |
I can confirm this behaviour for version 1.15.0. Although I set period to 30min the sentinel command runs every 1min. |
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). |
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) |
@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. |
Huzzah!
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. |
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). |
The configured sentinel command runs every minute, instead of once every
period
. The reason for this is appears to be the Prometheus metrics updatermaintainRebootRequiredMetric()
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-codedtime.Minute
that is there now.The text was updated successfully, but these errors were encountered: