Skip to content
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

🌱 Ignore new Machines when calculating MachinesUpToDate condition #11433

Merged
merged 1 commit into from
Nov 18, 2024
Merged

🌱 Ignore new Machines when calculating MachinesUpToDate condition #11433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions controlplane/kubeadm/internal/controllers/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
13 changes: 8 additions & 5 deletions controlplane/kubeadm/internal/controllers/status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
}
Expand Down
9 changes: 9 additions & 0 deletions internal/controllers/cluster/cluster_controller_status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand All @@ -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,
Expand Down
9 changes: 8 additions & 1 deletion internal/controllers/cluster/cluster_controller_status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 7 additions & 0 deletions internal/controllers/machinedeployment/machinedeployment_status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 8 additions & 1 deletion internal/controllers/machinedeployment/machinedeployment_status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)})
Expand Down
11 changes: 9 additions & 2 deletions internal/controllers/machineset/machineset_controller_status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand All @@ -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{
Expand Down
9 changes: 8 additions & 1 deletion internal/controllers/machineset/machineset_controller_status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand Down