Merge pull request #12758 from sbueringer/pr-simplify-kcp-rollout

✨ KCP: compare ClusterConfiguration via KubeadmConfig instead of annotation on Machine

Author: k8s-ci-robot

api/controlplane/kubeadm/v1beta2/kubeadm_control_plane_types.go

@@ -46,10 +46,6 @@ const (
 	// SkipKubeProxyAnnotation annotation explicitly skips reconciling kube-proxy if set.
 	SkipKubeProxyAnnotation = "controlplane.cluster.x-k8s.io/skip-kube-proxy"
 
-	// KubeadmClusterConfigurationAnnotation is a machine annotation that stores the json-marshalled string of KCP ClusterConfiguration.
-	// This annotation is used to detect any changes in ClusterConfiguration and trigger machine rollout in KCP.
-	KubeadmClusterConfigurationAnnotation = "controlplane.cluster.x-k8s.io/kubeadm-cluster-configuration"
-
 	// RemediationInProgressAnnotation is used to keep track that a KCP remediation is in progress, and more
 	// specifically it tracks that the system is in between having deleted an unhealthy machine and recreating its replacement.
 	// NOTE: if something external to CAPI removes this annotation the system cannot detect the above situation; this can lead to

controlplane/kubeadm/internal/controllers/controller_test.go

@@ -983,6 +983,9 @@ func TestReconcileCertificateExpiries(t *testing.T) {
 
 	cluster := newCluster(&types.NamespacedName{Name: "foo", Namespace: metav1.NamespaceDefault})
 	kcp := &controlplanev1.KubeadmControlPlane{
+		Spec: controlplanev1.KubeadmControlPlaneSpec{
+			Version: "v1.30.0",
+		},
 		Status: controlplanev1.KubeadmControlPlaneStatus{
 			Initialization: controlplanev1.KubeadmControlPlaneInitializationStatus{
 				ControlPlaneInitialized: ptr.To(true),

controlplane/kubeadm/internal/controllers/helpers.go

@@ -347,6 +347,12 @@ func (r *KubeadmControlPlaneReconciler) updateMachine(ctx context.Context, machi
 	return updatedMachine, nil
 }
 
+// kubeadmClusterConfigurationAnnotation is an annotation that was set in Cluster API <= v1.11.
+// Starting with Cluster API v1.12 we remove it from existing Machines.
+//
+// Deprecated: This constant and corresponding cleanup code can be removed once we don't support upgrades from Cluster API v1.12 anymore.
+const kubeadmClusterConfigurationAnnotation = "controlplane.cluster.x-k8s.io/kubeadm-cluster-configuration"
+
 // computeDesiredMachine computes the desired Machine.
 // This Machine will be used during reconciliation to:
 // * create a new Machine
@@ -376,15 +382,6 @@ func (r *KubeadmControlPlaneReconciler) computeDesiredMachine(kcp *controlplanev
 		machineName = generatedMachineName
 		version = kcp.Spec.Version
 
-		// Machine's bootstrap config may be missing ClusterConfiguration if it is not the first machine in the control plane.
-		// We store ClusterConfiguration as annotation here to detect any changes in KCP ClusterConfiguration and rollout the machine if any.
-		// Nb. This annotation is read when comparing the KubeadmConfig to check if a machine needs to be rolled out.
-		clusterConfigurationAnnotation, err := internal.ClusterConfigurationToMachineAnnotationValue(&kcp.Spec.KubeadmConfigSpec.ClusterConfiguration)
-		if err != nil {
-			return nil, err
-		}
-		annotations[controlplanev1.KubeadmClusterConfigurationAnnotation] = clusterConfigurationAnnotation
-
 		// In case this machine is being created as a consequence of a remediation, then add an annotation
 		// tracking remediating data.
 		// NOTE: This is required in order to track remediation retries.
@@ -397,24 +394,8 @@ func (r *KubeadmControlPlaneReconciler) computeDesiredMachine(kcp *controlplanev
 		machineUID = existingMachine.UID
 		version = existingMachine.Spec.Version
 
-		// For existing machine only set the ClusterConfiguration annotation if the machine already has it.
-		// We should not add the annotation if it was missing in the first place because we do not have enough
-		// information.
-		if clusterConfigurationAnnotation, ok := existingMachine.Annotations[controlplanev1.KubeadmClusterConfigurationAnnotation]; ok {
-			// In case the annotation is outdated, update it.
-			if internal.ClusterConfigurationAnnotationFromMachineIsOutdated(clusterConfigurationAnnotation) {
-				clusterConfiguration, err := internal.ClusterConfigurationFromMachine(existingMachine)
-				if err != nil {
-					return nil, err
-				}
-
-				clusterConfigurationAnnotation, err = internal.ClusterConfigurationToMachineAnnotationValue(clusterConfiguration)
-				if err != nil {
-					return nil, err
-				}
-			}
-			annotations[controlplanev1.KubeadmClusterConfigurationAnnotation] = clusterConfigurationAnnotation
-		}
+		// Cleanup the KubeadmClusterConfigurationAnnotation annotation that was set in Cluster API <= v1.11.
+		delete(annotations, kubeadmClusterConfigurationAnnotation)
 
 		// If the machine already has remediation data then preserve it.
 		// NOTE: This is required in order to track remediation retries.

controlplane/kubeadm/internal/controllers/helpers_test.go

@@ -508,13 +508,11 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
 	}
 
 	tests := []struct {
-		name                                   string
-		kcp                                    *controlplanev1.KubeadmControlPlane
-		isUpdatingExistingMachine              bool
-		existingClusterConfigurationAnnotation string
-		want                                   []gomegatypes.GomegaMatcher
-		wantClusterConfigurationAnnotation     string
-		wantErr                                bool
+		name                      string
+		kcp                       *controlplanev1.KubeadmControlPlane
+		isUpdatingExistingMachine bool
+		want                      []gomegatypes.GomegaMatcher
+		wantErr                   bool
 	}{
 		{
 			name: "should return the correct Machine object when creating a new Machine",
@@ -555,8 +553,7 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
 				HavePrefix(kcpName + namingTemplateKey),
 				Not(HaveSuffix("00000")),
 			},
-			wantClusterConfigurationAnnotation: "{\"marshalVersion\":\"v1beta2\",\"certificatesDir\":\"foo\"}",
-			wantErr:                            false,
+			wantErr: false,
 		},
 		{
 			name: "should return error when creating a new Machine when '.random' is not added in template",
@@ -624,8 +621,7 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
 				ContainSubstring(fmt.Sprintf("%053d", 0)),
 				Not(HaveSuffix("00000")),
 			},
-			wantClusterConfigurationAnnotation: "{\"marshalVersion\":\"v1beta2\",\"certificatesDir\":\"foo\"}",
-			wantErr:                            false,
+			wantErr: false,
 		},
 		{
 			name: "should return error when creating a new Machine with invalid template",
@@ -691,7 +687,6 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
 				HavePrefix(kcpName),
 				Not(HaveSuffix("00000")),
 			},
-			wantClusterConfigurationAnnotation: "{\"marshalVersion\":\"v1beta2\",\"certificatesDir\":\"foo\"}",
 		},
 		{
 			name: "should return the correct Machine object when creating a new Machine with additional kcp readinessGates",
@@ -724,9 +719,8 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
 					},
 				},
 			},
-			isUpdatingExistingMachine:          false,
-			wantClusterConfigurationAnnotation: "{\"marshalVersion\":\"v1beta2\",\"certificatesDir\":\"foo\"}",
-			wantErr:                            false,
+			isUpdatingExistingMachine: false,
+			wantErr:                   false,
 		},
 		{
 			name: "should return the correct Machine object when updating an existing Machine (empty ClusterConfiguration annotation)",
@@ -762,10 +756,8 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
 					},
 				},
 			},
-			isUpdatingExistingMachine:              true,
-			existingClusterConfigurationAnnotation: "",
-			wantClusterConfigurationAnnotation:     "",
-			wantErr:                                false,
+			isUpdatingExistingMachine: true,
+			wantErr:                   false,
 		},
 		{
 			name: "should return the correct Machine object when updating an existing Machine (outdated ClusterConfiguration annotation)",
@@ -802,10 +794,7 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
 				},
 			},
 			isUpdatingExistingMachine: true,
-
-			existingClusterConfigurationAnnotation: "{\"etcd\":{},\"apiServer\":{\"extraArgs\":{\"foo\":\"bar\"}},\"certificatesDir\":\"foo\"}",
-			wantClusterConfigurationAnnotation:     "{\"marshalVersion\":\"v1beta2\",\"apiServer\":{\"extraArgs\":[{\"name\":\"foo\",\"value\":\"bar\"}]},\"certificatesDir\":\"foo\"}",
-			wantErr:                                false,
+			wantErr:                   false,
 		},
 		{
 			name: "should return the correct Machine object when updating an existing Machine (up to date ClusterConfiguration annotation)",
@@ -841,10 +830,8 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
 					},
 				},
 			},
-			isUpdatingExistingMachine:              true,
-			existingClusterConfigurationAnnotation: "{\"marshalVersion\":\"v1beta2\",\"etcd\":{},\"apiServer\":{\"extraArgs\":[{\"name\":\"foo\",\"value\":\"bar\"}]},\"controllerManager\":{},\"scheduler\":{},\"dns\":{},\"certificatesDir\":\"foo\"}",
-			wantClusterConfigurationAnnotation:     "{\"marshalVersion\":\"v1beta2\",\"etcd\":{},\"apiServer\":{\"extraArgs\":[{\"name\":\"foo\",\"value\":\"bar\"}]},\"controllerManager\":{},\"scheduler\":{},\"dns\":{},\"certificatesDir\":\"foo\"}",
-			wantErr:                                false,
+			isUpdatingExistingMachine: true,
+			wantErr:                   false,
 		},
 	}
 
@@ -887,9 +874,6 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
 						ReadinessGates:    []clusterv1.MachineReadinessGate{{ConditionType: "Foo"}},
 					},
 				}
-				if tt.existingClusterConfigurationAnnotation != "" {
-					existingMachine.Annotations[controlplanev1.KubeadmClusterConfigurationAnnotation] = tt.existingClusterConfigurationAnnotation
-				}
 
 				desiredMachine, err = (&KubeadmControlPlaneReconciler{}).computeDesiredMachine(
 					tt.kcp, cluster,
@@ -924,9 +908,6 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
 				for k, v := range kcpMachineTemplateObjectMeta.Annotations {
 					expectedAnnotations[k] = v
 				}
-				if tt.wantClusterConfigurationAnnotation != "" {
-					expectedAnnotations[controlplanev1.KubeadmClusterConfigurationAnnotation] = tt.wantClusterConfigurationAnnotation
-				}
 				expectedAnnotations[controlplanev1.RemediationForAnnotation] = remediationData
 				// The pre-terminate annotation should always be added
 				expectedAnnotations[controlplanev1.PreTerminateHookCleanupAnnotation] = ""
@@ -962,7 +943,6 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
 				for k, v := range kcpMachineTemplateObjectMeta.Annotations {
 					expectedAnnotations[k] = v
 				}
-				expectedAnnotations[controlplanev1.KubeadmClusterConfigurationAnnotation] = tt.wantClusterConfigurationAnnotation
 				// The pre-terminate annotation should always be added
 				expectedAnnotations[controlplanev1.PreTerminateHookCleanupAnnotation] = ""
 				g.Expect(desiredMachine.Annotations).To(Equal(expectedAnnotations))