Skip to content

Commit bab3437

Browse files
fabriziopandinifabriziopandini
committed
Introduce util for isUpdating
1 parent b41e37d commit bab3437

File tree

12 files changed

+374
-64
lines changed

12 files changed

+374
-64
lines changed

‎controlplane/kubeadm/internal/controllers/controller.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import (
4848
"sigs.k8s.io/cluster-api/controlplane/kubeadm/internal"
4949
runtimeclient "sigs.k8s.io/cluster-api/exp/runtime/client"
5050
"sigs.k8s.io/cluster-api/feature"
51+
"sigs.k8s.io/cluster-api/internal/util/inplace"
5152
"sigs.k8s.io/cluster-api/internal/util/ssa"
5253
"sigs.k8s.io/cluster-api/util"
5354
"sigs.k8s.io/cluster-api/util/cache"
@@ -1039,6 +1040,16 @@ func reconcileMachineUpToDateCondition(_ context.Context, controlPlane *internal
10391040
})
10401041
continue
10411042
}
1043+
1044+
if inplace.IsUpdateInProgress(machine) {
1045+
conditions.Set(machine, metav1.Condition{
1046+
Type: clusterv1.MachineUpToDateCondition,
1047+
Status: metav1.ConditionFalse,
1048+
Reason: clusterv1.MachineUpToDateUpdatingReason,
1049+
})
1050+
continue
1051+
}
1052+
10421053
conditions.Set(machine, metav1.Condition{
10431054
Type: clusterv1.MachineUpToDateCondition,
10441055
Status: metav1.ConditionTrue,

‎controlplane/kubeadm/internal/controllers/controller_test.go‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2322,6 +2322,90 @@ func TestKubeadmControlPlaneReconciler_reconcileControlPlaneAndMachinesCondition
23222322
},
23232323
},
23242324
},
2325+
{
2326+
name: "Machines in place updating, machine not up-to-date date",
2327+
controlPlane: func() *internal.ControlPlane {
2328+
controlPlane, err := internal.NewControlPlane(ctx, nil, env.GetClient(), defaultCluster, defaultKCP.DeepCopy(), collections.FromMachines(
2329+
func() *clusterv1.Machine {
2330+
m := defaultMachine1.DeepCopy()
2331+
m.Annotations = map[string]string{
2332+
clusterv1.UpdateInProgressAnnotation: "",
2333+
}
2334+
return m
2335+
}(),
2336+
))
2337+
if err != nil {
2338+
panic(err)
2339+
}
2340+
return controlPlane
2341+
}(),
2342+
managementCluster: &fakeManagementCluster{
2343+
Workload: &fakeWorkloadCluster{
2344+
Workload: &internal.Workload{
2345+
Client: fake.NewClientBuilder().Build(),
2346+
},
2347+
},
2348+
},
2349+
lastProbeSuccessTime: now.Add(-3 * time.Minute),
2350+
expectKCPConditions: []metav1.Condition{
2351+
{
2352+
Type: controlplanev1.KubeadmControlPlaneInitializedCondition,
2353+
Status: metav1.ConditionTrue,
2354+
Reason: controlplanev1.KubeadmControlPlaneInitializedReason,
2355+
},
2356+
{
2357+
Type: controlplanev1.KubeadmControlPlaneEtcdClusterHealthyCondition,
2358+
Status: metav1.ConditionUnknown,
2359+
Reason: controlplanev1.KubeadmControlPlaneEtcdClusterHealthUnknownReason,
2360+
Message: "* Machine machine1-test:\n" +
2361+
" * EtcdMemberHealthy: Waiting for a Node with spec.providerID foo to exist",
2362+
},
2363+
{
2364+
Type: controlplanev1.KubeadmControlPlaneControlPlaneComponentsHealthyCondition,
2365+
Status: metav1.ConditionUnknown,
2366+
Reason: controlplanev1.KubeadmControlPlaneControlPlaneComponentsHealthUnknownReason,
2367+
Message: "* Machine machine1-test:\n" +
2368+
" * Control plane components: Waiting for a Node with spec.providerID foo to exist",
2369+
},
2370+
},
2371+
expectMachineConditions: []metav1.Condition{
2372+
{
2373+
Type: controlplanev1.KubeadmControlPlaneMachineAPIServerPodHealthyCondition,
2374+
Status: metav1.ConditionUnknown,
2375+
Reason: controlplanev1.KubeadmControlPlaneMachinePodInspectionFailedReason,
2376+
Message: "Waiting for a Node with spec.providerID foo to exist",
2377+
},
2378+
{
2379+
Type: controlplanev1.KubeadmControlPlaneMachineControllerManagerPodHealthyCondition,
2380+
Status: metav1.ConditionUnknown,
2381+
Reason: controlplanev1.KubeadmControlPlaneMachinePodInspectionFailedReason,
2382+
Message: "Waiting for a Node with spec.providerID foo to exist",
2383+
},
2384+
{
2385+
Type: controlplanev1.KubeadmControlPlaneMachineSchedulerPodHealthyCondition,
2386+
Status: metav1.ConditionUnknown,
2387+
Reason: controlplanev1.KubeadmControlPlaneMachinePodInspectionFailedReason,
2388+
Message: "Waiting for a Node with spec.providerID foo to exist",
2389+
},
2390+
{
2391+
Type: controlplanev1.KubeadmControlPlaneMachineEtcdPodHealthyCondition,
2392+
Status: metav1.ConditionUnknown,
2393+
Reason: controlplanev1.KubeadmControlPlaneMachinePodInspectionFailedReason,
2394+
Message: "Waiting for a Node with spec.providerID foo to exist",
2395+
},
2396+
{
2397+
Type: controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyCondition,
2398+
Status: metav1.ConditionUnknown,
2399+
Reason: controlplanev1.KubeadmControlPlaneMachineEtcdMemberInspectionFailedReason,
2400+
Message: "Waiting for a Node with spec.providerID foo to exist",
2401+
},
2402+
{
2403+
Type: clusterv1.MachineUpToDateCondition,
2404+
Status: metav1.ConditionFalse,
2405+
Reason: clusterv1.MachineUpToDateUpdatingReason,
2406+
},
2407+
},
2408+
},
23252409
{
23262410
name: "Machines not up to date",
23272411
controlPlane: func() *internal.ControlPlane {

‎internal/controllers/machine/machine_controller_status.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"sigs.k8s.io/cluster-api/controllers/clustercache"
3636
"sigs.k8s.io/cluster-api/internal/contract"
3737
"sigs.k8s.io/cluster-api/internal/controllers/machinedeployment/mdutil"
38+
"sigs.k8s.io/cluster-api/internal/util/inplace"
3839
"sigs.k8s.io/cluster-api/util/conditions"
3940
)
4041

@@ -637,20 +638,23 @@ func setDeletingCondition(_ context.Context, machine *clusterv1.Machine, reconci
637638
}
638639

639640
func setUpdatingCondition(_ context.Context, machine *clusterv1.Machine, updatingReason, updatingMessage string) {
640-
if updatingReason == "" {
641+
if inplace.IsUpdateInProgress(machine) {
642+
if updatingReason == "" {
643+
updatingReason = clusterv1.MachineInPlaceUpdatingReason
644+
}
641645
conditions.Set(machine, metav1.Condition{
642-
Type: clusterv1.MachineUpdatingCondition,
643-
Status: metav1.ConditionFalse,
644-
Reason: clusterv1.MachineNotUpdatingReason,
646+
Type: clusterv1.MachineUpdatingCondition,
647+
Status: metav1.ConditionTrue,
648+
Reason: updatingReason,
649+
Message: updatingMessage,
645650
})
646651
return
647652
}
648653

649654
conditions.Set(machine, metav1.Condition{
650-
Type: clusterv1.MachineUpdatingCondition,
651-
Status: metav1.ConditionTrue,
652-
Reason: updatingReason,
653-
Message: updatingMessage,
655+
Type: clusterv1.MachineUpdatingCondition,
656+
Status: metav1.ConditionFalse,
657+
Reason: clusterv1.MachineNotUpdatingReason,
654658
})
655659
}
656660

‎internal/controllers/machine/machine_controller_status_test.go‎

Lines changed: 92 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1408,6 +1408,94 @@ func TestTransformControlPlaneAndEtcdConditions(t *testing.T) {
14081408
}
14091409
}
14101410

1411+
func TestSetUpdatingCondition(t *testing.T) {
1412+
tests := []struct {
1413+
name string
1414+
machine *clusterv1.Machine
1415+
updatingReason string
1416+
updatingMessage string
1417+
expectCondition *metav1.Condition
1418+
}{
1419+
{
1420+
name: "A machine not in-place updating is not updating",
1421+
machine: &clusterv1.Machine{},
1422+
updatingReason: "foo",
1423+
updatingMessage: "bar",
1424+
expectCondition: &metav1.Condition{
1425+
Type: clusterv1.MachineUpdatingCondition,
1426+
Status: metav1.ConditionFalse,
1427+
Reason: clusterv1.MachineNotUpdatingReason,
1428+
},
1429+
},
1430+
{
1431+
name: "A machine starting in-place update is updating",
1432+
machine: &clusterv1.Machine{
1433+
ObjectMeta: metav1.ObjectMeta{
1434+
Annotations: map[string]string{
1435+
clusterv1.UpdateInProgressAnnotation: "",
1436+
},
1437+
},
1438+
},
1439+
updatingReason: "foo",
1440+
updatingMessage: "bar",
1441+
expectCondition: &metav1.Condition{
1442+
Type: clusterv1.MachineUpdatingCondition,
1443+
Status: metav1.ConditionTrue,
1444+
Reason: "foo",
1445+
Message: "bar",
1446+
},
1447+
},
1448+
{
1449+
name: "A machine in-place updating is updating",
1450+
machine: &clusterv1.Machine{
1451+
ObjectMeta: metav1.ObjectMeta{
1452+
Annotations: map[string]string{
1453+
clusterv1.UpdateInProgressAnnotation: "",
1454+
runtimev1.PendingHooksAnnotation: "UpdateMachine",
1455+
},
1456+
},
1457+
},
1458+
updatingReason: "foo",
1459+
updatingMessage: "bar",
1460+
expectCondition: &metav1.Condition{
1461+
Type: clusterv1.MachineUpdatingCondition,
1462+
Status: metav1.ConditionTrue,
1463+
Reason: "foo",
1464+
Message: "bar",
1465+
},
1466+
},
1467+
{
1468+
name: "A machine stopping in-place updating is updating",
1469+
machine: &clusterv1.Machine{
1470+
ObjectMeta: metav1.ObjectMeta{
1471+
Annotations: map[string]string{
1472+
runtimev1.PendingHooksAnnotation: "UpdateMachine",
1473+
},
1474+
},
1475+
},
1476+
updatingReason: "foo",
1477+
updatingMessage: "bar",
1478+
expectCondition: &metav1.Condition{
1479+
Type: clusterv1.MachineUpdatingCondition,
1480+
Status: metav1.ConditionTrue,
1481+
Reason: "foo",
1482+
Message: "bar",
1483+
},
1484+
},
1485+
}
1486+
for _, tt := range tests {
1487+
t.Run(tt.name, func(t *testing.T) {
1488+
g := NewWithT(t)
1489+
1490+
setUpdatingCondition(ctx, tt.machine, tt.updatingReason, tt.updatingMessage)
1491+
1492+
condition := conditions.Get(tt.machine, clusterv1.MachineUpdatingCondition)
1493+
g.Expect(condition).ToNot(BeNil())
1494+
g.Expect(*condition).To(conditions.MatchCondition(*tt.expectCondition, conditions.IgnoreLastTransitionTime(true)))
1495+
})
1496+
}
1497+
}
1498+
14111499
func TestSetUpToDateCondition(t *testing.T) {
14121500
reconciliationTime := time.Now()
14131501
tests := []struct {
@@ -1491,8 +1579,8 @@ func TestSetUpToDateCondition(t *testing.T) {
14911579
},
14921580
expectCondition: &metav1.Condition{
14931581
Type: clusterv1.MachineUpToDateCondition,
1494-
Status: metav1.ConditionTrue,
1495-
Reason: clusterv1.MachineUpToDateReason,
1582+
Status: metav1.ConditionFalse,
1583+
Reason: clusterv1.MachineUpToDateUpdatingReason,
14961584
},
14971585
},
14981586
{
@@ -1663,10 +1751,9 @@ func TestSetUpToDateCondition(t *testing.T) {
16631751
t.Run(tt.name, func(t *testing.T) {
16641752
g := NewWithT(t)
16651753

1666-
machine := &clusterv1.Machine{}
1667-
setUpToDateCondition(ctx, machine, tt.machineSet, tt.machineDeployment)
1754+
setUpToDateCondition(ctx, tt.machine, tt.machineSet, tt.machineDeployment)
16681755

1669-
condition := conditions.Get(machine, clusterv1.MachineUpToDateCondition)
1756+
condition := conditions.Get(tt.machine, clusterv1.MachineUpToDateCondition)
16701757
if tt.expectCondition != nil {
16711758
g.Expect(condition).ToNot(BeNil())
16721759
g.Expect(*condition).To(conditions.MatchCondition(*tt.expectCondition, conditions.IgnoreLastTransitionTime(true)))

‎internal/controllers/machinedeployment/machinedeployment_rollout_planner_test.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,8 @@ import (
3939
"k8s.io/utils/ptr"
4040

4141
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
42-
runtimehooksv1 "sigs.k8s.io/cluster-api/api/runtime/hooks/v1alpha1"
4342
"sigs.k8s.io/cluster-api/internal/controllers/machinedeployment/mdutil"
44-
"sigs.k8s.io/cluster-api/internal/hooks"
43+
"sigs.k8s.io/cluster-api/internal/util/inplace"
4544
"sigs.k8s.io/cluster-api/util"
4645
"sigs.k8s.io/cluster-api/util/conversion"
4746
)
@@ -699,7 +698,7 @@ func machineSetControllerMutatorMoveMachines(log *fileLogger, ms *clusterv1.Mach
699698
}
700699

701700
// Make sure we are not moving machines still updating in place (this includes also machines still pending AcknowledgeMove).
702-
if _, ok := m.Annotations[clusterv1.UpdateInProgressAnnotation]; ok || hooks.IsPending(runtimehooksv1.UpdateMachine, m) {
701+
if inplace.IsUpdateInProgress(m) {
703702
machinesSetMachines = append(machinesSetMachines, m)
704703
continue
705704
}
@@ -1131,11 +1130,11 @@ func sortMachineSetMachinesByDeletionPriorityAndName(machines []*clusterv1.Machi
11311130
// - if both machines are not updating in place, delete first the machine with the lowest machine NameIndex (e.g. between m3 and m4, pick m3)
11321131
sort.Slice(machinesSetMachinesSortedByDeletePriority, func(i, j int) bool {
11331132
iPriority := 100
1134-
if _, ok := machinesSetMachinesSortedByDeletePriority[i].Annotations[clusterv1.UpdateInProgressAnnotation]; ok {
1133+
if inplace.IsUpdateInProgress(machinesSetMachinesSortedByDeletePriority[i]) {
11351134
iPriority = 1
11361135
}
11371136
jPriority := 100
1138-
if _, ok := machinesSetMachinesSortedByDeletePriority[j].Annotations[clusterv1.UpdateInProgressAnnotation]; ok {
1137+
if inplace.IsUpdateInProgress(machinesSetMachinesSortedByDeletePriority[j]) {
11391138
jPriority = 1
11401139
}
11411140
if iPriority == jPriority {

‎internal/controllers/machinedeployment/machinedeployment_rollout_rollingupdate.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,9 @@ import (
2828
ctrl "sigs.k8s.io/controller-runtime"
2929

3030
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
31-
runtimehooksv1 "sigs.k8s.io/cluster-api/api/runtime/hooks/v1alpha1"
3231
"sigs.k8s.io/cluster-api/feature"
3332
"sigs.k8s.io/cluster-api/internal/controllers/machinedeployment/mdutil"
34-
"sigs.k8s.io/cluster-api/internal/hooks"
33+
"sigs.k8s.io/cluster-api/internal/util/inplace"
3534
"sigs.k8s.io/cluster-api/util"
3635
"sigs.k8s.io/cluster-api/util/collections"
3736
)
@@ -525,9 +524,7 @@ func (p *rolloutPlanner) scalingOrInPlaceUpdateInProgress(_ context.Context) boo
525524
return true
526525
}
527526
for _, m := range p.machines {
528-
_, inPlaceUpdateInProgress := m.Annotations[clusterv1.UpdateInProgressAnnotation]
529-
hasUpdateMachinePending := hooks.IsPending(runtimehooksv1.UpdateMachine, m)
530-
if inPlaceUpdateInProgress || hasUpdateMachinePending {
527+
if inplace.IsUpdateInProgress(m) {
531528
return true
532529
}
533530
}

‎internal/controllers/machineset/machineset_controller.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import (
5454
"sigs.k8s.io/cluster-api/internal/controllers/machine"
5555
"sigs.k8s.io/cluster-api/internal/hooks"
5656
topologynames "sigs.k8s.io/cluster-api/internal/topology/names"
57+
"sigs.k8s.io/cluster-api/internal/util/inplace"
5758
"sigs.k8s.io/cluster-api/internal/util/ssa"
5859
"sigs.k8s.io/cluster-api/util"
5960
"sigs.k8s.io/cluster-api/util/annotations"
@@ -999,7 +1000,7 @@ func (r *Reconciler) startMoveMachines(ctx context.Context, s *scope, targetMSNa
9991000
log := log.WithValues("Machine", klog.KObj(machine))
10001001

10011002
// Make sure we are not moving machines still updating in place from a previous move (this includes also machines still pending AcknowledgeMove).
1002-
if _, ok := machine.Annotations[clusterv1.UpdateInProgressAnnotation]; ok || hooks.IsPending(runtimehooksv1.UpdateMachine, machine) {
1003+
if inplace.IsUpdateInProgress(machine) {
10031004
continue
10041005
}
10051006

‎internal/controllers/machineset/machineset_controller_status.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
ctrl "sigs.k8s.io/controller-runtime"
3232

3333
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
34+
"sigs.k8s.io/cluster-api/internal/util/inplace"
3435
"sigs.k8s.io/cluster-api/util/collections"
3536
"sigs.k8s.io/cluster-api/util/conditions"
3637
clog "sigs.k8s.io/cluster-api/util/log"
@@ -81,6 +82,13 @@ func setReplicas(_ context.Context, ms *clusterv1.MachineSet, machines []*cluste
8182

8283
var readyReplicas, availableReplicas, upToDateReplicas int32
8384
for _, machine := range machines {
85+
// If a machine is in-place updating consider it not Ready, not Available and not UpToDate.
86+
// Note: We have to check this here because we don't want to rely on an additional Machine controller
87+
// reconcile to set the conditions to be able to update the replica counters here correctly.
88+
if inplace.IsUpdateInProgress(machine) {
89+
continue
90+
}
91+
8492
if conditions.IsTrue(machine, clusterv1.MachineReadyCondition) {
8593
readyReplicas++
8694
}

0 commit comments

Comments
 (0)