From 7626629c841e987305abb4f6abb891616d54cd31 Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Tue, 1 Oct 2024 11:15:45 +0200 Subject: [PATCH] Fix tests and in place propagation for MachineDeployment and MachineSet --- .../machinedeployment_sync.go | 1 + .../machinedeployment_sync_test.go | 4 ++ .../machinedeployment/mdutil/util.go | 1 + .../machinedeployment/mdutil/util_test.go | 51 ++++++++++--------- .../machineset/machineset_controller.go | 1 + .../machineset/machineset_controller_test.go | 4 ++ 6 files changed, 37 insertions(+), 25 deletions(-) diff --git a/internal/controllers/machinedeployment/machinedeployment_sync.go b/internal/controllers/machinedeployment/machinedeployment_sync.go index 8f4fb8cc50a0..df1288eaac4d 100644 --- a/internal/controllers/machinedeployment/machinedeployment_sync.go +++ b/internal/controllers/machinedeployment/machinedeployment_sync.go @@ -324,6 +324,7 @@ func (r *Reconciler) computeDesiredMachineSet(ctx context.Context, deployment *c } else { desiredMS.Spec.DeletePolicy = "" } + desiredMS.Spec.Template.Spec.ReadinessGates = deployment.Spec.Template.Spec.ReadinessGates desiredMS.Spec.Template.Spec.NodeDrainTimeout = deployment.Spec.Template.Spec.NodeDrainTimeout desiredMS.Spec.Template.Spec.NodeDeletionTimeout = deployment.Spec.Template.Spec.NodeDeletionTimeout desiredMS.Spec.Template.Spec.NodeVolumeDetachTimeout = deployment.Spec.Template.Spec.NodeVolumeDetachTimeout diff --git a/internal/controllers/machinedeployment/machinedeployment_sync_test.go b/internal/controllers/machinedeployment/machinedeployment_sync_test.go index 9169f1882e40..3904fb676894 100644 --- a/internal/controllers/machinedeployment/machinedeployment_sync_test.go +++ b/internal/controllers/machinedeployment/machinedeployment_sync_test.go @@ -582,6 +582,7 @@ func TestComputeDesiredMachineSet(t *testing.T) { Bootstrap: clusterv1.Bootstrap{ ConfigRef: &bootstrapRef, }, + ReadinessGates: []clusterv1.MachineReadinessGate{{ConditionType: "foo"}}, NodeDrainTimeout: duration10s, NodeVolumeDetachTimeout: duration10s, NodeDeletionTimeout: duration10s, @@ -649,6 +650,7 @@ func TestComputeDesiredMachineSet(t *testing.T) { "ms-label-2": "ms-value-2", } existingMS.Spec.Template.Annotations = nil + existingMS.Spec.Template.Spec.ReadinessGates = []clusterv1.MachineReadinessGate{{ConditionType: "bar"}} existingMS.Spec.Template.Spec.NodeDrainTimeout = duration5s existingMS.Spec.Template.Spec.NodeDeletionTimeout = duration5s existingMS.Spec.Template.Spec.NodeVolumeDetachTimeout = duration5s @@ -688,6 +690,7 @@ func TestComputeDesiredMachineSet(t *testing.T) { "ms-label-2": "ms-value-2", } existingMS.Spec.Template.Annotations = nil + existingMS.Spec.Template.Spec.ReadinessGates = []clusterv1.MachineReadinessGate{{ConditionType: "bar"}} existingMS.Spec.Template.Spec.NodeDrainTimeout = duration5s existingMS.Spec.Template.Spec.NodeDeletionTimeout = duration5s existingMS.Spec.Template.Spec.NodeVolumeDetachTimeout = duration5s @@ -741,6 +744,7 @@ func TestComputeDesiredMachineSet(t *testing.T) { "ms-label-2": "ms-value-2", } existingMS.Spec.Template.Annotations = nil + existingMS.Spec.Template.Spec.ReadinessGates = []clusterv1.MachineReadinessGate{{ConditionType: "bar"}} existingMS.Spec.Template.Spec.NodeDrainTimeout = duration5s existingMS.Spec.Template.Spec.NodeDeletionTimeout = duration5s existingMS.Spec.Template.Spec.NodeVolumeDetachTimeout = duration5s diff --git a/internal/controllers/machinedeployment/mdutil/util.go b/internal/controllers/machinedeployment/mdutil/util.go index eb6c1c6ce4a0..6080fd87cfb0 100644 --- a/internal/controllers/machinedeployment/mdutil/util.go +++ b/internal/controllers/machinedeployment/mdutil/util.go @@ -390,6 +390,7 @@ func MachineTemplateDeepCopyRolloutFields(template *clusterv1.MachineTemplateSpe templateCopy.Annotations = nil // Drop node timeout values + templateCopy.Spec.ReadinessGates = nil templateCopy.Spec.NodeDrainTimeout = nil templateCopy.Spec.NodeDeletionTimeout = nil templateCopy.Spec.NodeVolumeDetachTimeout = nil diff --git a/internal/controllers/machinedeployment/mdutil/util_test.go b/internal/controllers/machinedeployment/mdutil/util_test.go index 375302cf6314..e178c47e35af 100644 --- a/internal/controllers/machinedeployment/mdutil/util_test.go +++ b/internal/controllers/machinedeployment/mdutil/util_test.go @@ -216,6 +216,7 @@ func TestEqualMachineTemplate(t *testing.T) { machineTemplateWithDifferentAnnotations.Annotations = map[string]string{"a2": "v2"} machineTemplateWithDifferentInPlaceMutableSpecFields := machineTemplate.DeepCopy() + machineTemplateWithDifferentInPlaceMutableSpecFields.Spec.ReadinessGates = []clusterv1.MachineReadinessGate{{ConditionType: "foo"}} machineTemplateWithDifferentInPlaceMutableSpecFields.Spec.NodeDrainTimeout = &metav1.Duration{Duration: 20 * time.Second} machineTemplateWithDifferentInPlaceMutableSpecFields.Spec.NodeDeletionTimeout = &metav1.Duration{Duration: 20 * time.Second} machineTemplateWithDifferentInPlaceMutableSpecFields.Spec.NodeVolumeDetachTimeout = &metav1.Duration{Duration: 20 * time.Second} @@ -302,7 +303,7 @@ func TestEqualMachineTemplate(t *testing.T) { + ClusterName: "cluster2", Bootstrap: {ConfigRef: &{Kind: "BootstrapConfig", Namespace: "default", Name: "bootstrap1", APIVersion: "bootstrap.cluster.x-k8s.io", ...}}, InfrastructureRef: {Kind: "InfrastructureMachine", Namespace: "default", Name: "infra1", APIVersion: "infrastructure.cluster.x-k8s.io", ...}, - ... // 6 identical fields + ... // 7 identical fields }, }`, Diff2: `&v1beta1.MachineTemplateSpec{ @@ -312,7 +313,7 @@ func TestEqualMachineTemplate(t *testing.T) { + ClusterName: "cluster1", Bootstrap: {ConfigRef: &{Kind: "BootstrapConfig", Namespace: "default", Name: "bootstrap1", APIVersion: "bootstrap.cluster.x-k8s.io", ...}}, InfrastructureRef: {Kind: "InfrastructureMachine", Namespace: "default", Name: "infra1", APIVersion: "infrastructure.cluster.x-k8s.io", ...}, - ... // 6 identical fields + ... // 7 identical fields }, }`, }, @@ -331,7 +332,7 @@ func TestEqualMachineTemplate(t *testing.T) { + Version: &"v1.26.0", ProviderID: nil, FailureDomain: &"failure-domain1", - ... // 3 identical fields + ... // 4 identical fields }, }`, Diff2: `&v1beta1.MachineTemplateSpec{ @@ -344,7 +345,7 @@ func TestEqualMachineTemplate(t *testing.T) { + Version: &"v1.25.0", ProviderID: nil, FailureDomain: &"failure-domain1", - ... // 3 identical fields + ... // 4 identical fields }, }`, }, @@ -357,26 +358,26 @@ func TestEqualMachineTemplate(t *testing.T) { ObjectMeta: {}, Spec: v1beta1.MachineSpec{ ... // 3 identical fields - Version: &"v1.25.0", - ProviderID: nil, -- FailureDomain: &"failure-domain1", -+ FailureDomain: &"failure-domain2", - NodeDrainTimeout: nil, - NodeVolumeDetachTimeout: nil, - NodeDeletionTimeout: nil, + Version: &"v1.25.0", + ProviderID: nil, +- FailureDomain: &"failure-domain1", ++ FailureDomain: &"failure-domain2", + ReadinessGates: nil, + NodeDrainTimeout: nil, + ... // 2 identical fields }, }`, Diff2: `&v1beta1.MachineTemplateSpec{ ObjectMeta: {}, Spec: v1beta1.MachineSpec{ ... // 3 identical fields - Version: &"v1.25.0", - ProviderID: nil, -- FailureDomain: &"failure-domain2", -+ FailureDomain: &"failure-domain1", - NodeDrainTimeout: nil, - NodeVolumeDetachTimeout: nil, - NodeDeletionTimeout: nil, + Version: &"v1.25.0", + ProviderID: nil, +- FailureDomain: &"failure-domain2", ++ FailureDomain: &"failure-domain1", + ReadinessGates: nil, + NodeDrainTimeout: nil, + ... // 2 identical fields }, }`, }, @@ -401,7 +402,7 @@ func TestEqualMachineTemplate(t *testing.T) { }, Version: &"v1.25.0", ProviderID: nil, - ... // 4 identical fields + ... // 5 identical fields }, }`, Diff2: `&v1beta1.MachineTemplateSpec{ @@ -420,7 +421,7 @@ func TestEqualMachineTemplate(t *testing.T) { }, Version: &"v1.25.0", ProviderID: nil, - ... // 4 identical fields + ... // 5 identical fields }, }`, }, @@ -441,7 +442,7 @@ func TestEqualMachineTemplate(t *testing.T) { }, InfrastructureRef: {Kind: "InfrastructureMachine", Namespace: "default", Name: "infra1", APIVersion: "infrastructure.cluster.x-k8s.io", ...}, Version: &"v1.25.0", - ... // 5 identical fields + ... // 6 identical fields }, }`, Diff2: `&v1beta1.MachineTemplateSpec{ @@ -456,7 +457,7 @@ func TestEqualMachineTemplate(t *testing.T) { }, InfrastructureRef: {Kind: "InfrastructureMachine", Namespace: "default", Name: "infra1", APIVersion: "infrastructure.cluster.x-k8s.io", ...}, Version: &"v1.25.0", - ... // 5 identical fields + ... // 6 identical fields }, }`, }, @@ -483,7 +484,7 @@ func TestEqualMachineTemplate(t *testing.T) { }, InfrastructureRef: {Kind: "InfrastructureMachine", Namespace: "default", Name: "infra1", APIVersion: "infrastructure.cluster.x-k8s.io", ...}, Version: &"v1.25.0", - ... // 5 identical fields + ... // 6 identical fields }, }`, Diff2: `&v1beta1.MachineTemplateSpec{ @@ -504,7 +505,7 @@ func TestEqualMachineTemplate(t *testing.T) { }, InfrastructureRef: {Kind: "InfrastructureMachine", Namespace: "default", Name: "infra1", APIVersion: "infrastructure.cluster.x-k8s.io", ...}, Version: &"v1.25.0", - ... // 5 identical fields + ... // 6 identical fields }, }`, }, @@ -627,7 +628,7 @@ func TestFindNewMachineSet(t *testing.T) { }, Version: nil, ProviderID: nil, - ... // 4 identical fields + ... // 5 identical fields }, }`, oldMS.Name), }, diff --git a/internal/controllers/machineset/machineset_controller.go b/internal/controllers/machineset/machineset_controller.go index 45ec61b303ed..8497bc3442c4 100644 --- a/internal/controllers/machineset/machineset_controller.go +++ b/internal/controllers/machineset/machineset_controller.go @@ -424,6 +424,7 @@ func (r *Reconciler) syncMachines(ctx context.Context, machineSet *clusterv1.Mac } // Set all other in-place mutable fields that impact the ability to tear down existing machines. + m.Spec.ReadinessGates = machineSet.Spec.Template.Spec.ReadinessGates m.Spec.NodeDrainTimeout = machineSet.Spec.Template.Spec.NodeDrainTimeout m.Spec.NodeDeletionTimeout = machineSet.Spec.Template.Spec.NodeDeletionTimeout m.Spec.NodeVolumeDetachTimeout = machineSet.Spec.Template.Spec.NodeVolumeDetachTimeout diff --git a/internal/controllers/machineset/machineset_controller_test.go b/internal/controllers/machineset/machineset_controller_test.go index c9791cf31da1..449d09a0e954 100644 --- a/internal/controllers/machineset/machineset_controller_test.go +++ b/internal/controllers/machineset/machineset_controller_test.go @@ -1341,6 +1341,8 @@ func TestMachineSetReconciler_syncMachines(t *testing.T) { "modified-annotation": "modified-value-2", // Modify the value of the annotation // Drop "dropped-annotation" } + readinessGates := []clusterv1.MachineReadinessGate{{ConditionType: "foo"}} + ms.Spec.Template.Spec.ReadinessGates = readinessGates ms.Spec.Template.Spec.NodeDrainTimeout = duration10s ms.Spec.Template.Spec.NodeDeletionTimeout = duration10s ms.Spec.Template.Spec.NodeVolumeDetachTimeout = duration10s @@ -1367,6 +1369,8 @@ func TestMachineSetReconciler_syncMachines(t *testing.T) { Not(BeNil()), HaveValue(Equal(*ms.Spec.Template.Spec.NodeVolumeDetachTimeout)), )) + // Verify readiness gates. + g.Expect(updatedInPlaceMutatingMachine.Spec.ReadinessGates).Should(Equal(readinessGates)) }, timeout).Should(Succeed()) // Verify in-place mutable fields are updated on InfrastructureMachine