diff --git a/controlplane/kubeadm/internal/controllers/status.go b/controlplane/kubeadm/internal/controllers/status.go index 73afb280aa32..75b61a3a7744 100644 --- a/controlplane/kubeadm/internal/controllers/status.go +++ b/controlplane/kubeadm/internal/controllers/status.go @@ -358,6 +358,13 @@ func setMachinesReadyCondition(ctx context.Context, kcp *controlplanev1.KubeadmC } func setMachinesUpToDateCondition(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, machines collections.Machines) { + // Only consider Machines that have an UpToDate condition or are older than 10s. + // This is done to ensure the MachinesUpToDate condition doesn't flicker after a new Machine is created, + // because it can take a bit until the UpToDate condition is set on a new Machine. + machines = machines.Filter(func(machine *clusterv1.Machine) bool { + return v1beta2conditions.Has(machine, clusterv1.MachineUpToDateV1Beta2Condition) || time.Since(machine.CreationTimestamp.Time) > 10*time.Second + }) + if len(machines) == 0 { v1beta2conditions.Set(kcp, metav1.Condition{ Type: controlplanev1.KubeadmControlPlaneMachinesUpToDateV1Beta2Condition, diff --git a/controlplane/kubeadm/internal/controllers/status_test.go b/controlplane/kubeadm/internal/controllers/status_test.go index 1e421eb74274..72f162728a43 100644 --- a/controlplane/kubeadm/internal/controllers/status_test.go +++ b/controlplane/kubeadm/internal/controllers/status_test.go @@ -476,19 +476,22 @@ func Test_setMachinesReadyAndMachinesUpToDateConditions(t *testing.T) { &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "m1"}, Status: clusterv1.MachineStatus{V1Beta2: &clusterv1.MachineV1Beta2Status{Conditions: []metav1.Condition{readyTrue, upToDateTrue}}}}, &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "m2"}, Status: clusterv1.MachineStatus{V1Beta2: &clusterv1.MachineV1Beta2Status{Conditions: []metav1.Condition{readyTrue, upToDateFalse}}}}, &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "m3"}, Status: clusterv1.MachineStatus{V1Beta2: &clusterv1.MachineV1Beta2Status{Conditions: []metav1.Condition{readyFalse, upToDateFalse}}}}, + &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "m4"}, Status: clusterv1.MachineStatus{V1Beta2: &clusterv1.MachineV1Beta2Status{Conditions: []metav1.Condition{readyFalse}}}}, // Machine without UpToDate condition + &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) ), }, expectMachinesReadyCondition: metav1.Condition{ Type: controlplanev1.KubeadmControlPlaneMachinesReadyV1Beta2Condition, Status: metav1.ConditionFalse, Reason: controlplanev1.KubeadmControlPlaneMachinesNotReadyV1Beta2Reason, - Message: "* Machine m3: NotReady", + Message: "* Machines m3, m4, m5: NotReady", }, expectMachinesUpToDateCondition: metav1.Condition{ - Type: controlplanev1.KubeadmControlPlaneMachinesUpToDateV1Beta2Condition, - Status: metav1.ConditionFalse, - Reason: controlplanev1.KubeadmControlPlaneMachinesNotUpToDateV1Beta2Reason, - Message: "* Machines m2, m3: NotUpToDate", + Type: controlplanev1.KubeadmControlPlaneMachinesUpToDateV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: controlplanev1.KubeadmControlPlaneMachinesNotUpToDateV1Beta2Reason, + Message: "* Machines m2, m3: NotUpToDate\n" + + "* Machine m4: Condition UpToDate not yet reported", }, }, } diff --git a/internal/controllers/cluster/cluster_controller_status.go b/internal/controllers/cluster/cluster_controller_status.go index 22bc2927d03e..6eadc5e220e2 100644 --- a/internal/controllers/cluster/cluster_controller_status.go +++ b/internal/controllers/cluster/cluster_controller_status.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "sort" + "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -654,6 +655,7 @@ func setMachinesReadyCondition(ctx context.Context, cluster *clusterv1.Cluster, func setMachinesUpToDateCondition(ctx context.Context, cluster *clusterv1.Cluster, machines collections.Machines, getDescendantsSucceeded bool) { log := ctrl.LoggerFrom(ctx) + // If we got unexpected errors in listing the machines (this should never happen), surface them. if !getDescendantsSucceeded { v1beta2conditions.Set(cluster, metav1.Condition{ @@ -665,6 +667,13 @@ func setMachinesUpToDateCondition(ctx context.Context, cluster *clusterv1.Cluste return } + // Only consider Machines that have an UpToDate condition or are older than 10s. + // This is done to ensure the MachinesUpToDate condition doesn't flicker after a new Machine is created, + // because it can take a bit until the UpToDate condition is set on a new Machine. + machines = machines.Filter(func(machine *clusterv1.Machine) bool { + return v1beta2conditions.Has(machine, clusterv1.MachineUpToDateV1Beta2Condition) || time.Since(machine.CreationTimestamp.Time) > 10*time.Second + }) + if len(machines) == 0 { v1beta2conditions.Set(cluster, metav1.Condition{ Type: clusterv1.ClusterMachinesUpToDateV1Beta2Condition, diff --git a/internal/controllers/cluster/cluster_controller_status_test.go b/internal/controllers/cluster/cluster_controller_status_test.go index 11664ea0f471..35f52999d2ba 100644 --- a/internal/controllers/cluster/cluster_controller_status_test.go +++ b/internal/controllers/cluster/cluster_controller_status_test.go @@ -1005,10 +1005,11 @@ func TestSetMachinesUpToDateCondition(t *testing.T) { }, }, { - name: "One machine without up-to-date condition", + name: "One machine without up-to-date condition, one new Machines without up-to-date condition", cluster: fakeCluster("c"), machines: []*clusterv1.Machine{ fakeMachine("no-condition-machine-1"), + fakeMachine("no-condition-machine-2-new", creationTimestamp{Time: time.Now().Add(-5 * time.Second)}), // ignored because it's new }, getDescendantsSucceeded: true, expectCondition: metav1.Condition{ @@ -2650,6 +2651,12 @@ func (r initialized) ApplyToControlPlane(cp *unstructured.Unstructured) { _ = contract.ControlPlane().Initialized().Set(cp, bool(r)) } +type creationTimestamp metav1.Time + +func (t creationTimestamp) ApplyToMachine(m *clusterv1.Machine) { + m.CreationTimestamp = metav1.Time(t) +} + type nodeRef corev1.ObjectReference func (r nodeRef) ApplyToMachine(m *clusterv1.Machine) { diff --git a/internal/controllers/machinedeployment/machinedeployment_status.go b/internal/controllers/machinedeployment/machinedeployment_status.go index 6bff6bf0aee3..85afa15ff173 100644 --- a/internal/controllers/machinedeployment/machinedeployment_status.go +++ b/internal/controllers/machinedeployment/machinedeployment_status.go @@ -320,6 +320,13 @@ func setMachinesUpToDateCondition(ctx context.Context, machineDeployment *cluste return } + // Only consider Machines that have an UpToDate condition or are older than 10s. + // This is done to ensure the MachinesUpToDate condition doesn't flicker after a new Machine is created, + // because it can take a bit until the UpToDate condition is set on a new Machine. + machines = machines.Filter(func(machine *clusterv1.Machine) bool { + return v1beta2conditions.Has(machine, clusterv1.MachineUpToDateV1Beta2Condition) || time.Since(machine.CreationTimestamp.Time) > 10*time.Second + }) + if len(machines) == 0 { v1beta2conditions.Set(machineDeployment, metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition, diff --git a/internal/controllers/machinedeployment/machinedeployment_status_test.go b/internal/controllers/machinedeployment/machinedeployment_status_test.go index 721a6f2fc642..5de23106ac79 100644 --- a/internal/controllers/machinedeployment/machinedeployment_status_test.go +++ b/internal/controllers/machinedeployment/machinedeployment_status_test.go @@ -814,10 +814,11 @@ func Test_setMachinesUpToDateCondition(t *testing.T) { }, }, { - name: "One machine without up-to-date condition", + name: "One machine without up-to-date condition, one new Machines without up-to-date condition", machineDeployment: &clusterv1.MachineDeployment{}, machines: []*clusterv1.Machine{ fakeMachine("no-condition-machine-1"), + fakeMachine("no-condition-machine-2-new", withCreationTimestamp(time.Now().Add(-5*time.Second))), // ignored because it's new }, getMachinesSucceeded: true, expectCondition: metav1.Condition{ @@ -1172,6 +1173,12 @@ func fakeMachine(name string, options ...fakeMachinesOption) *clusterv1.Machine return p } +func withCreationTimestamp(t time.Time) fakeMachinesOption { + return func(m *clusterv1.Machine) { + m.CreationTimestamp = metav1.Time{Time: t} + } +} + func withStaleDeletion() fakeMachinesOption { return func(m *clusterv1.Machine) { m.DeletionTimestamp = ptr.To(metav1.Time{Time: time.Now().Add(-1 * time.Hour)}) diff --git a/internal/controllers/machineset/machineset_controller_status.go b/internal/controllers/machineset/machineset_controller_status.go index bc383f5d1320..fe9d603c2e63 100644 --- a/internal/controllers/machineset/machineset_controller_status.go +++ b/internal/controllers/machineset/machineset_controller_status.go @@ -256,7 +256,7 @@ func setMachinesReadyCondition(ctx context.Context, machineSet *clusterv1.Machin v1beta2conditions.Set(machineSet, *readyCondition) } -func setMachinesUpToDateCondition(ctx context.Context, machineSet *clusterv1.MachineSet, machines []*clusterv1.Machine, getAndAdoptMachinesForMachineSetSucceeded bool) { +func setMachinesUpToDateCondition(ctx context.Context, machineSet *clusterv1.MachineSet, machinesSlice []*clusterv1.Machine, getAndAdoptMachinesForMachineSetSucceeded bool) { log := ctrl.LoggerFrom(ctx) // If we got unexpected errors in listing the machines (this should never happen), surface them. if !getAndAdoptMachinesForMachineSetSucceeded { @@ -269,6 +269,13 @@ func setMachinesUpToDateCondition(ctx context.Context, machineSet *clusterv1.Mac return } + // Only consider Machines that have an UpToDate condition or are older than 10s. + // This is done to ensure the MachinesUpToDate condition doesn't flicker after a new Machine is created, + // because it can take a bit until the UpToDate condition is set on a new Machine. + machines := collections.FromMachines(machinesSlice...).Filter(func(machine *clusterv1.Machine) bool { + return v1beta2conditions.Has(machine, clusterv1.MachineUpToDateV1Beta2Condition) || time.Since(machine.CreationTimestamp.Time) > 10*time.Second + }) + if len(machines) == 0 { v1beta2conditions.Set(machineSet, metav1.Condition{ Type: clusterv1.MachineSetMachinesUpToDateV1Beta2Condition, @@ -279,7 +286,7 @@ func setMachinesUpToDateCondition(ctx context.Context, machineSet *clusterv1.Mac } upToDateCondition, err := v1beta2conditions.NewAggregateCondition( - machines, clusterv1.MachineUpToDateV1Beta2Condition, + machines.UnsortedList(), clusterv1.MachineUpToDateV1Beta2Condition, v1beta2conditions.TargetConditionType(clusterv1.MachineSetMachinesUpToDateV1Beta2Condition), // Using a custom merge strategy to override reasons applied during merge. v1beta2conditions.CustomMergeStrategy{ diff --git a/internal/controllers/machineset/machineset_controller_status_test.go b/internal/controllers/machineset/machineset_controller_status_test.go index 043a7c57a05b..4d6acf6fdcbb 100644 --- a/internal/controllers/machineset/machineset_controller_status_test.go +++ b/internal/controllers/machineset/machineset_controller_status_test.go @@ -727,10 +727,11 @@ func Test_setMachinesUpToDateCondition(t *testing.T) { }, }, { - name: "One machine without up-to-date condition", + name: "One machine without up-to-date condition, one new Machines without up-to-date condition", machineSet: machineSet, machines: []*clusterv1.Machine{ newMachine("no-condition-machine-1"), + fakeMachine("no-condition-machine-2-new", withCreationTimestamp(time.Now().Add(-5*time.Second))), // ignored because it's new }, getAndAdoptMachinesForMachineSetSucceeded: true, expectCondition: metav1.Condition{ @@ -1028,6 +1029,12 @@ func fakeMachine(name string, options ...fakeMachinesOption) *clusterv1.Machine return p } +func withCreationTimestamp(t time.Time) fakeMachinesOption { + return func(m *clusterv1.Machine) { + m.CreationTimestamp = metav1.Time{Time: t} + } +} + func withV1Beta2Condition(c metav1.Condition) fakeMachinesOption { return func(m *clusterv1.Machine) { if m.Status.V1Beta2 == nil {