Skip to content

Commit

Permalink
(backport) chore: Remove Post Install Hook (#6827) for v1.0.x (#6837)
Browse files Browse the repository at this point in the history
  • Loading branch information
engedaam authored Aug 22, 2024
1 parent 5bdf9c3 commit 365113f
Show file tree
Hide file tree
Showing 9 changed files with 9 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1343,7 +1343,7 @@ spec:
clientConfig:
service:
name: {{ .Values.webhook.serviceName }}
namespace: {{ .Values.webhook.serviceNamespace }}
namespace: {{ .Release.Namespace }}
port: {{ .Values.webhook.port }}
{{- end }}

Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,7 @@ spec:
clientConfig:
service:
name: {{ .Values.webhook.serviceName }}
namespace: {{ .Values.webhook.serviceNamespace }}
namespace: {{ .Release.Namespace }}
port: {{ .Values.webhook.port }}
{{- end }}

2 changes: 1 addition & 1 deletion charts/karpenter-crd/templates/karpenter.sh_nodepools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,7 @@ spec:
clientConfig:
service:
name: {{ .Values.webhook.serviceName }}
namespace: {{ .Values.webhook.serviceNamespace }}
namespace: {{ .Release.Namespace }}
port: {{ .Values.webhook.port }}
{{- end }}

3 changes: 1 addition & 2 deletions charts/karpenter-crd/values.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
webhook:
# -- Whether to enable the webhooks and webhook permissions.
# -- Whether to enable the webhooks.
enabled: true
serviceName: karpenter
serviceNamespace: kube-system
# -- The container port to use for the webhook.
port: 8443
11 changes: 0 additions & 11 deletions charts/karpenter/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -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" -}}
Expand Down
12 changes: 2 additions & 10 deletions charts/karpenter/templates/clusterrole-core.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,7 @@ rules:
{{- if .Values.webhook.enabled }}
- apiGroups: ["apiextensions.k8s.io"]
resources: ["customresourcedefinitions"]
verbs: ["get", "watch", "list"]
{{- else }}
- apiGroups: ["apiextensions.k8s.io"]
resources: ["customresourcedefinitions"]
verbs: ["get"]
verbs: ["watch", "list"]
{{- end }}
- apiGroups: ["policy"]
resources: ["poddisruptionbudgets"]
Expand All @@ -75,11 +71,7 @@ rules:
{{- if .Values.webhook.enabled }}
- apiGroups: ["apiextensions.k8s.io"]
resources: ["customresourcedefinitions"]
verbs: ["update", "patch"]
{{- else }}
- apiGroups: ["apiextensions.k8s.io"]
resources: ["customresourcedefinitions"]
verbs: ["patch"]
verbs: ["update"]
{{- end }}
{{- with .Values.additionalClusterRoleRules -}}
{{ toYaml . | nindent 2 }}
Expand Down
41 changes: 0 additions & 41 deletions charts/karpenter/templates/post-install-hook.yaml

This file was deleted.

8 changes: 0 additions & 8 deletions charts/karpenter/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,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: true
Expand Down
6 changes: 3 additions & 3 deletions hack/mutation/conversion_webhooks_injection.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

2 comments on commit 365113f

@promzeus
Copy link

@promzeus promzeus commented on 365113f Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that something is wrong after these changes. If karpenter is placed in karpenter namespace, we get an error when NodePool is deployed:
karpenter_provisioner.yaml": conversion webhook for karpenter.sh/v1beta1, Kind=NodePool failed: Post "https://karpenter.kube-system.svc:8443/conversion/karpenter.sh?timeout=30s": service "karpenter" not found

resource "helm_release" "karpenter" {
  namespace           = "karpenter"
  create_namespace    = true

  name                = "karpenter"
  repository          = "oci://public.ecr.aws/karpenter"
  repository_username = data.aws_ecrpublic_authorization_token.token.user_name
  repository_password = data.aws_ecrpublic_authorization_token.token.password
  chart               = "karpenter"
  version             = "1.0.1"

  set {
    name  = "settings.clusterName"
    value = module.eks.cluster_name
  }
  set {
    name  = "settings.interruptionQueue"
    value = module.karpenter.queue_name
  }
  set {
    name  = "serviceAccount.annotations.eks\\.amazonaws\\.com/role-arn"
    value = module.karpenter.iam_role_arn
  }
  set {
    name  = "settings.defaultInstanceProfile"
    value = module.karpenter.instance_profile_name
  }
  set {
    name  = "controller.resources.requests.cpu"
    value = "1"
  }
  set {
    name  = "controller.resources.requests.memory"
    value = "1Gi"
  }
  set {
    name  = "controller.resources.limits.cpu"
    value = "1"
  }
  set {
    name  = "controller.resources.limits.memory"
    value = "1Gi"
  }
  set {
    name  = "serviceMonitor.enabled"
    value = "true"
  }  
  depends_on = [
    module.karpenter
  ]
}

tried adding
set {
name = “controller.webhooks.namespace”
value = “karpenter”
}
doesn't work

@indra0007
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree looks like namespace: {{ .Release.Namespace }} change got missed in karpenter chart

Please sign in to comment.