Repository navigation
Shutdown manager if webhook & metrics server cert-watchers fail #3413
Description
Just recently noticed that the webhook & metrics server are starting certWatchers like this
go func() {
if err := certWatcher.Start(ctx); err != nil {
log.Error(err, "certificate watcher error")
}
}()
If the cert-watcher is failing we only log this error and that's it. As soon as the certs are rotated again the servers will use outdated certificates.
If this is somehow covered by a livenessProbe the manager will restart otherwise it won't.
I think we should look into how we can shutdown the manager / restart the binary if one of these cert-watchers fail.
One option might be to ensure the Start func of the servers return an error when the cert-watcher fails.
Activity
sbueringer commentedon Jan 8, 2026
cc @alvaroaleman @fabriziopandini
alvaroaleman commentedon Jan 8, 2026
yeah this seems bad, maybe we can find a way to add the certwatchers to the manager rather than having to deal with it failing in the webhook/metrics server?
sbueringer commentedon Jan 9, 2026
I was thinking that maybe handling it internally is cleaner than somehow trying to connect the certwatcher runnables to the manager. But maybe there's a clean way to do that.
In general it would be nice if the certwatcher stays an implementation detail of these servers (and as these servers are already runnables themselves I thought it's nice if they just keep an eye on their certwatchers)
But no strong opinion
k8s-triage-robot commentedon Apr 9, 2026
The Kubernetes project currently lacks enough contributors to adequately respond to all issues.
This bot triages un-triaged issues according to the following rules:
lifecycle/staleis appliedlifecycle/stalewas applied,lifecycle/rottenis appliedlifecycle/rottenwas applied, the issue is closedYou can:
/remove-lifecycle stale/closePlease send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
sbueringer commentedon Apr 29, 2026
/remove-lifecycle stale
GaurangKhanderay commentedon Jul 24, 2026
I'd like to work on this.
After looking through the code, it seems like both the webhook server and metrics server already implement
Runnable, and theirStart()methods can propagate errors back to the manager.The missing piece appears to be the
certWatcher.Start(ctx)goroutine. Currently, if the watcher fails, the error is only logged:go func() {
if err := certWatcher.Start(ctx); err != nil {
log.Error(err, "certificate watcher error")
}
}()
This means the failure does not reach the existing runnable error handling path.
I'm thinking of keeping the cert watcher as an internal implementation detail of the webhook/metrics servers and propagating its failure through the existing Start() error contract, instead of making cert watchers separate manager runnables. This seems aligned with the discussion above and avoids changes to manager runnable orchestration.
One design question before I start: should a cert-watcher failure always be fatal (triggering manager shutdown), or is there appetite for retry/recovery within the server first?
sbueringer commentedon Jul 31, 2026
Please stop posting AI generated comments, thx!
somtochiama commentedon Oct 13, 2026
@sbueringer I'd like to pick this up if that's okay. I am leaning towardds keeping the cert-watchers in the metrics/webhook servers and having the start function fail if they don't start.
sbueringer commentedon Oct 19, 2026
I took another look. I still think #3413 (comment) is the way to go compared to adding the metrics/webhook servers cert-watchers to the manager
@alvaroaleman is that okay for you as well?
alvaroaleman commentedon Oct 19, 2026
Yeah for as long as we ensure that the corresponding runnables fail which should then typically cause the manager to fail, that seems fine
sbueringer commentedon Oct 19, 2026
@somtochiama Feel free to go ahead