Skip to content

Commit b4a02a6

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #11433 from sbueringer/pr-uptodate-ignore-new-machines
🌱 Ignore new Machines when calculating MachinesUpToDate condition
2 parents 16c46dd + df85ff6 commit b4a02a6

File tree

8 files changed

+64
-10
lines changed

8 files changed

+64
-10
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,13 @@ func setMachinesReadyCondition(ctx context.Context, kcp *controlplanev1.KubeadmC
358358
}
359359

360360
func setMachinesUpToDateCondition(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, machines collections.Machines) {
361+
// Only consider Machines that have an UpToDate condition or are older than 10s.
362+
// This is done to ensure the MachinesUpToDate condition doesn't flicker after a new Machine is created,
363+
// because it can take a bit until the UpToDate condition is set on a new Machine.
364+
machines = machines.Filter(func(machine *clusterv1.Machine) bool {
365+
return v1beta2conditions.Has(machine, clusterv1.MachineUpToDateV1Beta2Condition) || time.Since(machine.CreationTimestamp.Time) > 10*time.Second
366+
})
367+
361368
if len(machines) == 0 {
362369
v1beta2conditions.Set(kcp, metav1.Condition{
363370
Type: controlplanev1.KubeadmControlPlaneMachinesUpToDateV1Beta2Condition,

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -476,19 +476,22 @@ func Test_setMachinesReadyAndMachinesUpToDateConditions(t *testing.T) {
476476
&clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "m1"}, Status: clusterv1.MachineStatus{V1Beta2: &clusterv1.MachineV1Beta2Status{Conditions: []metav1.Condition{readyTrue, upToDateTrue}}}},
477477
&clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "m2"}, Status: clusterv1.MachineStatus{V1Beta2: &clusterv1.MachineV1Beta2Status{Conditions: []metav1.Condition{readyTrue, upToDateFalse}}}},
478478
&clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "m3"}, Status: clusterv1.MachineStatus{V1Beta2: &clusterv1.MachineV1Beta2Status{Conditions: []metav1.Condition{readyFalse, upToDateFalse}}}},
479+
&clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "m4"}, Status: clusterv1.MachineStatus{V1Beta2: &clusterv1.MachineV1Beta2Status{Conditions: []metav1.Condition{readyFalse}}}}, // Machine without UpToDate condition
480+
&clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "m5", CreationTimestamp: metav1.Time{Time: time.Now().Add(-5 * time.Second)}}, Status: clusterv1.MachineStatus{V1Beta2: &clusterv1.MachineV1Beta2Status{Conditions: []metav1.Condition{readyFalse}}}}, // New Machine without UpToDate condition (should be ignored)
479481
),
480482
},
481483
expectMachinesReadyCondition: metav1.Condition{
482484
Type: controlplanev1.KubeadmControlPlaneMachinesReadyV1Beta2Condition,
483485
Status: metav1.ConditionFalse,
484486
Reason: controlplanev1.KubeadmControlPlaneMachinesNotReadyV1Beta2Reason,
485-
Message: "* Machine m3: NotReady",
487+
Message: "* Machines m3, m4, m5: NotReady",
486488
},
487489
expectMachinesUpToDateCondition: metav1.Condition{
488-
Type: controlplanev1.KubeadmControlPlaneMachinesUpToDateV1Beta2Condition,
489-
Status: metav1.ConditionFalse,
490-
Reason: controlplanev1.KubeadmControlPlaneMachinesNotUpToDateV1Beta2Reason,
491-
Message: "* Machines m2, m3: NotUpToDate",
490+
Type: controlplanev1.KubeadmControlPlaneMachinesUpToDateV1Beta2Condition,
491+
Status: metav1.ConditionFalse,
492+
Reason: controlplanev1.KubeadmControlPlaneMachinesNotUpToDateV1Beta2Reason,
493+
Message: "* Machines m2, m3: NotUpToDate\n" +
494+
"* Machine m4: Condition UpToDate not yet reported",
492495
},
493496
},
494497
}

‎internal/controllers/cluster/cluster_controller_status.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222
"sort"
23+
"time"
2324

2425
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2526
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -654,6 +655,7 @@ func setMachinesReadyCondition(ctx context.Context, cluster *clusterv1.Cluster,
654655

655656
func setMachinesUpToDateCondition(ctx context.Context, cluster *clusterv1.Cluster, machines collections.Machines, getDescendantsSucceeded bool) {
656657
log := ctrl.LoggerFrom(ctx)
658+
657659
// If we got unexpected errors in listing the machines (this should never happen), surface them.
658660
if !getDescendantsSucceeded {
659661
v1beta2conditions.Set(cluster, metav1.Condition{
@@ -665,6 +667,13 @@ func setMachinesUpToDateCondition(ctx context.Context, cluster *clusterv1.Cluste
665667
return
666668
}
667669

670+
// Only consider Machines that have an UpToDate condition or are older than 10s.
671+
// This is done to ensure the MachinesUpToDate condition doesn't flicker after a new Machine is created,
672+
// because it can take a bit until the UpToDate condition is set on a new Machine.
673+
machines = machines.Filter(func(machine *clusterv1.Machine) bool {
674+
return v1beta2conditions.Has(machine, clusterv1.MachineUpToDateV1Beta2Condition) || time.Since(machine.CreationTimestamp.Time) > 10*time.Second
675+
})
676+
668677
if len(machines) == 0 {
669678
v1beta2conditions.Set(cluster, metav1.Condition{
670679
Type: clusterv1.ClusterMachinesUpToDateV1Beta2Condition,

‎internal/controllers/cluster/cluster_controller_status_test.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1005,10 +1005,11 @@ func TestSetMachinesUpToDateCondition(t *testing.T) {
10051005
},
10061006
},
10071007
{
1008-
name: "One machine without up-to-date condition",
1008+
name: "One machine without up-to-date condition, one new Machines without up-to-date condition",
10091009
cluster: fakeCluster("c"),
10101010
machines: []*clusterv1.Machine{
10111011
fakeMachine("no-condition-machine-1"),
1012+
fakeMachine("no-condition-machine-2-new", creationTimestamp{Time: time.Now().Add(-5 * time.Second)}), // ignored because it's new
10121013
},
10131014
getDescendantsSucceeded: true,
10141015
expectCondition: metav1.Condition{
@@ -2650,6 +2651,12 @@ func (r initialized) ApplyToControlPlane(cp *unstructured.Unstructured) {
26502651
_ = contract.ControlPlane().Initialized().Set(cp, bool(r))
26512652
}
26522653

2654+
type creationTimestamp metav1.Time
2655+
2656+
func (t creationTimestamp) ApplyToMachine(m *clusterv1.Machine) {
2657+
m.CreationTimestamp = metav1.Time(t)
2658+
}
2659+
26532660
type nodeRef corev1.ObjectReference
26542661

26552662
func (r nodeRef) ApplyToMachine(m *clusterv1.Machine) {

‎internal/controllers/machinedeployment/machinedeployment_status.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,13 @@ func setMachinesUpToDateCondition(ctx context.Context, machineDeployment *cluste
320320
return
321321
}
322322

323+
// Only consider Machines that have an UpToDate condition or are older than 10s.
324+
// This is done to ensure the MachinesUpToDate condition doesn't flicker after a new Machine is created,
325+
// because it can take a bit until the UpToDate condition is set on a new Machine.
326+
machines = machines.Filter(func(machine *clusterv1.Machine) bool {
327+
return v1beta2conditions.Has(machine, clusterv1.MachineUpToDateV1Beta2Condition) || time.Since(machine.CreationTimestamp.Time) > 10*time.Second
328+
})
329+
323330
if len(machines) == 0 {
324331
v1beta2conditions.Set(machineDeployment, metav1.Condition{
325332
Type: clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition,

‎internal/controllers/machinedeployment/machinedeployment_status_test.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -814,10 +814,11 @@ func Test_setMachinesUpToDateCondition(t *testing.T) {
814814
},
815815
},
816816
{
817-
name: "One machine without up-to-date condition",
817+
name: "One machine without up-to-date condition, one new Machines without up-to-date condition",
818818
machineDeployment: &clusterv1.MachineDeployment{},
819819
machines: []*clusterv1.Machine{
820820
fakeMachine("no-condition-machine-1"),
821+
fakeMachine("no-condition-machine-2-new", withCreationTimestamp(time.Now().Add(-5*time.Second))), // ignored because it's new
821822
},
822823
getMachinesSucceeded: true,
823824
expectCondition: metav1.Condition{
@@ -1172,6 +1173,12 @@ func fakeMachine(name string, options ...fakeMachinesOption) *clusterv1.Machine
11721173
return p
11731174
}
11741175

1176+
func withCreationTimestamp(t time.Time) fakeMachinesOption {
1177+
return func(m *clusterv1.Machine) {
1178+
m.CreationTimestamp = metav1.Time{Time: t}
1179+
}
1180+
}
1181+
11751182
func withStaleDeletion() fakeMachinesOption {
11761183
return func(m *clusterv1.Machine) {
11771184
m.DeletionTimestamp = ptr.To(metav1.Time{Time: time.Now().Add(-1 * time.Hour)})

‎internal/controllers/machineset/machineset_controller_status.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ func setMachinesReadyCondition(ctx context.Context, machineSet *clusterv1.Machin
256256
v1beta2conditions.Set(machineSet, *readyCondition)
257257
}
258258

259-
func setMachinesUpToDateCondition(ctx context.Context, machineSet *clusterv1.MachineSet, machines []*clusterv1.Machine, getAndAdoptMachinesForMachineSetSucceeded bool) {
259+
func setMachinesUpToDateCondition(ctx context.Context, machineSet *clusterv1.MachineSet, machinesSlice []*clusterv1.Machine, getAndAdoptMachinesForMachineSetSucceeded bool) {
260260
log := ctrl.LoggerFrom(ctx)
261261
// If we got unexpected errors in listing the machines (this should never happen), surface them.
262262
if !getAndAdoptMachinesForMachineSetSucceeded {
@@ -269,6 +269,13 @@ func setMachinesUpToDateCondition(ctx context.Context, machineSet *clusterv1.Mac
269269
return
270270
}
271271

272+
// Only consider Machines that have an UpToDate condition or are older than 10s.
273+
// This is done to ensure the MachinesUpToDate condition doesn't flicker after a new Machine is created,
274+
// because it can take a bit until the UpToDate condition is set on a new Machine.
275+
machines := collections.FromMachines(machinesSlice...).Filter(func(machine *clusterv1.Machine) bool {
276+
return v1beta2conditions.Has(machine, clusterv1.MachineUpToDateV1Beta2Condition) || time.Since(machine.CreationTimestamp.Time) > 10*time.Second
277+
})
278+
272279
if len(machines) == 0 {
273280
v1beta2conditions.Set(machineSet, metav1.Condition{
274281
Type: clusterv1.MachineSetMachinesUpToDateV1Beta2Condition,
@@ -279,7 +286,7 @@ func setMachinesUpToDateCondition(ctx context.Context, machineSet *clusterv1.Mac
279286
}
280287

281288
upToDateCondition, err := v1beta2conditions.NewAggregateCondition(
282-
machines, clusterv1.MachineUpToDateV1Beta2Condition,
289+
machines.UnsortedList(), clusterv1.MachineUpToDateV1Beta2Condition,
283290
v1beta2conditions.TargetConditionType(clusterv1.MachineSetMachinesUpToDateV1Beta2Condition),
284291
// Using a custom merge strategy to override reasons applied during merge.
285292
v1beta2conditions.CustomMergeStrategy{

‎internal/controllers/machineset/machineset_controller_status_test.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -727,10 +727,11 @@ func Test_setMachinesUpToDateCondition(t *testing.T) {
727727
},
728728
},
729729
{
730-
name: "One machine without up-to-date condition",
730+
name: "One machine without up-to-date condition, one new Machines without up-to-date condition",
731731
machineSet: machineSet,
732732
machines: []*clusterv1.Machine{
733733
newMachine("no-condition-machine-1"),
734+
fakeMachine("no-condition-machine-2-new", withCreationTimestamp(time.Now().Add(-5*time.Second))), // ignored because it's new
734735
},
735736
getAndAdoptMachinesForMachineSetSucceeded: true,
736737
expectCondition: metav1.Condition{
@@ -1028,6 +1029,12 @@ func fakeMachine(name string, options ...fakeMachinesOption) *clusterv1.Machine
10281029
return p
10291030
}
10301031

1032+
func withCreationTimestamp(t time.Time) fakeMachinesOption {
1033+
return func(m *clusterv1.Machine) {
1034+
m.CreationTimestamp = metav1.Time{Time: t}
1035+
}
1036+
}
1037+
10311038
func withV1Beta2Condition(c metav1.Condition) fakeMachinesOption {
10321039
return func(m *clusterv1.Machine) {
10331040
if m.Status.V1Beta2 == nil {

0 commit comments

Comments
 (0)