Skip to content

Commit 4fdf6cb

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #8063 from sbueringer/pr-cc-md-unavailable-replicas
🌱 ClusterClass: also consider MD unavailableReplicas for rollout
2 parents eae0775 + 9d20ebf commit 4fdf6cb

File tree

3 files changed

+124
-50
lines changed

3 files changed

+124
-50
lines changed

‎internal/controllers/topology/cluster/conditions_test.go

Lines changed: 96 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,11 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) {
199199
Object: builder.MachineDeployment("ns1", "md0-abc123").
200200
WithReplicas(2).
201201
WithStatus(clusterv1.MachineDeploymentStatus{
202-
Replicas: int32(1),
203-
UpdatedReplicas: int32(1),
204-
ReadyReplicas: int32(1),
205-
AvailableReplicas: int32(1),
202+
Replicas: int32(1),
203+
UpdatedReplicas: int32(1),
204+
ReadyReplicas: int32(1),
205+
AvailableReplicas: int32(1),
206+
UnavailableReplicas: int32(0),
206207
}).
207208
Build(),
208209
},
@@ -241,10 +242,11 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) {
241242
Object: builder.MachineDeployment("ns1", "md0-abc123").
242243
WithReplicas(2).
243244
WithStatus(clusterv1.MachineDeploymentStatus{
244-
Replicas: int32(2),
245-
UpdatedReplicas: int32(2),
246-
ReadyReplicas: int32(2),
247-
AvailableReplicas: int32(2),
245+
Replicas: int32(2),
246+
UpdatedReplicas: int32(2),
247+
ReadyReplicas: int32(2),
248+
AvailableReplicas: int32(2),
249+
UnavailableReplicas: int32(0),
248250
}).
249251
Build(),
250252
},
@@ -285,10 +287,11 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) {
285287
Object: builder.MachineDeployment("ns1", "md0-abc123").
286288
WithReplicas(2).
287289
WithStatus(clusterv1.MachineDeploymentStatus{
288-
Replicas: int32(2),
289-
UpdatedReplicas: int32(2),
290-
ReadyReplicas: int32(2),
291-
AvailableReplicas: int32(2),
290+
Replicas: int32(2),
291+
UpdatedReplicas: int32(2),
292+
ReadyReplicas: int32(2),
293+
AvailableReplicas: int32(2),
294+
UnavailableReplicas: int32(0),
292295
}).
293296
Build(),
294297
},
@@ -365,7 +368,7 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) {
365368
wantConditionStatus: corev1.ConditionTrue,
366369
},
367370
{
368-
name: "should set the condition to false is some machine deployments have not picked the new version because other machine deployments are rolling out",
371+
name: "should set the condition to false is some machine deployments have not picked the new version because other machine deployments are rolling out (not all replicas ready)",
369372
reconcileErr: nil,
370373
cluster: &clusterv1.Cluster{},
371374
s: &scope.Scope{
@@ -388,10 +391,12 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) {
388391
WithReplicas(2).
389392
WithVersion("v1.22.0").
390393
WithStatus(clusterv1.MachineDeploymentStatus{
391-
Replicas: int32(1),
392-
UpdatedReplicas: int32(1),
393-
ReadyReplicas: int32(1),
394-
AvailableReplicas: int32(1),
394+
// MD is not ready because we don't have 2 updated, ready and available replicas.
395+
Replicas: int32(2),
396+
UpdatedReplicas: int32(1),
397+
ReadyReplicas: int32(1),
398+
AvailableReplicas: int32(1),
399+
UnavailableReplicas: int32(0),
395400
}).
396401
Build(),
397402
},
@@ -400,10 +405,70 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) {
400405
WithReplicas(2).
401406
WithVersion("v1.21.2").
402407
WithStatus(clusterv1.MachineDeploymentStatus{
403-
Replicas: int32(2),
404-
UpdatedReplicas: int32(2),
405-
ReadyReplicas: int32(2),
406-
AvailableReplicas: int32(2),
408+
Replicas: int32(2),
409+
UpdatedReplicas: int32(2),
410+
ReadyReplicas: int32(2),
411+
AvailableReplicas: int32(2),
412+
UnavailableReplicas: int32(0),
413+
}).
414+
Build(),
415+
},
416+
},
417+
},
418+
UpgradeTracker: func() *scope.UpgradeTracker {
419+
ut := scope.NewUpgradeTracker()
420+
ut.ControlPlane.PendingUpgrade = false
421+
ut.MachineDeployments.MarkPendingUpgrade("md1-abc123")
422+
return ut
423+
}(),
424+
HookResponseTracker: scope.NewHookResponseTracker(),
425+
},
426+
wantConditionStatus: corev1.ConditionFalse,
427+
wantConditionReason: clusterv1.TopologyReconciledMachineDeploymentsUpgradePendingReason,
428+
},
429+
{
430+
name: "should set the condition to false is some machine deployments have not picked the new version because other machine deployments are rolling out (unavailable replica)",
431+
reconcileErr: nil,
432+
cluster: &clusterv1.Cluster{},
433+
s: &scope.Scope{
434+
Blueprint: &scope.ClusterBlueprint{
435+
Topology: &clusterv1.Topology{
436+
Version: "v1.22.0",
437+
},
438+
},
439+
Current: &scope.ClusterState{
440+
Cluster: &clusterv1.Cluster{},
441+
ControlPlane: &scope.ControlPlaneState{
442+
Object: builder.ControlPlane("ns1", "controlplane1").
443+
WithVersion("v1.22.0").
444+
WithReplicas(3).
445+
Build(),
446+
},
447+
MachineDeployments: scope.MachineDeploymentsStateMap{
448+
"md0": &scope.MachineDeploymentState{
449+
Object: builder.MachineDeployment("ns1", "md0-abc123").
450+
WithReplicas(2).
451+
WithVersion("v1.22.0").
452+
WithStatus(clusterv1.MachineDeploymentStatus{
453+
// MD is not ready because we still have an unavailable replica.
454+
Replicas: int32(2),
455+
UpdatedReplicas: int32(2),
456+
ReadyReplicas: int32(2),
457+
AvailableReplicas: int32(2),
458+
UnavailableReplicas: int32(1),
459+
}).
460+
Build(),
461+
},
462+
"md1": &scope.MachineDeploymentState{
463+
Object: builder.MachineDeployment("ns1", "md1-abc123").
464+
WithReplicas(2).
465+
WithVersion("v1.21.2").
466+
WithStatus(clusterv1.MachineDeploymentStatus{
467+
Replicas: int32(2),
468+
UpdatedReplicas: int32(2),
469+
ReadyReplicas: int32(2),
470+
AvailableReplicas: int32(2),
471+
UnavailableReplicas: int32(0),
407472
}).
408473
Build(),
409474
},
@@ -444,10 +509,11 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) {
444509
WithReplicas(2).
445510
WithVersion("v1.22.0").
446511
WithStatus(clusterv1.MachineDeploymentStatus{
447-
Replicas: int32(1),
448-
UpdatedReplicas: int32(1),
449-
ReadyReplicas: int32(1),
450-
AvailableReplicas: int32(1),
512+
Replicas: int32(1),
513+
UpdatedReplicas: int32(1),
514+
ReadyReplicas: int32(1),
515+
AvailableReplicas: int32(1),
516+
UnavailableReplicas: int32(0),
451517
}).
452518
Build(),
453519
},
@@ -456,10 +522,11 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) {
456522
WithReplicas(2).
457523
WithVersion("v1.22.0").
458524
WithStatus(clusterv1.MachineDeploymentStatus{
459-
Replicas: int32(2),
460-
UpdatedReplicas: int32(2),
461-
ReadyReplicas: int32(2),
462-
AvailableReplicas: int32(2),
525+
Replicas: int32(2),
526+
UpdatedReplicas: int32(2),
527+
ReadyReplicas: int32(2),
528+
AvailableReplicas: int32(2),
529+
UnavailableReplicas: int32(0),
463530
}).
464531
Build(),
465532
},

‎internal/controllers/topology/cluster/desired_state_test.go

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -634,22 +634,24 @@ func TestComputeControlPlaneVersion(t *testing.T) {
634634
WithGeneration(int64(1)).
635635
WithReplicas(int32(2)).
636636
WithStatus(clusterv1.MachineDeploymentStatus{
637-
ObservedGeneration: 2,
638-
Replicas: 2,
639-
UpdatedReplicas: 2,
640-
AvailableReplicas: 2,
641-
ReadyReplicas: 2,
637+
ObservedGeneration: 2,
638+
Replicas: 2,
639+
UpdatedReplicas: 2,
640+
AvailableReplicas: 2,
641+
ReadyReplicas: 2,
642+
UnavailableReplicas: 0,
642643
}).
643644
Build()
644645
machineDeploymentRollingOut := builder.MachineDeployment("test-namespace", "md2").
645646
WithGeneration(int64(1)).
646647
WithReplicas(int32(2)).
647648
WithStatus(clusterv1.MachineDeploymentStatus{
648-
ObservedGeneration: 2,
649-
Replicas: 1,
650-
UpdatedReplicas: 1,
651-
AvailableReplicas: 1,
652-
ReadyReplicas: 1,
649+
ObservedGeneration: 2,
650+
Replicas: 1,
651+
UpdatedReplicas: 1,
652+
AvailableReplicas: 1,
653+
ReadyReplicas: 1,
654+
UnavailableReplicas: 0,
653655
}).
654656
Build()
655657

@@ -1769,6 +1771,7 @@ func TestComputeMachineDeploymentVersion(t *testing.T) {
17691771
// - md.spec.replicas == md.status.replicas
17701772
// - md.spec.replicas == md.status.updatedReplicas
17711773
// - md.spec.replicas == md.status.readyReplicas
1774+
// - md.status.unavailableReplicas == 0
17721775
// - md.Generation < md.status.observedGeneration
17731776
//
17741777
// A machine deployment is considered upgrading if any of the above conditions
@@ -1777,22 +1780,24 @@ func TestComputeMachineDeploymentVersion(t *testing.T) {
17771780
WithGeneration(1).
17781781
WithReplicas(2).
17791782
WithStatus(clusterv1.MachineDeploymentStatus{
1780-
ObservedGeneration: 2,
1781-
Replicas: 2,
1782-
UpdatedReplicas: 2,
1783-
AvailableReplicas: 2,
1784-
ReadyReplicas: 2,
1783+
ObservedGeneration: 2,
1784+
Replicas: 2,
1785+
UpdatedReplicas: 2,
1786+
AvailableReplicas: 2,
1787+
ReadyReplicas: 2,
1788+
UnavailableReplicas: 0,
17851789
}).
17861790
Build()
17871791
machineDeploymentRollingOut := builder.MachineDeployment("test-namespace", "md-2").
17881792
WithGeneration(1).
17891793
WithReplicas(2).
17901794
WithStatus(clusterv1.MachineDeploymentStatus{
1791-
ObservedGeneration: 2,
1792-
Replicas: 1,
1793-
UpdatedReplicas: 1,
1794-
AvailableReplicas: 1,
1795-
ReadyReplicas: 1,
1795+
ObservedGeneration: 2,
1796+
Replicas: 1,
1797+
UpdatedReplicas: 1,
1798+
AvailableReplicas: 1,
1799+
ReadyReplicas: 1,
1800+
UnavailableReplicas: 1,
17961801
}).
17971802
Build()
17981803

‎internal/controllers/topology/cluster/scope/state.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,5 +94,7 @@ type MachineDeploymentState struct {
9494
// A machine deployment is considered upgrading if:
9595
// - if any of the replicas of the machine deployment is not ready.
9696
func (md *MachineDeploymentState) IsRollingOut() bool {
97-
return !mdutil.DeploymentComplete(md.Object, &md.Object.Status) || *md.Object.Spec.Replicas != md.Object.Status.ReadyReplicas
97+
return !mdutil.DeploymentComplete(md.Object, &md.Object.Status) ||
98+
*md.Object.Spec.Replicas != md.Object.Status.ReadyReplicas ||
99+
md.Object.Status.UnavailableReplicas > 0
98100
}

0 commit comments

Comments
 (0)