diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index 57f95ba5b3..2dc83c40b9 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -145,6 +145,9 @@ type controllerManager struct { certDir string webhookServer *webhook.Server + // webhookServerOnce will be called in GetWebhookServer() to optionally initialize + // webhookServer if unset, and Add() it to controllerManager. + webhookServerOnce sync.Once // leaseDuration is the duration that non-leader candidates will // wait to force acquire leadership. @@ -332,32 +335,19 @@ func (cm *controllerManager) GetAPIReader() client.Reader { } func (cm *controllerManager) GetWebhookServer() *webhook.Server { - server, wasNew := func() (*webhook.Server, bool) { - cm.mu.Lock() - defer cm.mu.Unlock() - - if cm.webhookServer != nil { - return cm.webhookServer, false - } - - cm.webhookServer = &webhook.Server{ - Port: cm.port, - Host: cm.host, - CertDir: cm.certDir, + cm.webhookServerOnce.Do(func() { + if cm.webhookServer == nil { + cm.webhookServer = &webhook.Server{ + Port: cm.port, + Host: cm.host, + CertDir: cm.certDir, + } } - return cm.webhookServer, true - }() - - // only add the server if *we ourselves* just registered it. - // Add has its own lock, so just do this separately -- there shouldn't - // be a "race" in this lock gap because the condition is the population - // of cm.webhookServer, not anything to do with Add. - if wasNew { - if err := cm.Add(server); err != nil { + if err := cm.Add(cm.webhookServer); err != nil { panic("unable to add webhook server to the controller manager") } - } - return server + }) + return cm.webhookServer } func (cm *controllerManager) GetLogger() logr.Logger { diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 30ff9e2516..21750a3185 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -209,17 +209,24 @@ type Options struct { LivenessEndpointName string // Port is the port that the webhook server serves at. - // It is used to set webhook.Server.Port. + // It is used to set webhook.Server.Port if WebhookServer is not set. Port int // Host is the hostname that the webhook server binds to. - // It is used to set webhook.Server.Host. + // It is used to set webhook.Server.Host if WebhookServer is not set. Host string // CertDir is the directory that contains the server key and certificate. - // if not set, webhook server would look up the server key and certificate in + // If not set, webhook server would look up the server key and certificate in // {TempDir}/k8s-webhook-server/serving-certs. The server key and certificate // must be named tls.key and tls.crt, respectively. + // It is used to set webhook.Server.CertDir if WebhookServer is not set. CertDir string + + // WebhookServer is an externally configured webhook.Server. By default, + // a Manager will create a default server using Port, Host, and CertDir; + // if this is set, the Manager will use this server instead. + WebhookServer *webhook.Server + // Functions to all for a user to customize the values that will be injected. // NewCache is the function that will create the cache to be used @@ -370,6 +377,7 @@ func New(config *rest.Config, options Options) (Manager, error) { port: options.Port, host: options.Host, certDir: options.CertDir, + webhookServer: options.WebhookServer, leaseDuration: *options.LeaseDuration, renewDeadline: *options.RenewDeadline, retryPeriod: *options.RetryPeriod, diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index ce1281d224..59255aa54e 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -53,6 +53,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/recorder" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" + "sigs.k8s.io/controller-runtime/pkg/webhook" ) var _ = Describe("manger.Manager", func() { @@ -273,6 +274,20 @@ var _ = Describe("manger.Manager", func() { close(done) }) + It("should not initialize a webhook server if Options.WebhookServer is set", func(done Done) { + By("creating a manager with options") + m, err := New(cfg, Options{Port: 9441, WebhookServer: &webhook.Server{Port: 9440}}) + Expect(err).NotTo(HaveOccurred()) + Expect(m).NotTo(BeNil()) + + By("checking the server contains the Port set on the webhook server and not passed to Options") + svr := m.GetWebhookServer() + Expect(svr).NotTo(BeNil()) + Expect(svr.Port).To(Equal(9440)) + + close(done) + }) + Context("with leader election enabled", func() { It("should only cancel the leader election after all runnables are done", func() { m, err := New(cfg, Options{