Skip to content

Fix flaky MachineDeployment tests on GHA #1143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gagan16k wants to merge 6 commits into
base: master
Choose a base branch
from
+30 −8
Open

Fix flaky MachineDeployment tests on GHA #1143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0993264
Retry reconcile to wait for cache sync
gagan16k Aug 28, 2026
14df214
use full UUID suffix and revert test table
gagan16k Aug 31, 2026
6e72c40
Comment change
gagan16k Aug 31, 2026
cec4b86
Reduce labels
gagan16k Aug 31, 2026
0c8083b
Copy label to priority 1 test
gagan16k Aug 31, 2026
d1187ca
use slices.IndexFunc
gagan16k Sep 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkg/controller/controller_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,8 @@ 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.
prefix = prefix + "-" + uuid.New().String()

@elankath elankath Sep 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debatable if this is really needed since we aren't doing some sort of burst test with millions of objects. This is a false hypothetical that is not the real issue - missing the selector.

@gagan16k gagan16k Sep 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When running the spec without these changes, it reports this error sometimes

------------------------------
• [FAILED] [0.103 seconds]
machineset #manageReplicas [It] should create new machines only equal to burstReplicas and should not return errors.
/Users/I765230/go/src/github.com/gagan16k/machine-controller-manager/pkg/controller/machineset_test.go:740

  [FAILED] Unexpected error:
      <*errors.StatusError | 0x1b18cb08a500>:
      machines.machine.sapcloud.io "MachineSet-test-ddb6d" already exists
      {
          ErrStatus: {
              TypeMeta: {Kind: "", APIVersion: ""},
              ListMeta: {
                  SelfLink: "",
                  ResourceVersion: "",
                  Continue: "",
                  RemainingItemCount: nil,
              },
              Status: "Failure",
              Message: "machines.machine.sapcloud.io \"MachineSet-test-ddb6d\" already exists",
              Reason: "AlreadyExists",
              Details: {
                  Name: "MachineSet-test-ddb6d",
                  Group: "machine.sapcloud.io",
                  Kind: "machines",
                  UID: "",
                  Causes: nil,
                  RetryAfterSeconds: 0,
              },
              Code: 409,
          },
      }
  occurred
  In [It] at: /Users/I765230/go/src/github.com/gagan16k/machine-controller-manager/pkg/controller/machineset_test.go:752 @ 09/19/26 12:54:33.898
------------------------------

This is what stress reports when focused on this test

1m30s: 23551 runs so far, 94 failures (0.40%), 64 active

Additionally, this change is only on GetFakeMachineFromTemplate(), which is only called by createMachines() belonging to FakeMachineControl

machine := &v1alpha1.Machine{
ObjectMeta: metav1.ObjectMeta{
Labels: desiredLabels,
Expand Down
35 changes: 28 additions & 7 deletions pkg/controller/deployment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import (
"context"
"errors"
"fmt"
"slices"
"strings"
"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"

Expand Down Expand Up @@ -1966,25 +1968,44 @@ 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) {
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 {
Expect(machines[0].Annotations[machineutils.MachinePriority]).To(Equal("1"))
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 machines[idx].Annotations[machineutils.MachinePriority] != "1" {
return errors.New("expected MachinePriority=1 on machine \"Machine-test\"")
}
return nil
},
),
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))
testMachineSet.Spec.Selector = labelsutil.CloneSelectorAndAddLabel(testMachineSet.Spec.Selector, machinev1.DefaultMachineDeploymentUniqueLabelKey, "testhash")
},
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())
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")
}
if _, exists := ms.Annotations[machineutils.TriggerDeletionByMCM]; exists {
return errors.New("TriggerDeletionByMCM annotation should not be set on the machineSet")
}
return nil
},
),
Expand Down
Loading