kubereboot / kured

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

update reboot required metric in main rebootRequired func #919

Closed jackfrancis closed 2 months ago

jackfrancis commented 6 months ago

This PR moves the prometheus metric posting mechanism inside the rebootRequired func itself rather than doing a separate, parallel check every minute.

The downside to this is that the periodicity of the metric (and thus the resolution) will be reduced (assuming you're running a periodic config of more than once per minute.

evrardjp commented 4 months ago

You're touching something I have done on my fork long ago... But always hesitated to bring here.

The plus side is the fact it's easier to debug: The metrics represent the "kured view" as a whole, not only a goroutine view.

The negative side is a change of behaviour, which for me is very hard on ppl, because now it really behaves differently.

Next to that the method rebootRequired was not really a pure function, but it could be made more pure by extracting the logging. Now it cannot be made cleaner.

I think the whole method rebootRequired could do with a refactor, but I feel this is a step not heading to the right direction without a larger refactor.

What was the reason you wanted to clean the goroutine @jackfrancis ? Is there something that annoyed you in the current code?

github-actions[bot] commented 2 months ago

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