Skip to content

🐛 Fix race conditions ScaleDownOldMS #12812

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
wants to merge 6 commits into
base: main
Choose a base branch
from
+1,442 −333
Open

🐛 Fix race conditions ScaleDownOldMS #12812

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d16533a
Fix ScaleDownOldMS
fabriziopandini Oct 1, 2025
142d1de
Minor improvements
fabriziopandini Oct 4, 2025
aca5de4
Address feedback
fabriziopandini Oct 6, 2025
7e8ac3a
Fix review findings
sbueringer Oct 7, 2025
26e82c4
Merge pull request #314 from sbueringer/fix-scaleDownOldMS-fixups
fabriziopandini Oct 7, 2025
40f23d9
More comments
fabriziopandini Oct 7, 2025
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
286 changes: 134 additions & 152 deletions internal/controllers/machinedeployment/machinedeployment_rolling.go
View file
Open in desktop

Large diffs are not rendered by default.

516 changes: 390 additions & 126 deletions internal/controllers/machinedeployment/machinedeployment_rolling_test.go
View file
Open in desktop

Large diffs are not rendered by default.

35 changes: 20 additions & 15 deletions internal/controllers/machinedeployment/machinedeployment_rollout_sequence_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"time"

"github.com/google/go-cmp/cmp"
"github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
Expand Down Expand Up @@ -107,9 +108,10 @@ type rolloutSequenceTestCase struct {
seed int64
}

func Test_rolloutSequencesWithPredictableReconcileOrder(t *testing.T) {
func Test_rolloutRollingSequences(t *testing.T) {
ctx := context.Background()
ctx = ctrl.LoggerInto(ctx, klog.Background())
klog.SetOutput(ginkgo.GinkgoWriter)

tests := []rolloutSequenceTestCase{
// Regular rollout (no in-place)
Expand Down Expand Up @@ -249,9 +251,8 @@ func Test_rolloutSequencesWithPredictableReconcileOrder(t *testing.T) {
}

testWithPredictableReconcileOrder := true
// TODO(in-place): enable tests with random reconcile order as soon as the issues in reconcileOldMachineSets are fixed
testWithRandomReconcileOrderFromConstantSeed := false
testWithRandomReconcileOrderFromRandomSeed := false
testWithRandomReconcileOrderFromConstantSeed := true
testWithRandomReconcileOrderFromRandomSeed := true

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -260,9 +261,7 @@ func Test_rolloutSequencesWithPredictableReconcileOrder(t *testing.T) {
if testWithPredictableReconcileOrder {
tt.maxIterations = 50
tt.randomControllerOrder = false
if tt.logAndGoldenFileName == "" {
tt.logAndGoldenFileName = strings.ToLower(tt.name)
}
tt.logAndGoldenFileName = strings.ToLower(tt.name)
t.Run("default", func(t *testing.T) {
runTestCase(ctx, t, tt)
})
Expand All @@ -273,11 +272,7 @@ func Test_rolloutSequencesWithPredictableReconcileOrder(t *testing.T) {
tt.name = fmt.Sprintf("%s, random(0)", name)
tt.randomControllerOrder = true
tt.seed = 0
// TODO(in-place): drop the following line as soon as issue with scale down are fixed
tt.skipLogToFileAndGoldenFileCheck = true
if tt.logAndGoldenFileName == "" {
tt.logAndGoldenFileName = strings.ToLower(tt.name)
}
tt.logAndGoldenFileName = strings.ToLower(tt.name)
t.Run("random(0)", func(t *testing.T) {
runTestCase(ctx, t, tt)
})
Expand Down Expand Up @@ -305,7 +300,7 @@ func runTestCase(ctx context.Context, t *testing.T, tt rolloutSequenceTestCase)

rng := rand.New(rand.NewSource(tt.seed)) //nolint:gosec // it is ok to use a weak randomizer here
fLogger := newFileLogger(t, tt.name, fmt.Sprintf("testdata/%s", tt.logAndGoldenFileName))
// uncomment this line to automatically generate/update golden files: fLogger.writeGoldenFile = true
fLogger.writeGoldenFile = true

// Init current and desired state from test case
current := tt.currentScope.Clone()
Expand Down Expand Up @@ -888,8 +883,14 @@ func createMD(failureDomain string, replicas int32, maxSurge, maxUnavailable int
}
}

func createMS(name, failureDomain string, replicas int32) *clusterv1.MachineSet {
return &clusterv1.MachineSet{
func withStatusAvailableReplicas(r int32) fakeMachineSetOption {
return func(ms *clusterv1.MachineSet) {
ms.Status.AvailableReplicas = ptr.To(r)
}
}

func createMS(name, failureDomain string, replicas int32, opts ...fakeMachineSetOption) *clusterv1.MachineSet {
ms := &clusterv1.MachineSet{
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
Expand All @@ -903,6 +904,10 @@ func createMS(name, failureDomain string, replicas int32) *clusterv1.MachineSet
AvailableReplicas: ptr.To(replicas),
},
}
for _, opt := range opts {
opt(ms)
}
return ms
}

func createM(name, ownedByMS, failureDomain string) *clusterv1.Machine {
Expand Down
59 changes: 59 additions & 0 deletions ...ut, 12 replicas, maxsurge 3, maxunavailable 1, scale down to 6, random(0).test.log.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
## Regular rollout, 12 Replicas, maxSurge 3, maxUnavailable 1, scale down to 6, random(0)

[Test] Initial state
md, 6/6 replicas
- ms1, 9/9 replicas (m4,m5,m6,m7,m8,m9,m10,m11,m12)
- ms2, 3/3 replicas (m13,m14,m15)
[Test] Rollout 12 replicas, MaxSurge=3, MaxUnavailable=1, random(0)
[MS controller] Iteration 1, Reconcile ms2, 3/3 replicas (m13,m14,m15)
[MS controller] Iteration 1, Reconcile ms1, 9/9 replicas (m4,m5,m6,m7,m8,m9,m10,m11,m12)
[MD controller] Iteration 1, Reconcile md
[MD controller] - Input to rollout planner
md, 6/6 replicas
- ms1, 9/9 replicas (m4,m5,m6,m7,m8,m9,m10,m11,m12)
- ms2, 3/3 replicas (m13,m14,m15)
[MD controller] - Result of rollout planner
md, 12/6 replicas
- ms1, 9/2 replicas (m4,m5,m6,m7,m8,m9,m10,m11,m12)
- ms2, 3/3 replicas (m13,m14,m15)
[Toleration] tolerate maxSurge breach
[MS controller] Iteration 2, Reconcile ms1, 9/2 replicas (m4,m5,m6,m7,m8,m9,m10,m11,m12)
[MS controller] - ms1 scale down to 2/2 replicas (m4,m5,m6,m7,m8,m9,m10 deleted)
[MS controller] Iteration 2, Reconcile ms1, 2/2 replicas (m11,m12)
[MD controller] Iteration 3, Reconcile md
[MD controller] - Input to rollout planner
md, 12/6 replicas
- ms1, 2/2 replicas (m11,m12)
- ms2, 3/3 replicas (m13,m14,m15)
[MD controller] - Result of rollout planner
md, 5/6 replicas
- ms1, 2/2 replicas (m11,m12)
- ms2, 3/6 replicas (m13,m14,m15)
[MS controller] Iteration 3, Reconcile ms2, 3/6 replicas (m13,m14,m15)
[MS controller] - ms2 scale up to 6/6 replicas (m16,m17,m18 created)
[MD controller] Iteration 4, Reconcile md
[MD controller] - Input to rollout planner
md, 5/6 replicas
- ms1, 2/2 replicas (m11,m12)
- ms2, 6/6 replicas (m13,m14,m15,m16,m17,m18)
[MD controller] - Result of rollout planner
md, 8/6 replicas
- ms1, 2/0 replicas (m11,m12)
- ms2, 6/6 replicas (m13,m14,m15,m16,m17,m18)
[MS controller] Iteration 4, Reconcile ms1, 2/0 replicas (m11,m12)
[MS controller] - ms1 scale down to 0/0 replicas (m11,m12 deleted)
[MS controller] Iteration 4, Reconcile ms2, 6/6 replicas (m13,m14,m15,m16,m17,m18)
[MS controller] Iteration 5, Reconcile ms2, 6/6 replicas (m13,m14,m15,m16,m17,m18)
[MD controller] Iteration 5, Reconcile md
[MD controller] - Input to rollout planner
md, 8/6 replicas
- ms1, 0/0 replicas ()
- ms2, 6/6 replicas (m13,m14,m15,m16,m17,m18)
[MD controller] - Result of rollout planner
md, 6/6 replicas
- ms1, 0/0 replicas ()
- ms2, 6/6 replicas (m13,m14,m15,m16,m17,m18)
[Test] Final state
md, 6/6 replicas
- ms1, 0/0 replicas ()
- ms2, 6/6 replicas (m13,m14,m15,m16,m17,m18)
105 changes: 105 additions & 0 deletions ...data/regular rollout, 3 replicas, maxsurge 0, maxunavailable 1, random(0).test.log.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
## Regular rollout, 3 Replicas, maxSurge 0, maxUnavailable 1, random(0)

[Test] Initial state
md, 3/3 replicas
- ms1, 3/3 replicas (m1,m2,m3)
- ms2, 0/0 replicas ()
[Test] Rollout 3 replicas, MaxSurge=0, MaxUnavailable=1, random(0)
[MS controller] Iteration 1, Reconcile ms2, 0/0 replicas ()
[MS controller] Iteration 1, Reconcile ms1, 3/3 replicas (m1,m2,m3)
[MD controller] Iteration 1, Reconcile md
[MD controller] - Input to rollout planner
md, 3/3 replicas
- ms1, 3/3 replicas (m1,m2,m3)
- ms2, 0/0 replicas ()
[MD controller] - Result of rollout planner
md, 3/3 replicas
- ms1, 3/2 replicas (m1,m2,m3)
- ms2, 0/0 replicas ()
[MS controller] Iteration 2, Reconcile ms1, 3/2 replicas (m1,m2,m3)
[MS controller] - ms1 scale down to 2/2 replicas (m1 deleted)
[MS controller] Iteration 2, Reconcile ms1, 2/2 replicas (m2,m3)
[MD controller] Iteration 3, Reconcile md
[MD controller] - Input to rollout planner
md, 3/3 replicas
- ms1, 2/2 replicas (m2,m3)
- ms2, 0/0 replicas ()
[MD controller] - Result of rollout planner
md, 2/3 replicas
- ms1, 2/2 replicas (m2,m3)
- ms2, 0/1 replicas ()
[MS controller] Iteration 3, Reconcile ms2, 0/1 replicas ()
[MS controller] - ms2 scale up to 1/1 replicas (m4 created)
[MD controller] Iteration 4, Reconcile md
[MD controller] - Input to rollout planner
md, 2/3 replicas
- ms1, 2/2 replicas (m2,m3)
- ms2, 1/1 replicas (m4)
[MD controller] - Result of rollout planner
md, 3/3 replicas
- ms1, 2/1 replicas (m2,m3)
- ms2, 1/1 replicas (m4)
[MS controller] Iteration 4, Reconcile ms1, 2/1 replicas (m2,m3)
[MS controller] - ms1 scale down to 1/1 replicas (m2 deleted)
[MS controller] Iteration 4, Reconcile ms2, 1/1 replicas (m4)
[MS controller] Iteration 5, Reconcile ms2, 1/1 replicas (m4)
[MD controller] Iteration 5, Reconcile md
[MD controller] - Input to rollout planner
md, 3/3 replicas
- ms1, 1/1 replicas (m3)
- ms2, 1/1 replicas (m4)
[MD controller] - Result of rollout planner
md, 2/3 replicas
- ms1, 1/1 replicas (m3)
- ms2, 1/2 replicas (m4)
[MS controller] Iteration 6, Reconcile ms2, 1/2 replicas (m4)
[MS controller] - ms2 scale up to 2/2 replicas (m5 created)
[MD controller] Iteration 7, Reconcile md
[MD controller] - Input to rollout planner
md, 2/3 replicas
- ms1, 1/1 replicas (m3)
- ms2, 2/2 replicas (m4,m5)
[MD controller] - Result of rollout planner
md, 3/3 replicas
- ms1, 1/0 replicas (m3)
- ms2, 2/2 replicas (m4,m5)
[MS controller] Iteration 7, Reconcile ms2, 2/2 replicas (m4,m5)
[MD controller] Iteration 8, Reconcile md
[MD controller] - Input to rollout planner
md, 3/3 replicas
- ms1, 1/0 replicas (m3)
- ms2, 2/2 replicas (m4,m5)
[MD controller] - Result of rollout planner
md, 3/3 replicas
- ms1, 1/0 replicas (m3)
- ms2, 2/2 replicas (m4,m5)
[MS controller] Iteration 8, Reconcile ms1, 1/0 replicas (m3)
[MS controller] - ms1 scale down to 0/0 replicas (m3 deleted)
[MS controller] Iteration 9, Reconcile ms2, 2/2 replicas (m4,m5)
[MS controller] Iteration 9, Reconcile ms1, 0/0 replicas ()
[MD controller] Iteration 9, Reconcile md
[MD controller] - Input to rollout planner
md, 3/3 replicas
- ms1, 0/0 replicas ()
- ms2, 2/2 replicas (m4,m5)
[MD controller] - Result of rollout planner
md, 2/3 replicas
- ms1, 0/0 replicas ()
- ms2, 2/3 replicas (m4,m5)
[MS controller] Iteration 10, Reconcile ms2, 2/3 replicas (m4,m5)
[MS controller] - ms2 scale up to 3/3 replicas (m6 created)
[MS controller] Iteration 10, Reconcile ms1, 0/0 replicas ()
[MS controller] Iteration 11, Reconcile ms2, 3/3 replicas (m4,m5,m6)
[MD controller] Iteration 11, Reconcile md
[MD controller] - Input to rollout planner
md, 2/3 replicas
- ms1, 0/0 replicas ()
- ms2, 3/3 replicas (m4,m5,m6)
[MD controller] - Result of rollout planner
md, 3/3 replicas
- ms1, 0/0 replicas ()
- ms2, 3/3 replicas (m4,m5,m6)
[Test] Final state
md, 3/3 replicas
- ms1, 0/0 replicas ()
- ms2, 3/3 replicas (m4,m5,m6)
136 changes: 136 additions & 0 deletions ...data/regular rollout, 3 replicas, maxsurge 1, maxunavailable 0, random(0).test.log.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
## Regular rollout, 3 Replicas, maxSurge 1, maxUnavailable 0, random(0)

[Test] Initial state
md, 3/3 replicas
- ms1, 3/3 replicas (m1,m2,m3)
- ms2, 0/0 replicas ()
[Test] Rollout 3 replicas, MaxSurge=1, MaxUnavailable=0, random(0)
[MS controller] Iteration 1, Reconcile ms2, 0/0 replicas ()
[MS controller] Iteration 1, Reconcile ms1, 3/3 replicas (m1,m2,m3)
[MD controller] Iteration 1, Reconcile md
[MD controller] - Input to rollout planner
md, 3/3 replicas
- ms1, 3/3 replicas (m1,m2,m3)
- ms2, 0/0 replicas ()
[MD controller] - Result of rollout planner
md, 3/3 replicas
- ms1, 3/3 replicas (m1,m2,m3)
- ms2, 0/1 replicas ()
[MS controller] Iteration 2, Reconcile ms1, 3/3 replicas (m1,m2,m3)
[MS controller] Iteration 2, Reconcile ms1, 3/3 replicas (m1,m2,m3)
[MD controller] Iteration 3, Reconcile md
[MD controller] - Input to rollout planner
md, 3/3 replicas
- ms1, 3/3 replicas (m1,m2,m3)
- ms2, 0/1 replicas ()
[MD controller] - Result of rollout planner
md, 3/3 replicas
- ms1, 3/3 replicas (m1,m2,m3)
- ms2, 0/1 replicas ()
[MS controller] Iteration 3, Reconcile ms2, 0/1 replicas ()
[MS controller] - ms2 scale up to 1/1 replicas (m4 created)
[MD controller] Iteration 4, Reconcile md
[MD controller] - Input to rollout planner
md, 3/3 replicas
- ms1, 3/3 replicas (m1,m2,m3)
- ms2, 1/1 replicas (m4)
[MD controller] - Result of rollout planner
md, 4/3 replicas
- ms1, 3/2 replicas (m1,m2,m3)
- ms2, 1/1 replicas (m4)
[MS controller] Iteration 4, Reconcile ms1, 3/2 replicas (m1,m2,m3)
[MS controller] - ms1 scale down to 2/2 replicas (m1 deleted)
[MS controller] Iteration 4, Reconcile ms2, 1/1 replicas (m4)
[MS controller] Iteration 5, Reconcile ms2, 1/1 replicas (m4)
[MD controller] Iteration 5, Reconcile md
[MD controller] - Input to rollout planner
md, 4/3 replicas
- ms1, 2/2 replicas (m2,m3)
- ms2, 1/1 replicas (m4)
[MD controller] - Result of rollout planner
md, 3/3 replicas
- ms1, 2/2 replicas (m2,m3)
- ms2, 1/2 replicas (m4)
[MS controller] Iteration 6, Reconcile ms2, 1/2 replicas (m4)
[MS controller] - ms2 scale up to 2/2 replicas (m5 created)
[MD controller] Iteration 7, Reconcile md
[MD controller] - Input to rollout planner
md, 3/3 replicas
- ms1, 2/2 replicas (m2,m3)
- ms2, 2/2 replicas (m4,m5)
[MD controller] - Result of rollout planner
md, 4/3 replicas
- ms1, 2/1 replicas (m2,m3)
- ms2, 2/2 replicas (m4,m5)
[MS controller] Iteration 7, Reconcile ms2, 2/2 replicas (m4,m5)
[MD controller] Iteration 8, Reconcile md
[MD controller] - Input to rollout planner
md, 4/3 replicas
- ms1, 2/1 replicas (m2,m3)
- ms2, 2/2 replicas (m4,m5)
[MD controller] - Result of rollout planner
md, 4/3 replicas
- ms1, 2/1 replicas (m2,m3)
- ms2, 2/2 replicas (m4,m5)
[MS controller] Iteration 8, Reconcile ms1, 2/1 replicas (m2,m3)
[MS controller] - ms1 scale down to 1/1 replicas (m2 deleted)
[MS controller] Iteration 9, Reconcile ms2, 2/2 replicas (m4,m5)
[MS controller] Iteration 9, Reconcile ms1, 1/1 replicas (m3)
[MD controller] Iteration 9, Reconcile md
[MD controller] - Input to rollout planner
md, 4/3 replicas
- ms1, 1/1 replicas (m3)
- ms2, 2/2 replicas (m4,m5)
[MD controller] - Result of rollout planner
md, 3/3 replicas
- ms1, 1/1 replicas (m3)
- ms2, 2/3 replicas (m4,m5)
[MS controller] Iteration 10, Reconcile ms2, 2/3 replicas (m4,m5)
[MS controller] - ms2 scale up to 3/3 replicas (m6 created)
[MS controller] Iteration 10, Reconcile ms1, 1/1 replicas (m3)
[MS controller] Iteration 11, Reconcile ms2, 3/3 replicas (m4,m5,m6)
[MD controller] Iteration 11, Reconcile md
[MD controller] - Input to rollout planner
md, 3/3 replicas
- ms1, 1/1 replicas (m3)
- ms2, 3/3 replicas (m4,m5,m6)
[MD controller] - Result of rollout planner
md, 4/3 replicas
- ms1, 1/0 replicas (m3)
- ms2, 3/3 replicas (m4,m5,m6)
[MD controller] Iteration 12, Reconcile md
[MD controller] - Input to rollout planner
md, 4/3 replicas
- ms1, 1/0 replicas (m3)
- ms2, 3/3 replicas (m4,m5,m6)
[MD controller] - Result of rollout planner
md, 4/3 replicas
- ms1, 1/0 replicas (m3)
- ms2, 3/3 replicas (m4,m5,m6)
[MD controller] Iteration 14, Reconcile md
[MD controller] - Input to rollout planner
md, 4/3 replicas
- ms1, 1/0 replicas (m3)
- ms2, 3/3 replicas (m4,m5,m6)
[MD controller] - Result of rollout planner
md, 4/3 replicas
- ms1, 1/0 replicas (m3)
- ms2, 3/3 replicas (m4,m5,m6)
[MS controller] Iteration 15, Reconcile ms2, 3/3 replicas (m4,m5,m6)
[MS controller] Iteration 16, Reconcile ms2, 3/3 replicas (m4,m5,m6)
[MS controller] Iteration 16, Reconcile ms2, 3/3 replicas (m4,m5,m6)
[MS controller] Iteration 16, Reconcile ms1, 1/0 replicas (m3)
[MS controller] - ms1 scale down to 0/0 replicas (m3 deleted)
[MD controller] Iteration 16, Reconcile md
[MD controller] - Input to rollout planner
md, 4/3 replicas
- ms1, 0/0 replicas ()
- ms2, 3/3 replicas (m4,m5,m6)
[MD controller] - Result of rollout planner
md, 3/3 replicas
- ms1, 0/0 replicas ()
- ms2, 3/3 replicas (m4,m5,m6)
[Test] Final state
md, 3/3 replicas
- ms1, 0/0 replicas ()
- ms2, 3/3 replicas (m4,m5,m6)
Loading