From be20b0d714ff7591b86e8cb6abcabc42cdd64bcb Mon Sep 17 00:00:00 2001 From: Amanuel Engeda <74629455+engedaam@users.noreply.github.com> Date: Thu, 22 Aug 2024 00:08:12 -0700 Subject: [PATCH] (backport) chore: Remove Post Install Hook (#6827) for v0.36.x (#6835) Co-authored-by: Jonathan Innis --- .../karpenter.k8s.aws_ec2nodeclasses.yaml | 2 +- .../templates/karpenter.sh_nodeclaims.yaml | 2 +- .../templates/karpenter.sh_nodepools.yaml | 2 +- charts/karpenter-crd/values.yaml | 3 +- charts/karpenter/templates/_helpers.tpl | 11 ----- .../karpenter/templates/clusterrole-core.yaml | 17 ++------ .../templates/post-install-hook.yaml | 40 ------------------- charts/karpenter/values.yaml | 8 ---- hack/docgen.sh | 10 ++--- .../compatibility.yaml} | 0 .../main.go} | 0 .../main.go} | 0 .../main.go} | 0 .../main.go} | 0 .../main.go} | 0 hack/mutation/conversion_webhook_injection.sh | 6 +-- hack/toolchain.sh | 2 +- pkg/apis/v1/ec2nodeclass_conversion.go | 3 +- pkg/apis/v1beta1/ec2nodeclass_conversion.go | 2 + pkg/operator/operator.go | 2 +- pkg/providers/amifamily/ami.go | 2 +- pkg/providers/amifamily/resolver.go | 3 ++ pkg/providers/instance/instance.go | 10 ++--- pkg/test/environment.go | 4 +- 24 files changed, 33 insertions(+), 96 deletions(-) delete mode 100644 charts/karpenter/templates/post-install-hook.yaml rename hack/docs/{compatibility-karpenter.yaml => compatibilitymatrix_gen/compatibility.yaml} (100%) rename hack/docs/{compatibilitymetrix_gen_docs.go => compatibilitymatrix_gen/main.go} (100%) rename hack/docs/{configuration_gen_docs.go => configuration_gen/main.go} (100%) rename hack/docs/{instancetypes_gen_docs.go => instancetypes_gen/main.go} (100%) rename hack/docs/{metrics_gen_docs.go => metrics_gen/main.go} (100%) rename hack/docs/{version_compatibility.go => version_compatibility_gen/main.go} (100%) diff --git a/charts/karpenter-crd/templates/karpenter.k8s.aws_ec2nodeclasses.yaml b/charts/karpenter-crd/templates/karpenter.k8s.aws_ec2nodeclasses.yaml index af375ad1e199..cf899af37a47 100644 --- a/charts/karpenter-crd/templates/karpenter.k8s.aws_ec2nodeclasses.yaml +++ b/charts/karpenter-crd/templates/karpenter.k8s.aws_ec2nodeclasses.yaml @@ -1289,7 +1289,7 @@ spec: clientConfig: service: name: {{ .Values.webhook.serviceName }} - namespace: {{ .Values.webhook.serviceNamespace }} + namespace: {{ .Release.Namespace }} port: {{ .Values.webhook.port }} {{- end }} diff --git a/charts/karpenter-crd/templates/karpenter.sh_nodeclaims.yaml b/charts/karpenter-crd/templates/karpenter.sh_nodeclaims.yaml index 460940083d5f..259844e1e055 100644 --- a/charts/karpenter-crd/templates/karpenter.sh_nodeclaims.yaml +++ b/charts/karpenter-crd/templates/karpenter.sh_nodeclaims.yaml @@ -816,7 +816,7 @@ spec: clientConfig: service: name: {{ .Values.webhook.serviceName }} - namespace: {{ .Values.webhook.serviceNamespace }} + namespace: {{ .Release.Namespace }} port: {{ .Values.webhook.port }} {{- end }} diff --git a/charts/karpenter-crd/templates/karpenter.sh_nodepools.yaml b/charts/karpenter-crd/templates/karpenter.sh_nodepools.yaml index 6da790b50688..b064e4f28c17 100644 --- a/charts/karpenter-crd/templates/karpenter.sh_nodepools.yaml +++ b/charts/karpenter-crd/templates/karpenter.sh_nodepools.yaml @@ -1029,7 +1029,7 @@ spec: clientConfig: service: name: {{ .Values.webhook.serviceName }} - namespace: {{ .Values.webhook.serviceNamespace }} + namespace: {{ .Release.Namespace }} port: {{ .Values.webhook.port }} {{- end }} diff --git a/charts/karpenter-crd/values.yaml b/charts/karpenter-crd/values.yaml index fa419ad42f33..bbd4cf1c4ffb 100644 --- a/charts/karpenter-crd/values.yaml +++ b/charts/karpenter-crd/values.yaml @@ -1,7 +1,6 @@ webhook: - # -- Whether to enable the webhooks and webhook permissions. + # -- Whether to enable the webhooks. enabled: false serviceName: karpenter - serviceNamespace: kube-system # -- The container port to use for the webhook. port: 8443 \ No newline at end of file diff --git a/charts/karpenter/templates/_helpers.tpl b/charts/karpenter/templates/_helpers.tpl index a74c4dbb1aea..9dce663e2382 100644 --- a/charts/karpenter/templates/_helpers.tpl +++ b/charts/karpenter/templates/_helpers.tpl @@ -75,17 +75,6 @@ Karpenter image to use {{- end }} {{- end }} -{{/* -Karpenter post-install hook image to use -*/}} -{{- define "karpenter.postInstallHook.image" -}} -{{- if .Values.postInstallHook.image.digest }} -{{- printf "%s:%s@%s" .Values.postInstallHook.image.repository (default (printf "v%s" .Chart.AppVersion) .Values.postInstallHook.image.tag) .Values.postInstallHook.image.digest }} -{{- else }} -{{- printf "%s:%s" .Values.postInstallHook.image.repository (default (printf "v%s" .Chart.AppVersion) .Values.postInstallHook.image.tag) }} -{{- end }} -{{- end }} - {{/* Get PodDisruptionBudget API Version */}} {{- define "karpenter.pdb.apiVersion" -}} diff --git a/charts/karpenter/templates/clusterrole-core.yaml b/charts/karpenter/templates/clusterrole-core.yaml index a650a11e7039..67540c524548 100644 --- a/charts/karpenter/templates/clusterrole-core.yaml +++ b/charts/karpenter/templates/clusterrole-core.yaml @@ -47,13 +47,8 @@ rules: verbs: ["get", "watch", "list"] - apiGroups: ["apiextensions.k8s.io"] resources: ["customresourcedefinitions"] - verbs: ["get", "watch", "list"] -{{- else }} - # Used for the post install hook - - apiGroups: ["apiextensions.k8s.io"] - resources: ["customresourcedefinitions"] - verbs: ["get"] -{{- end }} + verbs: ["watch", "list"] + {{- end }} - apiGroups: ["policy"] resources: ["poddisruptionbudgets"] verbs: ["get", "list", "watch"] @@ -80,12 +75,8 @@ rules: resourceNames: ["validation.webhook.karpenter.sh", "validation.webhook.config.karpenter.sh"] - apiGroups: ["apiextensions.k8s.io"] resources: ["customresourcedefinitions"] - verbs: ["update", "patch"] -{{- else }} - - apiGroups: ["apiextensions.k8s.io"] - resources: ["customresourcedefinitions"] - verbs: ["patch"] -{{- end }} + verbs: ["update"] + {{- end }} {{- with .Values.additionalClusterRoleRules -}} {{ toYaml . | nindent 2 }} {{- end -}} \ No newline at end of file diff --git a/charts/karpenter/templates/post-install-hook.yaml b/charts/karpenter/templates/post-install-hook.yaml deleted file mode 100644 index 5a38c33d8672..000000000000 --- a/charts/karpenter/templates/post-install-hook.yaml +++ /dev/null @@ -1,40 +0,0 @@ -apiVersion: batch/v1 -kind: Job -metadata: - name: {{ .Release.Name }}-post-install-hook - namespace: {{ .Release.Namespace }} - labels: - {{- include "karpenter.labels" . | nindent 4 }} - annotations: - "helm.sh/hook": post-install,post-upgrade,post-rollback - "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed - {{- with .Values.additionalAnnotations }} - {{- toYaml . | nindent 4 }} - {{- end }} -spec: - ttlSecondsAfterFinished: 0 - template: - spec: - serviceAccountName: {{ include "karpenter.serviceAccountName" . }} - restartPolicy: OnFailure - {{- with .Values.tolerations }} - tolerations: - {{- toYaml . | nindent 8 }} - {{- end }} - containers: - - name: post-install-job - image: {{ include "karpenter.postInstallHook.image" . }} - command: - - /bin/sh - - -c - - | - {{- if .Values.webhook.enabled }} - kubectl patch customresourcedefinitions nodepools.karpenter.sh --type='merge' -p '{"spec":{"conversion":{"strategy": "Webhook", "webhook":{"conversionReviewVersions": ["v1beta1", "v1"], "clientConfig":{"service":{"name":"{{ include "karpenter.fullname" . }}", "port": {{ .Values.webhook.port }} ,"namespace": "{{ .Release.Namespace }}"}}}}}}' - kubectl patch customresourcedefinitions nodeclaims.karpenter.sh --type='merge' -p '{"spec":{"conversion":{"strategy": "Webhook", "webhook":{"conversionReviewVersions": ["v1beta1", "v1"], "clientConfig":{"service":{"name":"{{ include "karpenter.fullname" . }}", "port": {{ .Values.webhook.port }} ,"namespace": "{{ .Release.Namespace }}"}}}}}}' - kubectl patch customresourcedefinitions ec2nodeclasses.karpenter.k8s.aws --type='merge' -p '{"spec":{"conversion":{"strategy": "Webhook", "webhook":{"conversionReviewVersions": ["v1beta1", "v1"], "clientConfig":{"service":{"name":"{{ include "karpenter.fullname" . }}", "port": {{ .Values.webhook.port }} ,"namespace": "{{ .Release.Namespace }}"}}}}}}' - {{- else }} - echo "disabled webhooks" - kubectl patch customresourcedefinitions nodepools.karpenter.sh --type='json' -p '[{'op': 'remove', 'path': '/spec/conversion'}]' - kubectl patch customresourcedefinitions nodeclaims.karpenter.sh --type='json' -p '[{'op': 'remove', 'path': '/spec/conversion'}]' - kubectl patch customresourcedefinitions ec2nodeclasses.karpenter.k8s.aws --type='json' -p '[{'op': 'remove', 'path': '/spec/conversion'}]' - {{- end }} \ No newline at end of file diff --git a/charts/karpenter/values.yaml b/charts/karpenter/values.yaml index 4f475daaa728..8dfc528cd085 100644 --- a/charts/karpenter/values.yaml +++ b/charts/karpenter/values.yaml @@ -135,14 +135,6 @@ controller: healthProbe: # -- The container port to use for http health probe. port: 8081 -postInstallHook: - image: - # -- Repository path to the post-install hook. This minimally needs to have `kubectl` installed - repository: public.ecr.aws/bitnami/kubectl - # -- Tag of the post-install hook image. - tag: "1.30" - # -- SHA256 digest of the post-install hook image. - digest: sha256:13a2ad1bd37ce42ee2a6f1ab0d30595f42eb7fe4a90d6ec848550524104a1ed6 webhook: # -- Whether to enable the webhooks and webhook permissions. enabled: false diff --git a/hack/docgen.sh b/hack/docgen.sh index 037114698363..41b3bb92f6d5 100755 --- a/hack/docgen.sh +++ b/hack/docgen.sh @@ -4,13 +4,13 @@ set -euo pipefail compatibilitymatrix() { # versionCount is the number of K8s versions to display in the compatibility matrix versionCount=7 - go run hack/docs/version_compatibility.go hack/docs/compatibility-karpenter.yaml "$(git describe --exact-match --tags || echo "no tag")" - go run hack/docs/compatibilitymetrix_gen_docs.go website/content/en/preview/upgrading/compatibility.md hack/docs/compatibility-karpenter.yaml $versionCount + go run hack/docs/version_compatibility_gen/main.go hack/docs/compatibilitymatrix_gen/compatibility.yaml "$(git describe --exact-match --tags || echo "no tag")" + go run hack/docs/compatibilitymatrix_gen/main.go website/content/en/preview/upgrading/compatibility.md hack/docs/compatibilitymatrix_gen/compatibility.yaml $versionCount } compatibilitymatrix -go run hack/docs/metrics_gen_docs.go pkg/ ${KARPENTER_CORE_DIR}/pkg website/content/en/preview/reference/metrics.md -go run hack/docs/instancetypes_gen_docs.go website/content/en/preview/reference/instance-types.md -go run hack/docs/configuration_gen_docs.go website/content/en/preview/reference/settings.md +go run hack/docs/metrics_gen/main.go pkg/ "${KARPENTER_CORE_DIR}/pkg" website/content/en/preview/reference/metrics.md +go run hack/docs/instancetypes_gen/main.go website/content/en/preview/reference/instance-types.md +go run hack/docs/configuration_gen/main.go website/content/en/preview/reference/settings.md cd charts/karpenter && helm-docs diff --git a/hack/docs/compatibility-karpenter.yaml b/hack/docs/compatibilitymatrix_gen/compatibility.yaml similarity index 100% rename from hack/docs/compatibility-karpenter.yaml rename to hack/docs/compatibilitymatrix_gen/compatibility.yaml diff --git a/hack/docs/compatibilitymetrix_gen_docs.go b/hack/docs/compatibilitymatrix_gen/main.go similarity index 100% rename from hack/docs/compatibilitymetrix_gen_docs.go rename to hack/docs/compatibilitymatrix_gen/main.go diff --git a/hack/docs/configuration_gen_docs.go b/hack/docs/configuration_gen/main.go similarity index 100% rename from hack/docs/configuration_gen_docs.go rename to hack/docs/configuration_gen/main.go diff --git a/hack/docs/instancetypes_gen_docs.go b/hack/docs/instancetypes_gen/main.go similarity index 100% rename from hack/docs/instancetypes_gen_docs.go rename to hack/docs/instancetypes_gen/main.go diff --git a/hack/docs/metrics_gen_docs.go b/hack/docs/metrics_gen/main.go similarity index 100% rename from hack/docs/metrics_gen_docs.go rename to hack/docs/metrics_gen/main.go diff --git a/hack/docs/version_compatibility.go b/hack/docs/version_compatibility_gen/main.go similarity index 100% rename from hack/docs/version_compatibility.go rename to hack/docs/version_compatibility_gen/main.go diff --git a/hack/mutation/conversion_webhook_injection.sh b/hack/mutation/conversion_webhook_injection.sh index 1a466499d041..65a3ab3c8503 100755 --- a/hack/mutation/conversion_webhook_injection.sh +++ b/hack/mutation/conversion_webhook_injection.sh @@ -18,7 +18,7 @@ echo "{{- if .Values.webhook.enabled }} clientConfig: service: name: {{ .Values.webhook.serviceName }} - namespace: {{ .Values.webhook.serviceNamespace }} + namespace: {{ .Release.Namespace }} port: {{ .Values.webhook.port }} {{- end }} " >> charts/karpenter-crd/templates/karpenter.sh_nodepools.yaml @@ -33,7 +33,7 @@ echo "{{- if .Values.webhook.enabled }} clientConfig: service: name: {{ .Values.webhook.serviceName }} - namespace: {{ .Values.webhook.serviceNamespace }} + namespace: {{ .Release.Namespace }} port: {{ .Values.webhook.port }} {{- end }} " >> charts/karpenter-crd/templates/karpenter.sh_nodeclaims.yaml @@ -48,7 +48,7 @@ echo "{{- if .Values.webhook.enabled }} clientConfig: service: name: {{ .Values.webhook.serviceName }} - namespace: {{ .Values.webhook.serviceNamespace }} + namespace: {{ .Release.Namespace }} port: {{ .Values.webhook.port }} {{- end }} " >> charts/karpenter-crd/templates/karpenter.k8s.aws_ec2nodeclasses.yaml \ No newline at end of file diff --git a/hack/toolchain.sh b/hack/toolchain.sh index 9bf6dbc47c96..c871e0dd724e 100755 --- a/hack/toolchain.sh +++ b/hack/toolchain.sh @@ -16,7 +16,7 @@ tools() { go install github.com/mikefarah/yq/v4@latest go install github.com/norwoodj/helm-docs/cmd/helm-docs@latest go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest - go install sigs.k8s.io/controller-tools/cmd/controller-gen@latest + go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.15.0 go install github.com/sigstore/cosign/v2/cmd/cosign@latest go install -tags extended github.com/gohugoio/hugo@v0.110.0 go install golang.org/x/vuln/cmd/govulncheck@latest diff --git a/pkg/apis/v1/ec2nodeclass_conversion.go b/pkg/apis/v1/ec2nodeclass_conversion.go index 6adc3325c3fd..555e294c3f89 100644 --- a/pkg/apis/v1/ec2nodeclass_conversion.go +++ b/pkg/apis/v1/ec2nodeclass_conversion.go @@ -26,8 +26,9 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" - "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" ) func (in *EC2NodeClass) ConvertTo(ctx context.Context, to apis.Convertible) error { diff --git a/pkg/apis/v1beta1/ec2nodeclass_conversion.go b/pkg/apis/v1beta1/ec2nodeclass_conversion.go index 0f9b330138dc..b10d91cace27 100644 --- a/pkg/apis/v1beta1/ec2nodeclass_conversion.go +++ b/pkg/apis/v1beta1/ec2nodeclass_conversion.go @@ -2,7 +2,9 @@ Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 0b010a64c20c..6969f4607b74 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -144,7 +144,7 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont *sess.Config.Region, ) versionProvider := version.NewDefaultProvider(operator.KubernetesInterface, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) - ssmProvider := ssmp.NewDefaultProvider( ssm.New(sess), cache.New(awscache.SSMProviderTTL, awscache.DefaultCleanupInterval)) + ssmProvider := ssmp.NewDefaultProvider(ssm.New(sess), cache.New(awscache.SSMProviderTTL, awscache.DefaultCleanupInterval)) amiProvider := amifamily.NewDefaultProvider(versionProvider, ssmProvider, ec2api, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) amiResolver := amifamily.NewResolver(amiProvider) launchTemplateProvider := launchtemplate.NewDefaultProvider( diff --git a/pkg/providers/amifamily/ami.go b/pkg/providers/amifamily/ami.go index 813f8e0c7620..8609513d6163 100644 --- a/pkg/providers/amifamily/ami.go +++ b/pkg/providers/amifamily/ami.go @@ -106,7 +106,7 @@ func NewDefaultProvider(versionProvider version.Provider, ssmProvider ssm.Provid cache: cache, ec2api: ec2api, cm: pretty.NewChangeMonitor(), - ssmProvider: ssmProvider, + ssmProvider: ssmProvider, versionProvider: versionProvider, } } diff --git a/pkg/providers/amifamily/resolver.go b/pkg/providers/amifamily/resolver.go index 65adec3a9abb..0975ac618f7e 100644 --- a/pkg/providers/amifamily/resolver.go +++ b/pkg/providers/amifamily/resolver.go @@ -219,6 +219,9 @@ func (r Resolver) resolveLaunchTemplate(nodeClass *v1beta1.EC2NodeClass, nodeCla } } if kubeletConfig.MaxPods == nil { + // nolint:gosec + // We know that it's not possible to have values that would overflow int32 here since we control + // the maxPods values that we pass in here kubeletConfig.MaxPods = lo.ToPtr(int32(maxPods)) } resolved := &LaunchTemplate{ diff --git a/pkg/providers/instance/instance.go b/pkg/providers/instance/instance.go index 50abbaf45826..bff04c727a96 100644 --- a/pkg/providers/instance/instance.go +++ b/pkg/providers/instance/instance.go @@ -487,17 +487,17 @@ func instancesFromOutput(out *ec2.DescribeInstancesOutput) ([]*Instance, error) return lo.Map(instances, func(i *ec2.Instance, _ int) *Instance { return NewInstance(i) }), nil } -func combineFleetErrors(errors []*ec2.CreateFleetError) (errs error) { +func combineFleetErrors(fleetErrs []*ec2.CreateFleetError) (errs error) { unique := sets.NewString() - for _, err := range errors { + for _, err := range fleetErrs { unique.Insert(fmt.Sprintf("%s: %s", aws.StringValue(err.ErrorCode), aws.StringValue(err.ErrorMessage))) } for errorCode := range unique { - errs = multierr.Append(errs, fmt.Errorf(errorCode)) + errs = multierr.Append(errs, errors.New(errorCode)) } // If all the Fleet errors are ICE errors then we should wrap the combined error in the generic ICE error - iceErrorCount := lo.CountBy(errors, func(err *ec2.CreateFleetError) bool { return awserrors.IsUnfulfillableCapacity(err) }) - if iceErrorCount == len(errors) { + iceErrorCount := lo.CountBy(fleetErrs, func(err *ec2.CreateFleetError) bool { return awserrors.IsUnfulfillableCapacity(err) }) + if iceErrorCount == len(fleetErrs) { return cloudprovider.NewInsufficientCapacityError(fmt.Errorf("with fleet error(s), %w", errs)) } return fmt.Errorf("with fleet error(s), %w", errs) diff --git a/pkg/test/environment.go b/pkg/test/environment.go index dcf2c73157c5..276615f19780 100644 --- a/pkg/test/environment.go +++ b/pkg/test/environment.go @@ -67,7 +67,7 @@ type Environment struct { SubnetCache *cache.Cache SecurityGroupCache *cache.Cache InstanceProfileCache *cache.Cache - SSMProviderCache *cache.Cache + SSMProviderCache *cache.Cache // Providers InstanceTypesProvider *instancetype.DefaultProvider @@ -151,7 +151,7 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment SecurityGroupCache: securityGroupCache, InstanceProfileCache: instanceProfileCache, UnavailableOfferingsCache: unavailableOfferingsCache, - SSMProviderCache: ssmProviderCache, + SSMProviderCache: ssmProviderCache, InstanceTypesProvider: instanceTypesProvider, InstanceProvider: instanceProvider,