Merge pull request #13035 from sbueringer/pr-fix-KCP-isjoin

🐛 Fix KCP KubeadmConfig isJoin detection

Author: k8s-ci-robot

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

@@ -695,11 +695,6 @@ type Discovery struct {
 	TLSBootstrapToken string `json:"tlsBootstrapToken,omitempty"`
 }
 
-// IsDefined returns true if the Discovery is defined.
-func (r *Discovery) IsDefined() bool {
-	return !reflect.DeepEqual(r, &Discovery{})
-}
-
 // BootstrapTokenDiscovery is used to set the options for bootstrap token based discovery.
 // +kubebuilder:validation:MinProperties=1
 type BootstrapTokenDiscovery struct {

controlplane/kubeadm/internal/controllers/helpers_test.go

@@ -417,7 +417,9 @@ func TestCloneConfigsAndGenerateMachineAndSyncMachines(t *testing.T) {
 		UID:        kcp.UID,
 	}))
 	g.Expect(kubeadmConfig.Spec.InitConfiguration).To(BeComparableTo(bootstrapv1.InitConfiguration{}))
-	g.Expect(kubeadmConfig.Spec.JoinConfiguration).To(BeComparableTo(kcp.Spec.KubeadmConfigSpec.JoinConfiguration))
+	expectedJoinConfiguration := kcp.Spec.KubeadmConfigSpec.JoinConfiguration.DeepCopy()
+	expectedJoinConfiguration.ControlPlane = &bootstrapv1.JoinControlPlane{}
+	g.Expect(kubeadmConfig.Spec.JoinConfiguration).To(BeComparableTo(*expectedJoinConfiguration))
 	// Note: capi-kubeadmcontrolplane should own ownerReferences and spec, labels and annotations should be orphaned.
 	// 		 Labels and annotations will be owned by capi-kubeadmcontrolplane-metadata after the next update
 	//		 of labels and annotations.
@@ -433,6 +435,7 @@ func TestCloneConfigsAndGenerateMachineAndSyncMachines(t *testing.T) {
 },
 "f:spec":{
 	"f:joinConfiguration":{
+		"f:controlPlane":{},
 		"f:nodeRegistration":{
 			"f:kubeletExtraArgs":{
 				"k:{\"name\":\"v\",\"value\":\"8\"}":{
@@ -508,6 +511,7 @@ func TestCloneConfigsAndGenerateMachineAndSyncMachines(t *testing.T) {
 },
 "f:spec":{
 	"f:joinConfiguration":{
+		"f:controlPlane":{},
 		"f:nodeRegistration":{
 			"f:kubeletExtraArgs":{
 				"k:{\"name\":\"v\",\"value\":\"8\"}":{

controlplane/kubeadm/internal/controllers/inplace_canupdatemachine_test.go

@@ -202,6 +202,9 @@ func Test_canExtensionsUpdateMachine(t *testing.T) {
 					},
 				},
 			},
+			JoinConfiguration: bootstrapv1.JoinConfiguration{
+				ControlPlane: &bootstrapv1.JoinControlPlane{},
+			},
 		},
 	}
 	desiredKubeadmConfig := currentKubeadmConfig.DeepCopy()
@@ -323,7 +326,7 @@ func Test_canExtensionsUpdateMachine(t *testing.T) {
 +       "clusterConfiguration": map[string]any{"etcd": map[string]any{"local": map[string]any{"imageTag": string("3.6.4-0")}}},
         "format":               string("cloud-config"),
         "initConfiguration":    map[string]any{"nodeRegistration": map[string]any{"imagePullPolicy": string("IfNotPresent")}},
-        "joinConfiguration":    map[string]any{"nodeRegistration": map[string]any{"imagePullPolicy": string("IfNotPresent")}},
+        "joinConfiguration":    map[string]any{"controlPlane": map[string]any{}, "nodeRegistration": map[string]any{"imagePullPolicy": string("IfNotPresent")}},
       },
     },
   }`,
@@ -424,7 +427,7 @@ func Test_canExtensionsUpdateMachine(t *testing.T) {
 +       "clusterConfiguration": map[string]any{"etcd": map[string]any{"local": map[string]any{"imageTag": string("3.6.4-0")}}},
         "format":               string("cloud-config"),
         "initConfiguration":    map[string]any{"nodeRegistration": map[string]any{"imagePullPolicy": string("IfNotPresent")}},
-        "joinConfiguration":    map[string]any{"nodeRegistration": map[string]any{"imagePullPolicy": string("IfNotPresent")}},
+        "joinConfiguration":    map[string]any{"controlPlane": map[string]any{}, "nodeRegistration": map[string]any{"imagePullPolicy": string("IfNotPresent")}},
       },
     },
   }`,
@@ -619,12 +622,7 @@ func Test_createRequest(t *testing.T) {
 				},
 			},
 			JoinConfiguration: bootstrapv1.JoinConfiguration{
-				// This field is technically set by CABPK, but adding it here so that matchesKubeadmConfig detects this correctly as a join KubeadmConfig.
-				Discovery: bootstrapv1.Discovery{
-					BootstrapToken: bootstrapv1.BootstrapTokenDiscovery{
-						APIServerEndpoint: "1.2.3.4:6443",
-					},
-				},
+				ControlPlane: &bootstrapv1.JoinControlPlane{},
 				NodeRegistration: bootstrapv1.NodeRegistrationOptions{
 					KubeletExtraArgs: []bootstrapv1.Arg{{
 						Name:  "v",
@@ -641,7 +639,6 @@ func Test_createRequest(t *testing.T) {
 	currentKubeadmConfigCleanedUp.SetGroupVersionKind(bootstrapv1.GroupVersion.WithKind("KubeadmConfig")) // cleanupKubeadmConfig adds GVK.
 	currentKubeadmConfigCleanedUp.Status = bootstrapv1.KubeadmConfigStatus{}                              // cleanupKubeadmConfig drops status.
 	defaulting.ApplyPreviousKubeadmConfigDefaults(&currentKubeadmConfigCleanedUp.Spec)                    // PrepareKubeadmConfigsForDiff applies defaults.
-	currentKubeadmConfigCleanedUp.Spec.JoinConfiguration.Discovery = bootstrapv1.Discovery{}              // PrepareKubeadmConfigsForDiff cleans up Discovery.
 	currentKubeadmConfigWithOutdatedLabelsAndAnnotations := currentKubeadmConfig.DeepCopy()
 	currentKubeadmConfigWithOutdatedLabelsAndAnnotations.Labels["outdated-label-1"] = "outdated-label-value-1"
 	currentKubeadmConfigWithOutdatedLabelsAndAnnotations.Annotations["outdated-annotation-1"] = "outdated-annotation-value-1"
@@ -655,7 +652,6 @@ func Test_createRequest(t *testing.T) {
 	desiredKubeadmConfigCleanedUp.SetGroupVersionKind(bootstrapv1.GroupVersion.WithKind("KubeadmConfig")) // cleanupKubeadmConfig adds GVK.
 	desiredKubeadmConfigCleanedUp.Status = bootstrapv1.KubeadmConfigStatus{}                              // cleanupKubeadmConfig drops status.
 	defaulting.ApplyPreviousKubeadmConfigDefaults(&desiredKubeadmConfigCleanedUp.Spec)                    // PrepareKubeadmConfigsForDiff applies defaults.
-	desiredKubeadmConfigCleanedUp.Spec.JoinConfiguration.Discovery = bootstrapv1.Discovery{}              // PrepareKubeadmConfigsForDiff cleans up Discovery.
 
 	currentInfraMachine := &unstructured.Unstructured{
 		Object: map[string]interface{}{

controlplane/kubeadm/internal/desiredstate/desired_state.go

@@ -212,8 +212,13 @@ func ComputeDesiredKubeadmConfig(kcp *controlplanev1.KubeadmControlPlane, cluste
 	if isJoin {
 		// Note: When building a KubeadmConfig for a joining CP machine empty out the unnecessary InitConfiguration.
 		spec.InitConfiguration = bootstrapv1.InitConfiguration{}
-		// NOTE: For the joining we are preserving the ClusterConfiguration in order to determine if the
+		// Note: For the joining we are preserving the ClusterConfiguration in order to determine if the
 		// cluster is using an external etcd in the kubeadm bootstrap provider (even if this is not required by kubeadm Join).
+		// Note: We are always setting JoinConfiguration.ControlPlane so we can later identify this KubeadmConfig as a
+		// join KubeadmConfig.
+		if spec.JoinConfiguration.ControlPlane == nil {
+			spec.JoinConfiguration.ControlPlane = &bootstrapv1.JoinControlPlane{}
+		}
 	} else {
 		// Note: When building a KubeadmConfig for the first CP machine empty out the unnecessary JoinConfiguration.
 		spec.JoinConfiguration = bootstrapv1.JoinConfiguration{}

controlplane/kubeadm/internal/desiredstate/desired_state_test.go

@@ -619,6 +619,7 @@ func Test_ComputeDesiredKubeadmConfig(t *testing.T) {
 		if isJoin {
 			expectedKubeadmConfigWithoutOwner.Spec.InitConfiguration = bootstrapv1.InitConfiguration{}
 			expectedKubeadmConfigWithoutOwner.Spec.JoinConfiguration = kcp.Spec.KubeadmConfigSpec.JoinConfiguration
+			expectedKubeadmConfigWithoutOwner.Spec.JoinConfiguration.ControlPlane = &bootstrapv1.JoinControlPlane{}
 		} else {
 			expectedKubeadmConfigWithoutOwner.Spec.InitConfiguration = kcp.Spec.KubeadmConfigSpec.InitConfiguration
 			expectedKubeadmConfigWithoutOwner.Spec.JoinConfiguration = bootstrapv1.JoinConfiguration{}

controlplane/kubeadm/internal/filters.go

@@ -19,7 +19,6 @@ package internal
 import (
 	"context"
 	"fmt"
-	"reflect"
 
 	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -308,10 +307,8 @@ func PrepareKubeadmConfigsForDiff(desiredKubeadmConfig, currentKubeadmConfig *bo
 		currentKubeadmConfig.Spec.JoinConfiguration.Patches = currentKubeadmConfig.Spec.InitConfiguration.Patches
 		currentKubeadmConfig.Spec.JoinConfiguration.SkipPhases = currentKubeadmConfig.Spec.InitConfiguration.SkipPhases
 		currentKubeadmConfig.Spec.JoinConfiguration.NodeRegistration = currentKubeadmConfig.Spec.InitConfiguration.NodeRegistration
-		if currentKubeadmConfig.Spec.InitConfiguration.LocalAPIEndpoint.IsDefined() {
-			currentKubeadmConfig.Spec.JoinConfiguration.ControlPlane = &bootstrapv1.JoinControlPlane{
-				LocalAPIEndpoint: currentKubeadmConfig.Spec.InitConfiguration.LocalAPIEndpoint,
-			}
+		currentKubeadmConfig.Spec.JoinConfiguration.ControlPlane = &bootstrapv1.JoinControlPlane{
+			LocalAPIEndpoint: currentKubeadmConfig.Spec.InitConfiguration.LocalAPIEndpoint,
 		}
 		currentKubeadmConfig.Spec.InitConfiguration = bootstrapv1.InitConfiguration{}
 
@@ -351,14 +348,6 @@ func PrepareKubeadmConfigsForDiff(desiredKubeadmConfig, currentKubeadmConfig *bo
 	currentKubeadmConfig.Spec.InitConfiguration.Timeouts.ControlPlaneComponentHealthCheckSeconds = nil
 	currentKubeadmConfig.Spec.JoinConfiguration.Timeouts.ControlPlaneComponentHealthCheckSeconds = nil
 
-	// If KCP JoinConfiguration.ControlPlane is nil and the Machine JoinConfiguration.ControlPlane is empty,
-	// set Machine JoinConfiguration.ControlPlane to nil.
-	// NOTE: This is required because CABPK applies an empty JoinConfiguration.ControlPlane in case it is nil.
-	if desiredKubeadmConfig.Spec.JoinConfiguration.ControlPlane == nil &&
-		reflect.DeepEqual(currentKubeadmConfig.Spec.JoinConfiguration.ControlPlane, &bootstrapv1.JoinControlPlane{}) {
-		currentKubeadmConfig.Spec.JoinConfiguration.ControlPlane = nil
-	}
-
 	// Drop differences that do not lead to changes to Machines, but that might exist due
 	// to changes in how we serialize objects or how webhooks work.
 	dropOmittableFields(&desiredKubeadmConfig.Spec)
@@ -542,15 +531,15 @@ func dropOmittableFields(spec *bootstrapv1.KubeadmConfigSpec) {
 // isKubeadmConfigForJoin returns true if the KubeadmConfig is for a control plane
 // or a worker machine that joined an existing cluster.
 // Note: This check is based on the assumption that KubeadmConfig for joining
-// control plane and workers nodes always have a non-empty JoinConfiguration.Discovery, while
-// instead the JoinConfiguration for the first control plane machine in the
+// control plane always have a non-empty JoinConfiguration.ControlPlane, while
+// instead the entire JoinConfiguration for the first control plane machine in the
 // cluster is emptied out by KCP.
 // Note: Previously we checked if the entire JoinConfiguration is defined, but that
 // is not safe because apiServer.timeoutForControlPlane in v1beta1 is also converted to
 // joinConfiguration.timeouts.controlPlaneComponentHealthCheckSeconds in v1beta2 and
 // accordingly we would also detect init KubeadmConfigs as join.
 func isKubeadmConfigForJoin(c *bootstrapv1.KubeadmConfig) bool {
-	return c.Spec.JoinConfiguration.Discovery.IsDefined()
+	return c.Spec.JoinConfiguration.ControlPlane != nil
 }
 
 // isKubeadmConfigForInit returns true if the KubeadmConfig is for the first control plane