From c5467c08c07a93d2ff2b09c677a4152c14f19d1d Mon Sep 17 00:00:00 2001 From: Reed Schalo Date: Tue, 23 Jul 2024 10:53:21 -0700 Subject: [PATCH] fix: put webhook disable option back (#1439) --- kwok/charts/README.md | 1 + kwok/charts/templates/deployment.yaml | 10 ++++++ kwok/charts/templates/role.yaml | 4 +++ .../charts/templates/secret-webhook-cert.yaml | 4 ++- kwok/charts/templates/service.yaml | 2 ++ kwok/charts/values.yaml | 2 ++ pkg/operator/operator.go | 34 +++++++++++-------- pkg/operator/options/options.go | 2 ++ pkg/operator/options/suite_test.go | 18 +++++++++- pkg/test/options.go | 1 + 10 files changed, 62 insertions(+), 16 deletions(-) diff --git a/kwok/charts/README.md b/kwok/charts/README.md index 71378f03a9..ae0ba786c7 100644 --- a/kwok/charts/README.md +++ b/kwok/charts/README.md @@ -60,5 +60,6 @@ For full Karpenter documentation please checkout [https://karpenter.sh](https:// | terminationGracePeriodSeconds | string | `nil` | Override the default termination grace period for the pod. | | tolerations | list | `[{"key":"CriticalAddonsOnly","operator":"Exists"}]` | Tolerations to allow the pod to be scheduled to nodes with taints. | | topologySpreadConstraints | list | `[{"maxSkew":1,"topologyKey":"topology.kubernetes.io/zone","whenUnsatisfiable":"ScheduleAnyway"}]` | Topology spread constraints to increase the controller resilience by distributing pods across the cluster zones. If an explicit label selector is not provided one will be created from the pod selector labels. | +| webhook.enabled | bool | `true` | Whether to enable the webhooks and webhook permissions. | | webhook.port | int | `8443` | The container port to use for the webhook. | diff --git a/kwok/charts/templates/deployment.yaml b/kwok/charts/templates/deployment.yaml index 5af4b414fd..fa8bb5132f 100644 --- a/kwok/charts/templates/deployment.yaml +++ b/kwok/charts/templates/deployment.yaml @@ -76,8 +76,16 @@ spec: value: "1.19.0-0" - name: KARPENTER_SERVICE value: {{ include "karpenter.fullname" . }} + {{- if .Values.webhook.enabled }} - name: WEBHOOK_PORT value: "{{ .Values.webhook.port }}" + - name: DISABLE_WEBHOOK + value: "false" + {{- end }} + {{- if not .Values.webhook.enabled }} + - name: DISABLE_WEBHOOK + value: "true" + {{- end }} {{- with .Values.logLevel }} - name: LOG_LEVEL value: "{{ . }}" @@ -120,9 +128,11 @@ spec: - name: http containerPort: {{ .Values.controller.healthProbe.port }} protocol: TCP + {{- if .Values.webhook.enabled }} - name: https-webhook containerPort: {{ .Values.webhook.port }} protocol: TCP + {{- end }} livenessProbe: initialDelaySeconds: 30 timeoutSeconds: 30 diff --git a/kwok/charts/templates/role.yaml b/kwok/charts/templates/role.yaml index 048c443c78..b80a34a11f 100644 --- a/kwok/charts/templates/role.yaml +++ b/kwok/charts/templates/role.yaml @@ -14,15 +14,19 @@ rules: - apiGroups: ["coordination.k8s.io"] resources: ["leases"] verbs: ["get", "watch"] +{{- if .Values.webhook.enabled }} - apiGroups: [""] resources: ["configmaps", "secrets"] verbs: ["get", "list", "watch"] +{{- end}} # Write +{{- if .Values.webhook.enabled }} - apiGroups: [""] resources: ["secrets"] verbs: ["update"] resourceNames: - "{{ include "karpenter.fullname" . }}-cert" +{{- end }} - apiGroups: ["coordination.k8s.io"] resources: ["leases"] verbs: ["patch", "update"] diff --git a/kwok/charts/templates/secret-webhook-cert.yaml b/kwok/charts/templates/secret-webhook-cert.yaml index 3eb10042cc..ea80c1f8a4 100644 --- a/kwok/charts/templates/secret-webhook-cert.yaml +++ b/kwok/charts/templates/secret-webhook-cert.yaml @@ -1,3 +1,4 @@ +{{- if .Values.webhook.enabled }} apiVersion: v1 kind: Secret metadata: @@ -9,4 +10,5 @@ metadata: annotations: {{- toYaml . | nindent 4 }} {{- end }} -# data: {} # Injected by karpenter-webhook \ No newline at end of file +# data: {} # Injected by karpenter-webhook +{{- end }} \ No newline at end of file diff --git a/kwok/charts/templates/service.yaml b/kwok/charts/templates/service.yaml index 5236489ae6..e7e4441fb1 100644 --- a/kwok/charts/templates/service.yaml +++ b/kwok/charts/templates/service.yaml @@ -16,9 +16,11 @@ spec: port: {{ .Values.controller.metrics.port }} targetPort: http-metrics protocol: TCP + {{- if .Values.webhook.enabled }} - name: https-webhook port: {{ .Values.webhook.port }} targetPort: https-webhook protocol: TCP + {{- end }} selector: {{- include "karpenter.selectorLabels" . | nindent 4 }} diff --git a/kwok/charts/values.yaml b/kwok/charts/values.yaml index 4ad57b9a94..0a7c2d1fed 100644 --- a/kwok/charts/values.yaml +++ b/kwok/charts/values.yaml @@ -123,6 +123,8 @@ controller: # -- The container port to use for http health probe. port: 8081 webhook: + # -- Whether to enable the webhooks and webhook permissions. + enabled: true # -- The container port to use for the webhook. port: 8443 # -- Global log level, defaults to 'info' diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 802aaebdaa..af75cc1a73 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -241,9 +241,11 @@ func (o *Operator) WithControllers(ctx context.Context, controllers ...controlle } func (o *Operator) WithWebhooks(ctx context.Context, ctors ...knativeinjection.ControllerConstructor) *Operator { - o.webhooks = append(o.webhooks, ctors...) - lo.Must0(o.Manager.AddReadyzCheck("webhooks", webhooks.HealthProbe(ctx))) - lo.Must0(o.Manager.AddHealthzCheck("webhooks", webhooks.HealthProbe(ctx))) + if !options.FromContext(ctx).DisableWebhook { + o.webhooks = append(o.webhooks, ctors...) + lo.Must0(o.Manager.AddReadyzCheck("webhooks", webhooks.HealthProbe(ctx))) + lo.Must0(o.Manager.AddHealthzCheck("webhooks", webhooks.HealthProbe(ctx))) + } return o } @@ -254,16 +256,20 @@ func (o *Operator) Start(ctx context.Context, cp cloudprovider.CloudProvider) { defer wg.Done() lo.Must0(o.Manager.Start(ctx)) }() - wg.Add(1) - go func() { - defer wg.Done() - // Taking the first supported NodeClass to be the default NodeClass - gvk := lo.Map(cp.GetSupportedNodeClasses(), func(nc status.Object, _ int) schema.GroupVersionKind { - return object.GVK(nc) - }) - ctx = injection.WithNodeClasses(ctx, gvk) - ctx = injection.WithClient(ctx, o.GetClient()) - webhooks.Start(ctx, o.GetConfig(), o.webhooks...) - }() + if options.FromContext(ctx).DisableWebhook { + log.FromContext(ctx).Info("conversion webhooks are disabled") + } else { + wg.Add(1) + go func() { + defer wg.Done() + // Taking the first supported NodeClass to be the default NodeClass + gvk := lo.Map(cp.GetSupportedNodeClasses(), func(nc status.Object, _ int) schema.GroupVersionKind { + return object.GVK(nc) + }) + ctx = injection.WithNodeClasses(ctx, gvk) + ctx = injection.WithClient(ctx, o.GetClient()) + webhooks.Start(ctx, o.GetConfig(), o.webhooks...) + }() + } wg.Wait() } diff --git a/pkg/operator/options/options.go b/pkg/operator/options/options.go index 6168a56cfc..cb98afe1a7 100644 --- a/pkg/operator/options/options.go +++ b/pkg/operator/options/options.go @@ -47,6 +47,7 @@ type FeatureGates struct { // Options contains all CLI flags / env vars for karpenter-core. It adheres to the options.Injectable interface. type Options struct { ServiceName string + DisableWebhook bool WebhookPort int MetricsPort int WebhookMetricsPort int @@ -81,6 +82,7 @@ func (fs *FlagSet) BoolVarWithEnv(p *bool, name string, envVar string, val bool, func (o *Options) AddFlags(fs *FlagSet) { fs.StringVar(&o.ServiceName, "karpenter-service", env.WithDefaultString("KARPENTER_SERVICE", ""), "The Karpenter Service name for the dynamic webhook certificate") + fs.BoolVarWithEnv(&o.DisableWebhook, "disable-webhook", "DISABLE_WEBHOOK", false, "Disable the conversion webhooks") fs.IntVar(&o.WebhookPort, "webhook-port", env.WithDefaultInt("WEBHOOK_PORT", 8443), "The port the webhook endpoint binds to for validation and mutation of resources") fs.IntVar(&o.MetricsPort, "metrics-port", env.WithDefaultInt("METRICS_PORT", 8000), "The port the metric endpoint binds to for operating metrics about the controller itself") fs.IntVar(&o.WebhookMetricsPort, "webhook-metrics-port", env.WithDefaultInt("WEBHOOK_METRICS_PORT", 8001), "The port the webhook metric endpoing binds to for operating metrics about the webhook") diff --git a/pkg/operator/options/suite_test.go b/pkg/operator/options/suite_test.go index ef461ff067..5c8fb261a0 100644 --- a/pkg/operator/options/suite_test.go +++ b/pkg/operator/options/suite_test.go @@ -97,7 +97,7 @@ var _ = Describe("Options", func() { Expect(err).To(BeNil()) expectOptionsMatch(opts, test.Options(test.OptionsFields{ ServiceName: lo.ToPtr(""), - DisableWebhook: lo.ToPtr(true), + DisableWebhook: lo.ToPtr(false), WebhookPort: lo.ToPtr(8443), MetricsPort: lo.ToPtr(8000), WebhookMetricsPort: lo.ToPtr(8001), @@ -120,6 +120,7 @@ var _ = Describe("Options", func() { err := opts.Parse( fs, "--karpenter-service", "cli", + "--disable-webhook", "--webhook-port", "0", "--metrics-port", "0", "--webhook-metrics-port", "0", @@ -220,6 +221,7 @@ var _ = Describe("Options", func() { err := opts.Parse( fs, "--karpenter-service", "cli", + "--disable-webhook", ) Expect(err).To(BeNil()) expectOptionsMatch(opts, test.Options(test.OptionsFields{ @@ -244,6 +246,19 @@ var _ = Describe("Options", func() { }) }) + DescribeTable( + "should correctly parse boolean values", + func(arg string, expected bool) { + err := opts.Parse(fs, arg) + Expect(err).ToNot(HaveOccurred()) + Expect(opts.DisableWebhook).To(Equal(expected)) + }, + Entry("explicit true", "--disable-webhook=true", true), + Entry("explicit false", "--disable-webhook=false", false), + Entry("implicit true", "--disable-webhook", true), + Entry("implicit false", "", false), + ) + Context("Validation", func() { DescribeTable( "should parse valid log levels successfully", @@ -271,6 +286,7 @@ func expectOptionsMatch(optsA, optsB *options.Options) { Expect(optsA).ToNot(BeNil()) Expect(optsB).ToNot(BeNil()) Expect(optsA.ServiceName).To(Equal(optsB.ServiceName)) + Expect(optsA.DisableWebhook).To(Equal(optsB.DisableWebhook)) Expect(optsA.WebhookPort).To(Equal(optsB.WebhookPort)) Expect(optsA.MetricsPort).To(Equal(optsB.MetricsPort)) Expect(optsA.WebhookMetricsPort).To(Equal(optsB.WebhookMetricsPort)) diff --git a/pkg/test/options.go b/pkg/test/options.go index 4aeb672df5..3e3de9c041 100644 --- a/pkg/test/options.go +++ b/pkg/test/options.go @@ -59,6 +59,7 @@ func Options(overrides ...OptionsFields) *options.Options { return &options.Options{ ServiceName: lo.FromPtrOr(opts.ServiceName, ""), + DisableWebhook: lo.FromPtrOr(opts.DisableWebhook, false), WebhookPort: lo.FromPtrOr(opts.WebhookPort, 8443), MetricsPort: lo.FromPtrOr(opts.MetricsPort, 8000), WebhookMetricsPort: lo.FromPtrOr(opts.WebhookMetricsPort, 8001),