From 0993264082453acdc66d0961d4231967e49744f2 Mon Sep 17 00:00:00 2001 From: Gagan163264 Date: Fri, 28 Aug 2026 18:08:57 +0530 Subject: [PATCH 1/6] Retry reconcile to wait for cache sync Signed-off-by: Gagan163264 --- pkg/controller/deployment_test.go | 52 ++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/pkg/controller/deployment_test.go b/pkg/controller/deployment_test.go index 9bc8e05b69..1d1feb2ffa 100644 --- a/pkg/controller/deployment_test.go +++ b/pkg/controller/deployment_test.go @@ -1751,17 +1751,15 @@ var _ = Describe("machineDeployment", func() { defer trackers.Stop() waitForCacheSync(stop, c) Key := testNamespace + "/" + testMachineDeployment.Name - Expect(c.reconcileClusterMachineDeployment(Key)).NotTo(HaveOccurred()) - waitForCacheSync(stop, c) - actualMachineDeployment, _ := c.controlMachineClient.MachineDeployments(testNamespace).Get(context.Background(), testMachineDeployment.Name, metav1.GetOptions{}) - waitForCacheSync(stop, c) - actualMachineSets, _ := c.controlMachineClient.MachineSets(testNamespace).List(context.Background(), metav1.ListOptions{}) - waitForCacheSync(stop, c) - actualMachines, _ := c.controlMachineClient.Machines(testNamespace).List(context.Background(), metav1.ListOptions{}) - testNode, _ := c.targetCoreClient.CoreV1().Nodes().Get(context.Background(), testNode.Name, metav1.GetOptions{}) - - Expect(postCheck(actualMachineDeployment, actualMachineSets.Items, actualMachines.Items, testNode)).To(BeNil()) + Eventually(func() error { + _ = c.reconcileClusterMachineDeployment(Key) + actualMachineDeployment, _ := c.controlMachineClient.MachineDeployments(testNamespace).Get(context.Background(), testMachineDeployment.Name, metav1.GetOptions{}) + actualMachineSets, _ := c.controlMachineClient.MachineSets(testNamespace).List(context.Background(), metav1.ListOptions{}) + actualMachines, _ := c.controlMachineClient.Machines(testNamespace).List(context.Background(), metav1.ListOptions{}) + actualNode, _ := c.targetCoreClient.CoreV1().Nodes().Get(context.Background(), testNode.Name, metav1.GetOptions{}) + return postCheck(actualMachineDeployment, actualMachineSets.Items, actualMachines.Items, actualNode) + }).WithTimeout(500 * time.Millisecond).WithPolling(100 * time.Millisecond).Should(Succeed()) }, Entry("reconcile the machineDeployment and return nil", func(_ *machinev1.MachineDeployment, _ *machinev1.MachineSet) {}, @@ -1966,13 +1964,24 @@ var _ = Describe("machineDeployment", func() { return nil }, ), - // flaky test because of reusing same testMachine for mutilple tests. Entry("should set MachinePriority=1 for the machines named in TriggerDeletionByMCM annotation in the MachineDeployment", func(testMachineDeployment *machinev1.MachineDeployment, _ *machinev1.MachineSet) { testMachineDeployment.Annotations[machineutils.TriggerDeletionByMCM] = fmt.Sprintf("%s~%s", testMachine.Name, time.Now().Format(time.RFC3339)) }, func(_ *machinev1.MachineDeployment, _ []machinev1.MachineSet, machines []machinev1.Machine, _ *corev1.Node) error { - Expect(machines[0].Annotations[machineutils.MachinePriority]).To(Equal("1")) + var targetMachine *machinev1.Machine + for i := range machines { + if machines[i].Name == "Machine-test" { + targetMachine = &machines[i] + break + } + } + if targetMachine == nil { + return errors.New("machine \"Machine-test\" not found") + } + if targetMachine.Annotations[machineutils.MachinePriority] != "1" { + return errors.New("expected MachinePriority=1 on machine \"Machine-test\"") + } return nil }, ), @@ -1982,9 +1991,22 @@ var _ = Describe("machineDeployment", func() { testMachineDeployment.Annotations[machineutils.TriggerDeletionByMCM] = fmt.Sprintf("%s~%s", testMachine.Name, time.Now().Format(time.RFC3339)) }, func(_ *machinev1.MachineDeployment, mcs []machinev1.MachineSet, _ []machinev1.Machine, _ *corev1.Node) error { - Expect(mcs[0].Annotations[machineutils.LastDeploymentReplicaChangeByScalerTime]).To(Equal(ts)) - _, exists := mcs[0].Annotations[machineutils.TriggerDeletionByMCM] - Expect(exists).To(BeFalse()) + var ms *machinev1.MachineSet + for i := range mcs { + if mcs[i].Name == "MachineSet-test" { + ms = &mcs[i] + break + } + } + if ms == nil { + return errors.New("machineSet \"MachineSet-test\" not found") + } + if ms.Annotations[machineutils.LastDeploymentReplicaChangeByScalerTime] != ts { + return errors.New("expected LastDeploymentReplicaChangeByScalerTime annotation to be preserved on the machineSet") + } + if _, exists := ms.Annotations[machineutils.TriggerDeletionByMCM]; exists { + return errors.New("TriggerDeletionByMCM annotation should not be set on the machineSet") + } return nil }, ), From 14df2147809c8f67cf16728921f3fef4dfe31549 Mon Sep 17 00:00:00 2001 From: Gagan163264 Date: Mon, 31 Aug 2026 21:27:48 +0530 Subject: [PATCH 2/6] use full UUID suffix and revert test table Signed-off-by: Gagan163264 --- pkg/controller/controller_utils.go | 6 +++++- pkg/controller/deployment_test.go | 27 ++++++++++++++++++--------- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/pkg/controller/controller_utils.go b/pkg/controller/controller_utils.go index c3d0716b15..c14f6b8d77 100644 --- a/pkg/controller/controller_utils.go +++ b/pkg/controller/controller_utils.go @@ -717,7 +717,11 @@ func GetFakeMachineFromTemplate(template *v1alpha1.MachineTemplateSpec, parentOb desiredAnnotations := getMachinesAnnotationSet(template, parentMetaObj) prefix := getMachinesPrefix(parentMetaObj.GetName()) - prefix = prefix + "-" + uuid.New().String()[:5] + // Use the full UUID as the suffix to avoid name collisions when many machines + // are created in a single reconcile (e.g. burstReplicas). Unlike the real + // apiserver, the fake client does not retry on GenerateName conflicts, so a + // truncated suffix can produce duplicate names and flake the test. + prefix = prefix + "-" + uuid.New().String() machine := &v1alpha1.Machine{ ObjectMeta: metav1.ObjectMeta{ Labels: desiredLabels, diff --git a/pkg/controller/deployment_test.go b/pkg/controller/deployment_test.go index 1d1feb2ffa..d4623f9ca1 100644 --- a/pkg/controller/deployment_test.go +++ b/pkg/controller/deployment_test.go @@ -12,6 +12,7 @@ import ( "time" "github.com/gardener/machine-controller-manager/pkg/util/annotations" + labelsutil "github.com/gardener/machine-controller-manager/pkg/util/labels" "github.com/gardener/machine-controller-manager/pkg/util/provider/machineutils" "k8s.io/utils/ptr" @@ -1751,15 +1752,17 @@ var _ = Describe("machineDeployment", func() { defer trackers.Stop() waitForCacheSync(stop, c) Key := testNamespace + "/" + testMachineDeployment.Name + Expect(c.reconcileClusterMachineDeployment(Key)).NotTo(HaveOccurred()) - Eventually(func() error { - _ = c.reconcileClusterMachineDeployment(Key) - actualMachineDeployment, _ := c.controlMachineClient.MachineDeployments(testNamespace).Get(context.Background(), testMachineDeployment.Name, metav1.GetOptions{}) - actualMachineSets, _ := c.controlMachineClient.MachineSets(testNamespace).List(context.Background(), metav1.ListOptions{}) - actualMachines, _ := c.controlMachineClient.Machines(testNamespace).List(context.Background(), metav1.ListOptions{}) - actualNode, _ := c.targetCoreClient.CoreV1().Nodes().Get(context.Background(), testNode.Name, metav1.GetOptions{}) - return postCheck(actualMachineDeployment, actualMachineSets.Items, actualMachines.Items, actualNode) - }).WithTimeout(500 * time.Millisecond).WithPolling(100 * time.Millisecond).Should(Succeed()) + waitForCacheSync(stop, c) + actualMachineDeployment, _ := c.controlMachineClient.MachineDeployments(testNamespace).Get(context.Background(), testMachineDeployment.Name, metav1.GetOptions{}) + waitForCacheSync(stop, c) + actualMachineSets, _ := c.controlMachineClient.MachineSets(testNamespace).List(context.Background(), metav1.ListOptions{}) + waitForCacheSync(stop, c) + actualMachines, _ := c.controlMachineClient.Machines(testNamespace).List(context.Background(), metav1.ListOptions{}) + testNode, _ := c.targetCoreClient.CoreV1().Nodes().Get(context.Background(), testNode.Name, metav1.GetOptions{}) + + Expect(postCheck(actualMachineDeployment, actualMachineSets.Items, actualMachines.Items, testNode)).To(BeNil()) }, Entry("reconcile the machineDeployment and return nil", func(_ *machinev1.MachineDeployment, _ *machinev1.MachineSet) {}, @@ -1986,9 +1989,15 @@ var _ = Describe("machineDeployment", func() { }, ), Entry("set LDRCBST annotation on the machineSet and TriggerDeletionByMCM annotation is not set on the machineSet", - func(testMachineDeployment *machinev1.MachineDeployment, _ *machinev1.MachineSet) { + func(testMachineDeployment *machinev1.MachineDeployment, testMachineSet *machinev1.MachineSet) { testMachineDeployment.Annotations[machineutils.LastDeploymentReplicaChangeByScalerTime] = ts testMachineDeployment.Annotations[machineutils.TriggerDeletionByMCM] = fmt.Sprintf("%s~%s", testMachine.Name, time.Now().Format(time.RFC3339)) + + const hash = "testhash" + testMachineSet.Labels = labelsutil.CloneAndAddLabel(testMachineSet.Labels, machinev1.DefaultMachineDeploymentUniqueLabelKey, hash) + testMachineSet.Spec.Selector = labelsutil.CloneSelectorAndAddLabel(testMachineSet.Spec.Selector, machinev1.DefaultMachineDeploymentUniqueLabelKey, hash) + testMachineSet.Spec.Template.Labels = labelsutil.CloneAndAddLabel(testMachineSet.Spec.Template.Labels, machinev1.DefaultMachineDeploymentUniqueLabelKey, hash) + testMachine.Labels = labelsutil.CloneAndAddLabel(testMachine.Labels, machinev1.DefaultMachineDeploymentUniqueLabelKey, hash) }, func(_ *machinev1.MachineDeployment, mcs []machinev1.MachineSet, _ []machinev1.Machine, _ *corev1.Node) error { var ms *machinev1.MachineSet From 6e72c40133d3b5eaf4b321521e44227541b91eb7 Mon Sep 17 00:00:00 2001 From: Gagan163264 Date: Mon, 31 Aug 2026 21:29:07 +0530 Subject: [PATCH 3/6] Comment change Signed-off-by: Gagan163264 --- pkg/controller/controller_utils.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/controller/controller_utils.go b/pkg/controller/controller_utils.go index c14f6b8d77..661b71f96a 100644 --- a/pkg/controller/controller_utils.go +++ b/pkg/controller/controller_utils.go @@ -717,10 +717,7 @@ func GetFakeMachineFromTemplate(template *v1alpha1.MachineTemplateSpec, parentOb desiredAnnotations := getMachinesAnnotationSet(template, parentMetaObj) prefix := getMachinesPrefix(parentMetaObj.GetName()) - // Use the full UUID as the suffix to avoid name collisions when many machines - // are created in a single reconcile (e.g. burstReplicas). Unlike the real - // apiserver, the fake client does not retry on GenerateName conflicts, so a - // truncated suffix can produce duplicate names and flake the test. + // Use the full UUID as the suffix to avoid name collisions when many machines are created in a single reconcile. prefix = prefix + "-" + uuid.New().String() machine := &v1alpha1.Machine{ ObjectMeta: metav1.ObjectMeta{ From cec4b8600fe392a8b2bdfaed5760d8d26e77922a Mon Sep 17 00:00:00 2001 From: Gagan163264 Date: Mon, 31 Aug 2026 21:34:49 +0530 Subject: [PATCH 4/6] Reduce labels Signed-off-by: Gagan163264 --- pkg/controller/deployment_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/controller/deployment_test.go b/pkg/controller/deployment_test.go index d4623f9ca1..979f4b525a 100644 --- a/pkg/controller/deployment_test.go +++ b/pkg/controller/deployment_test.go @@ -1992,12 +1992,7 @@ var _ = Describe("machineDeployment", func() { func(testMachineDeployment *machinev1.MachineDeployment, testMachineSet *machinev1.MachineSet) { testMachineDeployment.Annotations[machineutils.LastDeploymentReplicaChangeByScalerTime] = ts testMachineDeployment.Annotations[machineutils.TriggerDeletionByMCM] = fmt.Sprintf("%s~%s", testMachine.Name, time.Now().Format(time.RFC3339)) - - const hash = "testhash" - testMachineSet.Labels = labelsutil.CloneAndAddLabel(testMachineSet.Labels, machinev1.DefaultMachineDeploymentUniqueLabelKey, hash) - testMachineSet.Spec.Selector = labelsutil.CloneSelectorAndAddLabel(testMachineSet.Spec.Selector, machinev1.DefaultMachineDeploymentUniqueLabelKey, hash) - testMachineSet.Spec.Template.Labels = labelsutil.CloneAndAddLabel(testMachineSet.Spec.Template.Labels, machinev1.DefaultMachineDeploymentUniqueLabelKey, hash) - testMachine.Labels = labelsutil.CloneAndAddLabel(testMachine.Labels, machinev1.DefaultMachineDeploymentUniqueLabelKey, hash) + testMachineSet.Spec.Selector = labelsutil.CloneSelectorAndAddLabel(testMachineSet.Spec.Selector, machinev1.DefaultMachineDeploymentUniqueLabelKey, "testhash") }, func(_ *machinev1.MachineDeployment, mcs []machinev1.MachineSet, _ []machinev1.Machine, _ *corev1.Node) error { var ms *machinev1.MachineSet From 0c8083b657c1f1a2a086adfb7e7863ed4524924a Mon Sep 17 00:00:00 2001 From: Gagan163264 Date: Tue, 1 Sep 2026 00:16:31 +0530 Subject: [PATCH 5/6] Copy label to priority 1 test Signed-off-by: Gagan163264 --- pkg/controller/deployment_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/controller/deployment_test.go b/pkg/controller/deployment_test.go index 979f4b525a..e011a9715c 100644 --- a/pkg/controller/deployment_test.go +++ b/pkg/controller/deployment_test.go @@ -1968,8 +1968,9 @@ var _ = Describe("machineDeployment", func() { }, ), Entry("should set MachinePriority=1 for the machines named in TriggerDeletionByMCM annotation in the MachineDeployment", - func(testMachineDeployment *machinev1.MachineDeployment, _ *machinev1.MachineSet) { + func(testMachineDeployment *machinev1.MachineDeployment, testMachineSet *machinev1.MachineSet) { testMachineDeployment.Annotations[machineutils.TriggerDeletionByMCM] = fmt.Sprintf("%s~%s", testMachine.Name, time.Now().Format(time.RFC3339)) + testMachineSet.Spec.Selector = labelsutil.CloneSelectorAndAddLabel(testMachineSet.Spec.Selector, machinev1.DefaultMachineDeploymentUniqueLabelKey, "testhash") }, func(_ *machinev1.MachineDeployment, _ []machinev1.MachineSet, machines []machinev1.Machine, _ *corev1.Node) error { var targetMachine *machinev1.Machine From d1187cafe4d9a0d866ca58743b210481fb40972f Mon Sep 17 00:00:00 2001 From: Gagan163264 Date: Sat, 19 Sep 2026 13:06:15 +0530 Subject: [PATCH 6/6] use slices.IndexFunc Signed-off-by: Gagan163264 --- pkg/controller/deployment_test.go | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/pkg/controller/deployment_test.go b/pkg/controller/deployment_test.go index e011a9715c..d6dfe4a8a3 100644 --- a/pkg/controller/deployment_test.go +++ b/pkg/controller/deployment_test.go @@ -8,6 +8,7 @@ import ( "context" "errors" "fmt" + "slices" "strings" "time" @@ -1973,17 +1974,13 @@ var _ = Describe("machineDeployment", func() { testMachineSet.Spec.Selector = labelsutil.CloneSelectorAndAddLabel(testMachineSet.Spec.Selector, machinev1.DefaultMachineDeploymentUniqueLabelKey, "testhash") }, func(_ *machinev1.MachineDeployment, _ []machinev1.MachineSet, machines []machinev1.Machine, _ *corev1.Node) error { - var targetMachine *machinev1.Machine - for i := range machines { - if machines[i].Name == "Machine-test" { - targetMachine = &machines[i] - break - } - } - if targetMachine == nil { + idx := slices.IndexFunc(machines, func(m machinev1.Machine) bool { + return m.Name == "Machine-test" + }) + if idx == -1 { return errors.New("machine \"Machine-test\" not found") } - if targetMachine.Annotations[machineutils.MachinePriority] != "1" { + if machines[idx].Annotations[machineutils.MachinePriority] != "1" { return errors.New("expected MachinePriority=1 on machine \"Machine-test\"") } return nil @@ -1996,16 +1993,13 @@ var _ = Describe("machineDeployment", func() { testMachineSet.Spec.Selector = labelsutil.CloneSelectorAndAddLabel(testMachineSet.Spec.Selector, machinev1.DefaultMachineDeploymentUniqueLabelKey, "testhash") }, func(_ *machinev1.MachineDeployment, mcs []machinev1.MachineSet, _ []machinev1.Machine, _ *corev1.Node) error { - var ms *machinev1.MachineSet - for i := range mcs { - if mcs[i].Name == "MachineSet-test" { - ms = &mcs[i] - break - } - } - if ms == nil { + idx := slices.IndexFunc(mcs, func(ms machinev1.MachineSet) bool { + return ms.Name == "MachineSet-test" + }) + if idx == -1 { return errors.New("machineSet \"MachineSet-test\" not found") } + ms := &mcs[idx] if ms.Annotations[machineutils.LastDeploymentReplicaChangeByScalerTime] != ts { return errors.New("expected LastDeploymentReplicaChangeByScalerTime annotation to be preserved on the machineSet") }