Change KubeadmConfig bootstrap config reference to ensure nodes get…

… rolled when making changes to node specification, add instance warmup setting, run kubeadm after containerd (#457) kubeadm fix taken from kubernetes-sigs/cluster-api-provider-aws#4529
giantswarm · Dec 21, 2023 · 6253fca · 6253fca
1 parent 5d80dd6
commit 6253fca
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 55 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,11 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Change `KubeadmConfig` bootstrap config reference to ensure nodes get rolled when making changes to node specification (requires newer versions of CAPI/CAPA as shown in the original [issue](https://github.com/kubernetes-sigs/cluster-api/issues/8858)). Add machine pool instance warmup setting (5 minutes) to ensure nodes do not get replaced too quickly.
+- Run kubeadm after containerd to avoid node startup problems
+
 ## [0.54.0] - 2023-12-21
 
 ### **Breaking change**
 
-- Added option to customize app via configmap or secret with `global.apps.{app_name}.extraConfigs`. 
+- Added option to customize app via configmap or secret with `global.apps.{app_name}.extraConfigs`.
 - In-line custom values for app moved from `global.apps.{app_name}` to `global.apps.{app_name}.values`.
 
 ## [0.53.0] - 2023-12-13

diff --git a/helm/cluster-aws/templates/_helpers.tpl b/helm/cluster-aws/templates/_helpers.tpl
@@ -282,6 +282,8 @@ imageLookupOrg: "706635527432"
       # kubeadm must run after coreos-metadata populated /run/metadata directory.
       Requires=coreos-metadata.service
       After=coreos-metadata.service
+      # kubeadm must run after containerd - see https://github.com/kubernetes-sigs/image-builder/issues/939.
+      After=containerd.service
       [Service]
       # Ensure kubeadm service has access to kubeadm binary in /opt/bin on Flatcar.
       Environment=PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/opt/bin

diff --git a/helm/cluster-aws/templates/_machine_pools.tpl b/helm/cluster-aws/templates/_machine_pools.tpl
@@ -1,5 +1,81 @@
+{{/*
+CAPA reconciliation currently has a shortcoming that doesn't lead to rolling AWSMachinePool nodes
+if the referenced `KubeadmConfig` changes (such as `KubeadmConfig.spec.{files,preKubeadmCommands,...}`).
+While not solved by our long-term involvement in this bug, we hash `KubeadmConfig.spec` and rename
+the `KubeadmConfig` name since that enforces a new bootstrap secret name, triggering a rollout of
+nodes in CAPA's `AWSMachinePool` reconciler. Mind that CAPI also has a bug where CAPA only
+gets triggered after a few minutes.
+
+All of this is detailed in https://github.com/giantswarm/roadmap/issues/2217 and
+https://github.com/kubernetes-sigs/cluster-api/issues/8858, so please follow
+the issues to find out when we can get rid of our own checksum workaround.
+
+Full-line comments are excluded from the hash so that simple changes will not lead to
+node rollouts and thus possibly interruped workloads for customers.
+*/}}
+{{- define "machinepool-kubeadmconfig-spec-hash" -}}
+{{ $spec := include "machinepool-kubeadmconfig-spec" $ }}{{ regexReplaceAll `^\s*#.*$` $spec "" | sha256sum | trunc 5 }}
+{{- end -}}
+{{- define "machinepool-kubeadmconfig-spec" -}}
+{{- $_unused := required "nodePoolName must be set" $.nodePoolName -}}
+{{- $_unused := required "nodePoolObject must be set" $.nodePoolObject -}}
+format: ignition
+ignition:
+  containerLinuxConfig:
+    additionalConfig: |
+      systemd:
+        units:
+        {{- include "flatcarSystemdUnits" $ | nindent 10 }}
+        {{- if $.Values.internal.teleport.enabled }}
+        {{- include "teleportSystemdUnits" $ | nindent 10 }}
+        {{- end }}
+      storage:
+        directories:
+        {{- include "nodeDirectories" $ | nindent 10 }}
+joinConfiguration:
+  discovery: {}
+  nodeRegistration:
+    kubeletExtraArgs:
+      cloud-provider: external
+      {{- if $.Values.internal.cgroupsv1 }}
+      cgroup-driver: cgroupfs
+      {{- end }}
+      feature-gates: CronJobTimeZone=true
+      healthz-bind-address: 0.0.0.0
+      node-ip: ${COREOS_EC2_IPV4_LOCAL}
+      node-labels: role=worker,giantswarm.io/machine-pool={{ include "resource.default.name" $ }}-{{ $.nodePoolName }},{{- join "," $.nodePoolObject.customNodeLabels }}
+      v: "2"
+    name: ${COREOS_EC2_HOSTNAME}
+    {{- if $.nodePoolObject.customNodeTaints }}
+    {{- if (gt (len $.nodePoolObject.customNodeTaints) 0) }}
+    taints:
+    {{- range $.nodePoolObject.customNodeTaints }}
+    - key: {{ .key | quote }}
+      value: {{ .value | quote }}
+      effect: {{ .effect | quote }}
+    {{- end }}
+    {{- end }}
+    {{- end }}
+preKubeadmCommands:
+  {{- include "flatcarKubeadmPreCommands" . | nindent 2 }}
+  {{- if $.Values.global.connectivity.proxy.enabled }}{{- include "proxyCommand" $ | nindent 2 }}{{- end }}
+files:
+{{- include "kubeletConfigFiles" $ | nindent 0 }}
+{{- if $.Values.global.connectivity.proxy.enabled }}{{- include "proxyFiles" $ | nindent 0 }}{{- end }}
+{{- include "containerdConfigFiles" $ | nindent 0 }}
+{{- if $.Values.internal.teleport.enabled }}
+{{- include "teleportFiles" $ | nindent 0 }}
+{{- end }}
+{{- if $.Values.internal.cgroupsv1 }}
+{{- include "cgroupv1Files" $ | nindent 0 }}
+{{- end }}
+{{- include "nodeConfigFiles" $ | nindent 0 }}
+{{- end }}
+
 {{- define "machine-pools" }}
 {{- range $name, $value := .Values.global.nodePools | default .Values.internal.nodePools }}
+{{- $ := set $ "nodePoolName" $name }}
+{{- $ := set $ "nodePoolObject" $value }}
 apiVersion: cluster.x-k8s.io/v1beta1
 kind: MachinePool
 metadata:
@@ -21,7 +97,7 @@ spec:
         configRef:
           apiVersion: bootstrap.cluster.x-k8s.io/v1beta1
           kind: KubeadmConfig
-          name: {{ include "resource.default.name" $ }}-{{ $name }}
+          name: {{ include "resource.default.name" $ }}-{{ $name }}-{{ include "machinepool-kubeadmconfig-spec-hash" $ }}
       clusterName: {{ include "resource.default.name" $ }}
       infrastructureRef:
         apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
@@ -93,6 +169,8 @@ spec:
     {{- end }}
     {{- end }}
   {{- end }}
+  refreshPreferences:
+    instanceWarmup: 300
 ---
 apiVersion: bootstrap.cluster.x-k8s.io/v1beta1
 kind: KubeadmConfig
@@ -101,60 +179,9 @@ metadata:
     giantswarm.io/machine-pool: {{ include "resource.default.name" $ }}-{{ $name }}
     {{- include "labels.common" $ | nindent 4 }}
     app.kubernetes.io/version: {{ $.Chart.Version | quote }}
-  name: {{ include "resource.default.name" $ }}-{{ $name }}
+  name: {{ include "resource.default.name" $ }}-{{ $name }}-{{ include "machinepool-kubeadmconfig-spec-hash" $ }}
   namespace: {{ $.Release.Namespace }}
-spec:
-  format: ignition
-  ignition:
-    containerLinuxConfig:
-      additionalConfig: |
-        systemd:
-          units:
-          {{- include "flatcarSystemdUnits" $ | nindent 12 }}
-          {{- if $.Values.internal.teleport.enabled }}
-          {{- include "teleportSystemdUnits" $ | nindent 12 }}
-          {{- end }}
-        storage:
-          directories:
-          {{- include "nodeDirectories" $ | nindent 12 }}
-  joinConfiguration:
-    discovery: {}
-    nodeRegistration:
-      kubeletExtraArgs:
-        cloud-provider: external
-        {{- if $.Values.internal.cgroupsv1 }}
-        cgroup-driver: cgroupfs
-        {{- end }}
-        feature-gates: CronJobTimeZone=true
-        healthz-bind-address: 0.0.0.0
-        node-ip: ${COREOS_EC2_IPV4_LOCAL}
-        node-labels: role=worker,giantswarm.io/machine-pool={{ include "resource.default.name" $ }}-{{ $name }},{{- join "," $value.customNodeLabels }}
-        v: "2"
-      name: ${COREOS_EC2_HOSTNAME}
-      {{- if $value.customNodeTaints }}
-      {{- if (gt (len $value.customNodeTaints) 0) }}
-      taints:
-      {{- range $value.customNodeTaints }}
-      - key: {{ .key | quote }}
-        value: {{ .value | quote }}
-        effect: {{ .effect | quote }}
-      {{- end }}
-      {{- end }}
-      {{- end }}
-  preKubeadmCommands:
-    {{- include "flatcarKubeadmPreCommands" . | nindent 4 }}
-    {{- if $.Values.global.connectivity.proxy.enabled }}{{- include "proxyCommand" $ | nindent 4 }}{{- end }}
-  files:
-  {{- include "kubeletConfigFiles" $ | nindent 2 }}
-  {{- if $.Values.global.connectivity.proxy.enabled }}{{- include "proxyFiles" $ | nindent 2 }}{{- end }}
-  {{- include "containerdConfigFiles" $ | nindent 2 }}
-  {{- if $.Values.internal.teleport.enabled }}
-  {{- include "teleportFiles" $ | nindent 2 }}
-  {{- end }}
-  {{- if $.Values.internal.cgroupsv1 }}
-  {{- include "cgroupv1Files" $ | nindent 2 }}
-  {{- end }}
-  {{- include "nodeConfigFiles" $ | nindent 2 }}
+spec: {{- include "machinepool-kubeadmconfig-spec" $ | nindent 2 }}
 ---
 {{ end }}
 {{- end -}}