diff --git a/controlplane/kubeadm/internal/filters.go b/controlplane/kubeadm/internal/filters.go index 537fa5a4affe..812ddb05a4a1 100644 --- a/controlplane/kubeadm/internal/filters.go +++ b/controlplane/kubeadm/internal/filters.go @@ -288,42 +288,28 @@ func getAdjustedKcpConfig(kcp *controlplanev1.KubeadmControlPlane, machineConfig } // cleanupConfigFields cleanups all the fields that are not relevant for the comparison. +// Note: This function assumes that old defaults have been applied to kcpConfig and machineConfig +// as a consequence we can assume JoinConfiguration and JoinConfiguration.NodeRegistration are always defined. func cleanupConfigFields(kcpConfig *bootstrapv1.KubeadmConfigSpec, machineConfig *bootstrapv1.KubeadmConfig) { - // KCP ClusterConfiguration will only be compared with a machine's ClusterConfiguration annotation, so - // we are cleaning up from the reflect.DeepEqual comparison. + // KCP ClusterConfiguration will be compared in `matchClusterConfiguration` so we are cleaning it up here + // so it doesn't lead to a duplicate diff in `matchInitOrJoinConfiguration`. kcpConfig.ClusterConfiguration = bootstrapv1.ClusterConfiguration{} machineConfig.Spec.ClusterConfiguration = bootstrapv1.ClusterConfiguration{} - // If KCP JoinConfiguration is not present, set machine JoinConfiguration to nil (nothing can trigger rollout here). - // NOTE: this is required because CABPK applies an empty joinConfiguration in case no one is provided. - if !kcpConfig.JoinConfiguration.IsDefined() { - machineConfig.Spec.JoinConfiguration = bootstrapv1.JoinConfiguration{} - } - // Cleanup JoinConfiguration.Discovery from kcpConfig and machineConfig, because those info are relevant only for // the join process and not for comparing the configuration of the machine. - emptyDiscovery := bootstrapv1.Discovery{} - if kcpConfig.JoinConfiguration.IsDefined() { - kcpConfig.JoinConfiguration.Discovery = emptyDiscovery - } - if machineConfig.Spec.JoinConfiguration.IsDefined() { - machineConfig.Spec.JoinConfiguration.Discovery = emptyDiscovery - } - - // If KCP JoinConfiguration.ControlPlane is not present, set machine join configuration to nil (nothing can trigger rollout here). - // NOTE: this is required because CABPK applies an empty joinConfiguration.ControlPlane in case no one is provided. - if kcpConfig.JoinConfiguration.IsDefined() && kcpConfig.JoinConfiguration.ControlPlane == nil && - machineConfig.Spec.JoinConfiguration.ControlPlane != nil { + // Note: Changes to Discovery will apply for the next join, but they will not lead to a rollout. + kcpConfig.JoinConfiguration.Discovery = bootstrapv1.Discovery{} + machineConfig.Spec.JoinConfiguration.Discovery = bootstrapv1.Discovery{} + + // 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 kcpConfig.JoinConfiguration.ControlPlane == nil && + reflect.DeepEqual(machineConfig.Spec.JoinConfiguration.ControlPlane, &bootstrapv1.JoinControlPlane{}) { machineConfig.Spec.JoinConfiguration.ControlPlane = nil } - // If KCP's join NodeRegistration is empty, set machine's node registration to empty as no changes should trigger rollout. - emptyNodeRegistration := bootstrapv1.NodeRegistrationOptions{} - if kcpConfig.JoinConfiguration.IsDefined() && reflect.DeepEqual(kcpConfig.JoinConfiguration.NodeRegistration, emptyNodeRegistration) && - !reflect.DeepEqual(machineConfig.Spec.JoinConfiguration.NodeRegistration, emptyNodeRegistration) { - machineConfig.Spec.JoinConfiguration.NodeRegistration = emptyNodeRegistration - } - // 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(kcpConfig) diff --git a/controlplane/kubeadm/internal/filters_test.go b/controlplane/kubeadm/internal/filters_test.go index 4b6d492dbbad..243908a7dd70 100644 --- a/controlplane/kubeadm/internal/filters_test.go +++ b/controlplane/kubeadm/internal/filters_test.go @@ -333,22 +333,6 @@ func TestCleanupConfigFields(t *testing.T) { g.Expect(kcpConfig.ClusterConfiguration.IsDefined()).To(BeFalse()) g.Expect(machineConfig.Spec.ClusterConfiguration.IsDefined()).To(BeFalse()) }) - t.Run("JoinConfiguration gets removed from MachineConfig if it was not derived by KCPConfig", func(t *testing.T) { - g := NewWithT(t) - kcpConfig := &bootstrapv1.KubeadmConfigSpec{ - JoinConfiguration: bootstrapv1.JoinConfiguration{}, // KCP not providing a JoinConfiguration - } - machineConfig := &bootstrapv1.KubeadmConfig{ - Spec: bootstrapv1.KubeadmConfigSpec{ - JoinConfiguration: bootstrapv1.JoinConfiguration{ - ControlPlane: &bootstrapv1.JoinControlPlane{}, - }, // Machine gets a default JoinConfiguration from CABPK - }, - } - cleanupConfigFields(kcpConfig, machineConfig) - g.Expect(kcpConfig.JoinConfiguration.IsDefined()).To(BeFalse()) - g.Expect(machineConfig.Spec.JoinConfiguration.IsDefined()).To(BeFalse()) - }) t.Run("JoinConfiguration.Discovery gets removed because it is not relevant for compare", func(t *testing.T) { g := NewWithT(t) kcpConfig := &bootstrapv1.KubeadmConfigSpec{ @@ -367,7 +351,7 @@ func TestCleanupConfigFields(t *testing.T) { g.Expect(kcpConfig.JoinConfiguration.Discovery).To(BeComparableTo(bootstrapv1.Discovery{})) g.Expect(machineConfig.Spec.JoinConfiguration.Discovery).To(BeComparableTo(bootstrapv1.Discovery{})) }) - t.Run("JoinConfiguration.ControlPlane gets removed from MachineConfig if it was not derived by KCPConfig", func(t *testing.T) { + t.Run("JoinConfiguration.ControlPlane gets removed from MachineConfig if it was added by CABPK", func(t *testing.T) { g := NewWithT(t) kcpConfig := &bootstrapv1.KubeadmConfigSpec{ JoinConfiguration: bootstrapv1.JoinConfiguration{ @@ -385,23 +369,28 @@ func TestCleanupConfigFields(t *testing.T) { g.Expect(kcpConfig.JoinConfiguration).ToNot(BeNil()) g.Expect(machineConfig.Spec.JoinConfiguration.ControlPlane).To(BeNil()) }) - t.Run("JoinConfiguration.NodeRegistrationOptions gets removed from MachineConfig if it was not derived by KCPConfig", func(t *testing.T) { + t.Run("JoinConfiguration.ControlPlane gets not removed from MachineConfig if it is not empty", func(t *testing.T) { g := NewWithT(t) kcpConfig := &bootstrapv1.KubeadmConfigSpec{ JoinConfiguration: bootstrapv1.JoinConfiguration{ - NodeRegistration: bootstrapv1.NodeRegistrationOptions{}, // NodeRegistrationOptions configuration missing in KCP + ControlPlane: nil, // Control plane configuration missing in KCP }, } machineConfig := &bootstrapv1.KubeadmConfig{ Spec: bootstrapv1.KubeadmConfigSpec{ JoinConfiguration: bootstrapv1.JoinConfiguration{ - NodeRegistration: bootstrapv1.NodeRegistrationOptions{Name: "test"}, // Machine gets a some JoinConfiguration.NodeRegistrationOptions + ControlPlane: &bootstrapv1.JoinControlPlane{ + LocalAPIEndpoint: bootstrapv1.APIEndpoint{ + AdvertiseAddress: "1.2.3.4", + BindPort: 6443, + }, + }, }, }, } cleanupConfigFields(kcpConfig, machineConfig) g.Expect(kcpConfig.JoinConfiguration).ToNot(BeNil()) - g.Expect(machineConfig.Spec.JoinConfiguration.NodeRegistration).To(BeComparableTo(bootstrapv1.NodeRegistrationOptions{})) + g.Expect(machineConfig.Spec.JoinConfiguration.ControlPlane).ToNot(BeNil()) }) t.Run("drops omittable fields", func(t *testing.T) { g := NewWithT(t) @@ -609,6 +598,53 @@ func TestMatchInitOrJoinConfiguration(t *testing.T) { Files: nil, DiskSetup: {}, ... // 9 identical fields + }`)) + }) + t.Run("returns false if JoinConfiguration is NOT equal", func(t *testing.T) { + g := NewWithT(t) + kcp := &controlplanev1.KubeadmControlPlane{ + Spec: controlplanev1.KubeadmControlPlaneSpec{ + KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ + ClusterConfiguration: bootstrapv1.ClusterConfiguration{}, + InitConfiguration: bootstrapv1.InitConfiguration{}, + // JoinConfiguration not set anymore. + }, + }, + } + machineConfig := &bootstrapv1.KubeadmConfig{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "test", + }, + Spec: bootstrapv1.KubeadmConfigSpec{ + JoinConfiguration: bootstrapv1.JoinConfiguration{ + NodeRegistration: bootstrapv1.NodeRegistrationOptions{ + Name: "An old name", // This is a change + }, + }, + }, + } + match, diff, err := matchInitOrJoinConfiguration(machineConfig, kcp) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(match).To(BeFalse()) + g.Expect(diff).To(BeComparableTo(`&v1beta2.KubeadmConfigSpec{ + ClusterConfiguration: {}, + InitConfiguration: {NodeRegistration: {ImagePullPolicy: "IfNotPresent"}}, + JoinConfiguration: v1beta2.JoinConfiguration{ + NodeRegistration: v1beta2.NodeRegistrationOptions{ +- Name: "An old name", ++ Name: "", + CRISocket: "", + Taints: nil, + ... // 4 identical fields + }, + CACertPath: "", + Discovery: {}, + ... // 4 identical fields + }, + Files: nil, + DiskSetup: {}, + ... // 9 identical fields }`)) }) t.Run("returns true if returns true if only omittable configurations are not equal", func(t *testing.T) {