From 4e548fa8011587ccf1e6f6b98f9b3747da98d285 Mon Sep 17 00:00:00 2001 From: Vince Prignano Date: Wed, 5 May 2021 10:15:50 -0700 Subject: [PATCH] :bug: Webhook servers should always start before cache syncs Signed-off-by: Vince Prignano --- pkg/manager/internal.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index f8c5619110..c16a5bb5f3 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -67,6 +67,7 @@ type controllerManager struct { // leaderElectionRunnables is the set of Controllers that the controllerManager injects deps into and Starts. // These Runnables are managed by lead election. leaderElectionRunnables []Runnable + // nonLeaderElectionRunnables is the set of webhook servers that the controllerManager injects deps into and Starts. // These Runnables will not be blocked by lead election. nonLeaderElectionRunnables []Runnable @@ -577,10 +578,27 @@ func (cm *controllerManager) startNonLeaderElectionRunnables() { cm.mu.Lock() defer cm.mu.Unlock() + // First start any webhook servers, which includes conversion, validation, and defaulting + // webhooks that are registered. + // + // WARNING: Webhooks MUST start before any cache is populated, otherwise there is a race condition + // between conversion webhooks and the cache sync (usually initial list) which causes the webhooks + // to never start because no cache can be populated. + for _, c := range cm.nonLeaderElectionRunnables { + if _, ok := c.(*webhook.Server); ok { + cm.startRunnable(c) + } + } + + // Start and wait for caches. cm.waitForCache(cm.internalCtx) // Start the non-leaderelection Runnables after the cache has synced for _, c := range cm.nonLeaderElectionRunnables { + if _, ok := c.(*webhook.Server); ok { + continue + } + // Controllers block, but we want to return an error if any have an error starting. // Write any Start errors to a channel so we can return them cm.startRunnable(c)