search

kubereboot / kured

Kubernetes Reboot Daemon
https://kured.dev
Apache License 2.0
2.12k stars 201 forks source link

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

Open haraldkoch opened 7 months ago

haraldkoch commented 7 months ago

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.

github-actions[bot] commented 5 months ago

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 commented 5 months ago

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.

github-actions[bot] commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@haraldkoch @kingdonb is this as simple as this? https://github.com/kubereboot/kured/pull/919

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

haraldkoch commented 3 months ago

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.

github-actions[bot] commented 1 month ago

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).

haraldkoch commented 1 month ago

It would be lovely to merge the linked PR instead of letting this issue decay again?