From fedf8c4489fa26ee0b1225663fd325bfe3298586 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Thu, 21 Nov 2024 18:16:33 +0100 Subject: [PATCH] review fixes --- api/v1beta1/cluster_types.go | 60 +-- .../controllers/cluster/cluster_controller.go | 4 +- .../cluster/cluster_controller_status.go | 182 ++++----- .../cluster/cluster_controller_status_test.go | 348 +++++++++++++----- util/conditions/v1beta2/sort.go | 12 +- 5 files changed, 368 insertions(+), 238 deletions(-) diff --git a/api/v1beta1/cluster_types.go b/api/v1beta1/cluster_types.go index 4d1d972e7c47..441471cb33ef 100644 --- a/api/v1beta1/cluster_types.go +++ b/api/v1beta1/cluster_types.go @@ -223,92 +223,92 @@ const ( // Cluster's ControlPlaneMachinesReady condition and corresponding reasons that will be used in v1Beta2 API version. const ( - // ClusterControlPlaneMachinesReadyV1Beta2Condition surfaces detail of issues on the controlled machines, if any. + // ClusterControlPlaneMachinesReadyV1Beta2Condition surfaces detail of issues on control plane machines, if any. ClusterControlPlaneMachinesReadyV1Beta2Condition = "ControlPlaneMachinesReady" - // ClusterControlPlaneMachinesReadyV1Beta2Reason surfaces when all the controlled machine's Ready conditions are true. + // ClusterControlPlaneMachinesReadyV1Beta2Reason surfaces when all control plane machine's Ready conditions are true. ClusterControlPlaneMachinesReadyV1Beta2Reason = ReadyV1Beta2Reason - // ClusterControlPlaneMachinesNotReadyV1Beta2Reason surfaces when at least one of the controlled machine's Ready conditions is false. + // ClusterControlPlaneMachinesNotReadyV1Beta2Reason surfaces when at least one of control plane machine's Ready conditions is false. ClusterControlPlaneMachinesNotReadyV1Beta2Reason = NotReadyV1Beta2Reason - // ClusterControlPlaneMachinesReadyUnknownV1Beta2Reason surfaces when at least one of the controlled machine's Ready conditions is unknown - // and none of the controlled machine's Ready conditions is false. + // ClusterControlPlaneMachinesReadyUnknownV1Beta2Reason surfaces when at least one of control plane machine's Ready conditions is unknown + // and none of control plane machine's Ready conditions is false. ClusterControlPlaneMachinesReadyUnknownV1Beta2Reason = ReadyUnknownV1Beta2Reason - // ClusterControlPlaneMachinesReadyNoReplicasV1Beta2Reason surfaces when no machines exist for the Cluster. + // ClusterControlPlaneMachinesReadyNoReplicasV1Beta2Reason surfaces when no control plane machines exist for the Cluster. ClusterControlPlaneMachinesReadyNoReplicasV1Beta2Reason = NoReplicasV1Beta2Reason - // ClusterControlPlaneMachinesReadyInternalErrorV1Beta2Reason surfaces unexpected failures when listing machines - // or aggregating machine's conditions. + // ClusterControlPlaneMachinesReadyInternalErrorV1Beta2Reason surfaces unexpected failures when listing control plane machines + // or aggregating control plane machine's conditions. ClusterControlPlaneMachinesReadyInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason ) // Cluster's WorkerMachinesReady condition and corresponding reasons that will be used in v1Beta2 API version. const ( - // ClusterWorkerMachinesReadyV1Beta2Condition surfaces detail of issues on the controlled machines, if any. + // ClusterWorkerMachinesReadyV1Beta2Condition surfaces detail of issues on the worker machines, if any. ClusterWorkerMachinesReadyV1Beta2Condition = "WorkerMachinesReady" - // ClusterWorkerMachinesReadyV1Beta2Reason surfaces when all the controlled machine's Ready conditions are true. + // ClusterWorkerMachinesReadyV1Beta2Reason surfaces when all the worker machine's Ready conditions are true. ClusterWorkerMachinesReadyV1Beta2Reason = ReadyV1Beta2Reason - // ClusterWorkerMachinesNotReadyV1Beta2Reason surfaces when at least one of the controlled machine's Ready conditions is false. + // ClusterWorkerMachinesNotReadyV1Beta2Reason surfaces when at least one of the worker machine's Ready conditions is false. ClusterWorkerMachinesNotReadyV1Beta2Reason = NotReadyV1Beta2Reason - // ClusterWorkerMachinesReadyUnknownV1Beta2Reason surfaces when at least one of the controlled machine's Ready conditions is unknown - // and none of the controlled machine's Ready conditions is false. + // ClusterWorkerMachinesReadyUnknownV1Beta2Reason surfaces when at least one of the worker machine's Ready conditions is unknown + // and none of the worker machine's Ready conditions is false. ClusterWorkerMachinesReadyUnknownV1Beta2Reason = ReadyUnknownV1Beta2Reason - // ClusterWorkerMachinesReadyNoReplicasV1Beta2Reason surfaces when no machines exist for the Cluster. + // ClusterWorkerMachinesReadyNoReplicasV1Beta2Reason surfaces when no worker machines exist for the Cluster. ClusterWorkerMachinesReadyNoReplicasV1Beta2Reason = NoReplicasV1Beta2Reason - // ClusterWorkerMachinesReadyInternalErrorV1Beta2Reason surfaces unexpected failures when listing machines - // or aggregating machine's conditions. + // ClusterWorkerMachinesReadyInternalErrorV1Beta2Reason surfaces unexpected failures when listing worker machines + // or aggregating worker machine's conditions. ClusterWorkerMachinesReadyInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason ) // Cluster's ControlPlaneMachinesUpToDate condition and corresponding reasons that will be used in v1Beta2 API version. const ( - // ClusterControlPlaneMachinesUpToDateV1Beta2Condition surfaces details of Cluster's machines not up to date, if any. + // ClusterControlPlaneMachinesUpToDateV1Beta2Condition surfaces details of control plane machines not up to date, if any. ClusterControlPlaneMachinesUpToDateV1Beta2Condition = "ControlPlaneMachinesUpToDate" - // ClusterControlPlaneMachinesUpToDateV1Beta2Reason surfaces when all the controlled machine's UpToDate conditions are true. + // ClusterControlPlaneMachinesUpToDateV1Beta2Reason surfaces when all the control plane machine's UpToDate conditions are true. ClusterControlPlaneMachinesUpToDateV1Beta2Reason = UpToDateV1Beta2Reason - // ClusterControlPlaneMachinesNotUpToDateV1Beta2Reason surfaces when at least one of the controlled machine's UpToDate conditions is false. + // ClusterControlPlaneMachinesNotUpToDateV1Beta2Reason surfaces when at least one of the control plane machine's UpToDate conditions is false. ClusterControlPlaneMachinesNotUpToDateV1Beta2Reason = NotUpToDateV1Beta2Reason - // ClusterControlPlaneMachinesUpToDateUnknownV1Beta2Reason surfaces when at least one of the controlled machine's UpToDate conditions is unknown - // and none of the controlled machine's UpToDate conditions is false. + // ClusterControlPlaneMachinesUpToDateUnknownV1Beta2Reason surfaces when at least one of the control plane machine's UpToDate conditions is unknown + // and none of the control plane machine's UpToDate conditions is false. ClusterControlPlaneMachinesUpToDateUnknownV1Beta2Reason = UpToDateUnknownV1Beta2Reason - // ClusterControlPlaneMachinesUpToDateNoReplicasV1Beta2Reason surfaces when no machines exist for the Cluster. + // ClusterControlPlaneMachinesUpToDateNoReplicasV1Beta2Reason surfaces when no control plane machines exist for the Cluster. ClusterControlPlaneMachinesUpToDateNoReplicasV1Beta2Reason = NoReplicasV1Beta2Reason - // ClusterControlPlaneMachinesUpToDateInternalErrorV1Beta2Reason surfaces unexpected failures when listing machines + // ClusterControlPlaneMachinesUpToDateInternalErrorV1Beta2Reason surfaces unexpected failures when listing control plane machines // or aggregating status. ClusterControlPlaneMachinesUpToDateInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason ) // Cluster's WorkerMachinesUpToDate condition and corresponding reasons that will be used in v1Beta2 API version. const ( - // ClusterWorkerMachinesUpToDateV1Beta2Condition surfaces details of Cluster's machines not up to date, if any. + // ClusterWorkerMachinesUpToDateV1Beta2Condition surfaces details of worker machines not up to date, if any. ClusterWorkerMachinesUpToDateV1Beta2Condition = "WorkerMachinesUpToDate" - // ClusterWorkerMachinesUpToDateV1Beta2Reason surfaces when all the controlled machine's UpToDate conditions are true. + // ClusterWorkerMachinesUpToDateV1Beta2Reason surfaces when all the worker machine's UpToDate conditions are true. ClusterWorkerMachinesUpToDateV1Beta2Reason = UpToDateV1Beta2Reason - // ClusterWorkerMachinesNotUpToDateV1Beta2Reason surfaces when at least one of the controlled machine's UpToDate conditions is false. + // ClusterWorkerMachinesNotUpToDateV1Beta2Reason surfaces when at least one of the worker machine's UpToDate conditions is false. ClusterWorkerMachinesNotUpToDateV1Beta2Reason = NotUpToDateV1Beta2Reason - // ClusterWorkerMachinesUpToDateUnknownV1Beta2Reason surfaces when at least one of the controlled machine's UpToDate conditions is unknown - // and none of the controlled machine's UpToDate conditions is false. + // ClusterWorkerMachinesUpToDateUnknownV1Beta2Reason surfaces when at least one of the worker machine's UpToDate conditions is unknown + // and none of the worker machine's UpToDate conditions is false. ClusterWorkerMachinesUpToDateUnknownV1Beta2Reason = UpToDateUnknownV1Beta2Reason - // ClusterWorkerMachinesUpToDateNoReplicasV1Beta2Reason surfaces when no machines exist for the Cluster. + // ClusterWorkerMachinesUpToDateNoReplicasV1Beta2Reason surfaces when no worker machines exist for the Cluster. ClusterWorkerMachinesUpToDateNoReplicasV1Beta2Reason = NoReplicasV1Beta2Reason - // ClusterWorkerMachinesUpToDateInternalErrorV1Beta2Reason surfaces unexpected failures when listing machines + // ClusterWorkerMachinesUpToDateInternalErrorV1Beta2Reason surfaces unexpected failures when listing worker machines // or aggregating status. ClusterWorkerMachinesUpToDateInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason ) diff --git a/internal/controllers/cluster/cluster_controller.go b/internal/controllers/cluster/cluster_controller.go index ef0fa4e21848..543b149614d9 100644 --- a/internal/controllers/cluster/cluster_controller.go +++ b/internal/controllers/cluster/cluster_controller.go @@ -261,10 +261,10 @@ func patchCluster(ctx context.Context, patchHelper *patch.Helper, cluster *clust clusterv1.ClusterInfrastructureReadyV1Beta2Condition, clusterv1.ClusterControlPlaneAvailableV1Beta2Condition, clusterv1.ClusterControlPlaneInitializedV1Beta2Condition, - clusterv1.ClusterWorkersAvailableV1Beta2Condition, clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Condition, - clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition, + clusterv1.ClusterWorkersAvailableV1Beta2Condition, + clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, clusterv1.ClusterRemoteConnectionProbeV1Beta2Condition, clusterv1.ClusterScalingUpV1Beta2Condition, diff --git a/internal/controllers/cluster/cluster_controller_status.go b/internal/controllers/cluster/cluster_controller_status.go index ce8441b88843..eb0892624731 100644 --- a/internal/controllers/cluster/cluster_controller_status.go +++ b/internal/controllers/cluster/cluster_controller_status.go @@ -65,38 +65,10 @@ func (r *Reconciler) updateStatus(ctx context.Context, s *scope) { setControlPlaneAvailableCondition(ctx, s.cluster, s.controlPlane, s.controlPlaneIsNotFound) setControlPlaneInitializedCondition(ctx, s.cluster, s.controlPlane, s.descendants.controlPlaneMachines, s.infraClusterIsNotFound, s.getDescendantsSucceeded) setWorkersAvailableCondition(ctx, s.cluster, expv1.MachinePoolList{}, s.descendants.machineDeployments, s.getDescendantsSucceeded) - machinesReadySetter{ - condition: clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Condition, - internalErrorReason: clusterv1.ClusterControlPlaneMachinesReadyInternalErrorV1Beta2Reason, - noReplicasReason: clusterv1.ClusterControlPlaneMachinesReadyNoReplicasV1Beta2Reason, - notReadyReason: clusterv1.ClusterControlPlaneMachinesNotReadyV1Beta2Reason, - readyUnknownReason: clusterv1.ClusterControlPlaneMachinesReadyUnknownV1Beta2Reason, - readyReason: clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Reason, - }.setMachinesReadyCondition(ctx, s.cluster, controlPlaneMachines, s.getDescendantsSucceeded) - machinesReadySetter{ - condition: clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, - internalErrorReason: clusterv1.ClusterWorkerMachinesReadyInternalErrorV1Beta2Reason, - noReplicasReason: clusterv1.ClusterWorkerMachinesReadyNoReplicasV1Beta2Reason, - notReadyReason: clusterv1.ClusterWorkerMachinesNotReadyV1Beta2Reason, - readyUnknownReason: clusterv1.ClusterWorkerMachinesReadyUnknownV1Beta2Reason, - readyReason: clusterv1.ClusterWorkerMachinesReadyV1Beta2Reason, - }.setMachinesReadyCondition(ctx, s.cluster, workerMachines, s.getDescendantsSucceeded) - machinesUpToDateSetter{ - condition: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition, - internalErrorReason: clusterv1.ClusterControlPlaneMachinesUpToDateInternalErrorV1Beta2Reason, - noReplicasReason: clusterv1.ClusterControlPlaneMachinesUpToDateNoReplicasV1Beta2Reason, - notUpToDateReason: clusterv1.ClusterControlPlaneMachinesNotUpToDateV1Beta2Reason, - upToDateUnknownReason: clusterv1.ClusterControlPlaneMachinesUpToDateUnknownV1Beta2Reason, - upToDateReason: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Reason, - }.setMachinesUpToDateCondition(ctx, s.cluster, controlPlaneMachines, s.getDescendantsSucceeded) - machinesUpToDateSetter{ - condition: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, - internalErrorReason: clusterv1.ClusterWorkerMachinesUpToDateInternalErrorV1Beta2Reason, - noReplicasReason: clusterv1.ClusterWorkerMachinesUpToDateNoReplicasV1Beta2Reason, - notUpToDateReason: clusterv1.ClusterWorkerMachinesNotUpToDateV1Beta2Reason, - upToDateUnknownReason: clusterv1.ClusterWorkerMachinesUpToDateUnknownV1Beta2Reason, - upToDateReason: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Reason, - }.setMachinesUpToDateCondition(ctx, s.cluster, workerMachines, s.getDescendantsSucceeded) + setControlPlaneMachinesReadyCondition(ctx, s.cluster, controlPlaneMachines, s.getDescendantsSucceeded) + setWorkerMachinesReadyCondition(ctx, s.cluster, workerMachines, s.getDescendantsSucceeded) + setControlPlaneMachinesUpToDateCondition(ctx, s.cluster, controlPlaneMachines, s.getDescendantsSucceeded) + setWorkerMachinesUpToDateCondition(ctx, s.cluster, workerMachines, s.getDescendantsSucceeded) setScalingUpCondition(ctx, s.cluster, s.controlPlane, expv1.MachinePoolList{}, s.descendants.machineDeployments, s.descendants.machineSets, s.controlPlaneIsNotFound, s.getDescendantsSucceeded) setScalingDownCondition(ctx, s.cluster, s.controlPlane, expv1.MachinePoolList{}, s.descendants.machineDeployments, s.descendants.machineSets, s.controlPlaneIsNotFound, s.getDescendantsSucceeded) setRemediatingCondition(ctx, s.cluster, machinesToBeRemediated, unhealthyMachines, s.getDescendantsSucceeded) @@ -632,79 +604,82 @@ func setWorkersAvailableCondition(ctx context.Context, cluster *clusterv1.Cluste v1beta2conditions.Set(cluster, *workersAvailableCondition) } -type machinesReadySetter struct { - condition string - internalErrorReason string - noReplicasReason string - notReadyReason string - readyUnknownReason string - readyReason string +func setControlPlaneMachinesReadyCondition(ctx context.Context, cluster *clusterv1.Cluster, machines collections.Machines, getDescendantsSucceeded bool) { + machinesConditionSetter{ + condition: clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Condition, + machineAggregationCondition: clusterv1.MachineReadyV1Beta2Condition, + internalErrorReason: clusterv1.ClusterControlPlaneMachinesReadyInternalErrorV1Beta2Reason, + noReplicasReason: clusterv1.ClusterControlPlaneMachinesReadyNoReplicasV1Beta2Reason, + issueReason: clusterv1.ClusterControlPlaneMachinesNotReadyV1Beta2Reason, + unknownReason: clusterv1.ClusterControlPlaneMachinesReadyUnknownV1Beta2Reason, + infoReason: clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Reason, + }.setMachinesCondition(ctx, cluster, machines, getDescendantsSucceeded) } -func (s machinesReadySetter) setMachinesReadyCondition(ctx context.Context, cluster *clusterv1.Cluster, machines collections.Machines, getDescendantsSucceeded bool) { - log := ctrl.LoggerFrom(ctx) +func setWorkerMachinesReadyCondition(ctx context.Context, cluster *clusterv1.Cluster, machines collections.Machines, getDescendantsSucceeded bool) { + machinesConditionSetter{ + condition: clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, + machineAggregationCondition: clusterv1.MachineReadyV1Beta2Condition, + internalErrorReason: clusterv1.ClusterWorkerMachinesReadyInternalErrorV1Beta2Reason, + noReplicasReason: clusterv1.ClusterWorkerMachinesReadyNoReplicasV1Beta2Reason, + issueReason: clusterv1.ClusterWorkerMachinesNotReadyV1Beta2Reason, + unknownReason: clusterv1.ClusterWorkerMachinesReadyUnknownV1Beta2Reason, + infoReason: clusterv1.ClusterWorkerMachinesReadyV1Beta2Reason, + }.setMachinesCondition(ctx, cluster, machines, getDescendantsSucceeded) +} - // If there was some unexpected errors in listing descendants (this should never happen), surface it. - if !getDescendantsSucceeded { - v1beta2conditions.Set(cluster, metav1.Condition{ - Type: s.condition, - Status: metav1.ConditionUnknown, - Reason: s.internalErrorReason, - Message: "Please check controller logs for errors", - }) - return - } +func setControlPlaneMachinesUpToDateCondition(ctx context.Context, cluster *clusterv1.Cluster, machines collections.Machines, getDescendantsSucceeded bool) { + // 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: s.condition, - Status: metav1.ConditionTrue, - Reason: s.noReplicasReason, - }) - return - } + machinesConditionSetter{ + condition: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition, + machineAggregationCondition: clusterv1.MachineUpToDateV1Beta2Condition, + internalErrorReason: clusterv1.ClusterControlPlaneMachinesUpToDateInternalErrorV1Beta2Reason, + noReplicasReason: clusterv1.ClusterControlPlaneMachinesUpToDateNoReplicasV1Beta2Reason, + issueReason: clusterv1.ClusterControlPlaneMachinesNotUpToDateV1Beta2Reason, + unknownReason: clusterv1.ClusterControlPlaneMachinesUpToDateUnknownV1Beta2Reason, + infoReason: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Reason, + }.setMachinesCondition(ctx, cluster, machines, getDescendantsSucceeded) +} - machinesReadyCondition, err := v1beta2conditions.NewAggregateCondition( - machines.UnsortedList(), clusterv1.MachineReadyV1Beta2Condition, - v1beta2conditions.TargetConditionType(s.condition), - // Using a custom merge strategy to override reasons applied during merge - v1beta2conditions.CustomMergeStrategy{ - MergeStrategy: v1beta2conditions.DefaultMergeStrategy( - v1beta2conditions.ComputeReasonFunc(v1beta2conditions.GetDefaultComputeMergeReasonFunc( - s.notReadyReason, - s.readyUnknownReason, - s.readyReason, - )), - ), - }, - ) - if err != nil { - log.Error(err, "Failed to aggregate Machine's Ready conditions") - v1beta2conditions.Set(cluster, metav1.Condition{ - Type: s.condition, - Status: metav1.ConditionUnknown, - Reason: s.internalErrorReason, - Message: "Please check controller logs for errors", - }) - return - } +func setWorkerMachinesUpToDateCondition(ctx context.Context, cluster *clusterv1.Cluster, machines collections.Machines, getDescendantsSucceeded bool) { + // 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 + }) - v1beta2conditions.Set(cluster, *machinesReadyCondition) + machinesConditionSetter{ + condition: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + machineAggregationCondition: clusterv1.MachineUpToDateV1Beta2Condition, + internalErrorReason: clusterv1.ClusterWorkerMachinesUpToDateInternalErrorV1Beta2Reason, + noReplicasReason: clusterv1.ClusterWorkerMachinesUpToDateNoReplicasV1Beta2Reason, + issueReason: clusterv1.ClusterWorkerMachinesNotUpToDateV1Beta2Reason, + unknownReason: clusterv1.ClusterWorkerMachinesUpToDateUnknownV1Beta2Reason, + infoReason: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Reason, + }.setMachinesCondition(ctx, cluster, machines, getDescendantsSucceeded) } -type machinesUpToDateSetter struct { - condition string - internalErrorReason string - noReplicasReason string - notUpToDateReason string - upToDateUnknownReason string - upToDateReason string +type machinesConditionSetter struct { + condition string + machineAggregationCondition string + internalErrorReason string + noReplicasReason string + issueReason string + unknownReason string + infoReason string } -func (s machinesUpToDateSetter) setMachinesUpToDateCondition(ctx context.Context, cluster *clusterv1.Cluster, machines collections.Machines, getDescendantsSucceeded bool) { +func (s machinesConditionSetter) setMachinesCondition(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 there was some unexpected errors in listing descendants (this should never happen), surface it. if !getDescendantsSucceeded { v1beta2conditions.Set(cluster, metav1.Condition{ Type: s.condition, @@ -715,13 +690,6 @@ func (s machinesUpToDateSetter) setMachinesUpToDateCondition(ctx context.Context 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: s.condition, @@ -731,22 +699,22 @@ func (s machinesUpToDateSetter) setMachinesUpToDateCondition(ctx context.Context return } - upToDateCondition, err := v1beta2conditions.NewAggregateCondition( - machines.UnsortedList(), clusterv1.MachineUpToDateV1Beta2Condition, + machinesCondition, err := v1beta2conditions.NewAggregateCondition( + machines.UnsortedList(), s.machineAggregationCondition, v1beta2conditions.TargetConditionType(s.condition), // Using a custom merge strategy to override reasons applied during merge v1beta2conditions.CustomMergeStrategy{ MergeStrategy: v1beta2conditions.DefaultMergeStrategy( v1beta2conditions.ComputeReasonFunc(v1beta2conditions.GetDefaultComputeMergeReasonFunc( - s.notUpToDateReason, - s.upToDateUnknownReason, - s.upToDateReason, + s.issueReason, + s.unknownReason, + s.infoReason, )), ), }, ) if err != nil { - log.Error(err, "Failed to aggregate Machine's UpToDate conditions") + log.Error(err, "Failed to aggregate Machine's Ready conditions") v1beta2conditions.Set(cluster, metav1.Condition{ Type: s.condition, Status: metav1.ConditionUnknown, @@ -756,7 +724,7 @@ func (s machinesUpToDateSetter) setMachinesUpToDateCondition(ctx context.Context return } - v1beta2conditions.Set(cluster, *upToDateCondition) + v1beta2conditions.Set(cluster, *machinesCondition) } func setRemediatingCondition(ctx context.Context, cluster *clusterv1.Cluster, machinesToBeRemediated, unhealthyMachines collections.Machines, getMachinesSucceeded bool) { diff --git a/internal/controllers/cluster/cluster_controller_status_test.go b/internal/controllers/cluster/cluster_controller_status_test.go index b36d55668606..c19ff13763e0 100644 --- a/internal/controllers/cluster/cluster_controller_status_test.go +++ b/internal/controllers/cluster/cluster_controller_status_test.go @@ -772,29 +772,11 @@ func TestSetControlPlaneMachinesReadyCondition(t *testing.T) { Reason: clusterv1.MachineReadyV1Beta2Reason, } - controlPlaneSetter := machinesReadySetter{ - condition: clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Condition, - internalErrorReason: clusterv1.ClusterControlPlaneMachinesReadyInternalErrorV1Beta2Reason, - noReplicasReason: clusterv1.ClusterControlPlaneMachinesReadyNoReplicasV1Beta2Reason, - notReadyReason: clusterv1.ClusterControlPlaneMachinesNotReadyV1Beta2Reason, - readyUnknownReason: clusterv1.ClusterControlPlaneMachinesReadyUnknownV1Beta2Reason, - readyReason: clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Reason, - } - workerSetter := machinesReadySetter{ - condition: clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, - internalErrorReason: clusterv1.ClusterWorkerMachinesReadyInternalErrorV1Beta2Reason, - noReplicasReason: clusterv1.ClusterWorkerMachinesReadyNoReplicasV1Beta2Reason, - notReadyReason: clusterv1.ClusterWorkerMachinesNotReadyV1Beta2Reason, - readyUnknownReason: clusterv1.ClusterWorkerMachinesReadyUnknownV1Beta2Reason, - readyReason: clusterv1.ClusterWorkerMachinesReadyV1Beta2Reason, - } - tests := []struct { name string cluster *clusterv1.Cluster machines []*clusterv1.Machine getDescendantsSucceeded bool - setter machinesReadySetter expectCondition metav1.Condition }{ { @@ -802,11 +784,10 @@ func TestSetControlPlaneMachinesReadyCondition(t *testing.T) { cluster: fakeCluster("c"), machines: nil, getDescendantsSucceeded: false, - setter: workerSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, + Type: clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Condition, Status: metav1.ConditionUnknown, - Reason: clusterv1.ClusterWorkerMachinesReadyInternalErrorV1Beta2Reason, + Reason: clusterv1.ClusterControlPlaneMachinesReadyInternalErrorV1Beta2Reason, Message: "Please check controller logs for errors", }, }, @@ -815,41 +796,38 @@ func TestSetControlPlaneMachinesReadyCondition(t *testing.T) { cluster: fakeCluster("c"), machines: []*clusterv1.Machine{}, getDescendantsSucceeded: true, - setter: workerSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, + Type: clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Condition, Status: metav1.ConditionTrue, - Reason: clusterv1.ClusterWorkerMachinesReadyNoReplicasV1Beta2Reason, + Reason: clusterv1.ClusterControlPlaneMachinesReadyNoReplicasV1Beta2Reason, }, }, { name: "all machines are ready", cluster: fakeCluster("c"), machines: []*clusterv1.Machine{ - fakeMachine("machine-1", v1beta2Condition(readyCondition)), - fakeMachine("machine-2", v1beta2Condition(readyCondition)), + fakeMachine("machine-1", controlPlane(true), v1beta2Condition(readyCondition)), + fakeMachine("machine-2", controlPlane(true), v1beta2Condition(readyCondition)), }, getDescendantsSucceeded: true, - setter: workerSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, + Type: clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Condition, Status: metav1.ConditionTrue, - Reason: clusterv1.ClusterWorkerMachinesReadyV1Beta2Reason, + Reason: clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Reason, }, }, { name: "one ready, one has nothing reported", cluster: fakeCluster("c"), machines: []*clusterv1.Machine{ - fakeMachine("machine-1", v1beta2Condition(readyCondition)), - fakeMachine("machine-2"), + fakeMachine("machine-1", controlPlane(true), v1beta2Condition(readyCondition)), + fakeMachine("machine-2", controlPlane(true)), }, getDescendantsSucceeded: true, - setter: workerSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, + Type: clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Condition, Status: metav1.ConditionUnknown, - Reason: clusterv1.ClusterWorkerMachinesReadyUnknownV1Beta2Reason, + Reason: clusterv1.ClusterControlPlaneMachinesReadyUnknownV1Beta2Reason, Message: "* Machine machine-2: Condition Ready not yet reported", }, }, @@ -857,20 +835,20 @@ func TestSetControlPlaneMachinesReadyCondition(t *testing.T) { name: "one ready, one reporting not ready, one reporting unknown, one reporting deleting", cluster: fakeCluster("c"), machines: []*clusterv1.Machine{ - fakeMachine("machine-1", v1beta2Condition(readyCondition)), - fakeMachine("machine-2", v1beta2Condition(metav1.Condition{ + fakeMachine("machine-1", controlPlane(true), v1beta2Condition(readyCondition)), + fakeMachine("machine-2", controlPlane(true), v1beta2Condition(metav1.Condition{ Type: clusterv1.MachineReadyV1Beta2Condition, Status: metav1.ConditionFalse, Reason: "SomeReason", Message: "HealthCheckSucceeded: Some message", })), - fakeMachine("machine-3", v1beta2Condition(metav1.Condition{ + fakeMachine("machine-3", controlPlane(true), v1beta2Condition(metav1.Condition{ Type: clusterv1.MachineReadyV1Beta2Condition, Status: metav1.ConditionUnknown, Reason: "SomeUnknownReason", Message: "Some unknown message", })), - fakeMachine("machine-4", v1beta2Condition(metav1.Condition{ + fakeMachine("machine-4", controlPlane(true), v1beta2Condition(metav1.Condition{ Type: clusterv1.MachineReadyV1Beta2Condition, Status: metav1.ConditionFalse, Reason: clusterv1.MachineDeletingV1Beta2Reason, @@ -878,22 +856,105 @@ func TestSetControlPlaneMachinesReadyCondition(t *testing.T) { })), }, getDescendantsSucceeded: true, - setter: workerSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, + Type: clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Condition, Status: metav1.ConditionFalse, - Reason: clusterv1.ClusterWorkerMachinesNotReadyV1Beta2Reason, + Reason: clusterv1.ClusterControlPlaneMachinesNotReadyV1Beta2Reason, Message: "* Machine machine-2: HealthCheckSucceeded: Some message\n" + "* Machine machine-4: Deleting: Machine deletion in progress, stage: DrainingNode\n" + "* Machine machine-3: Some unknown message", }, }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + var machines collections.Machines + if tt.machines != nil { + machines = collections.FromMachines(tt.machines...) + } + setControlPlaneMachinesReadyCondition(ctx, tt.cluster, machines, tt.getDescendantsSucceeded) + + condition := v1beta2conditions.Get(tt.cluster, clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Condition) + g.Expect(condition).ToNot(BeNil()) + g.Expect(*condition).To(v1beta2conditions.MatchCondition(tt.expectCondition, v1beta2conditions.IgnoreLastTransitionTime(true))) + }) + } +} + +func TestSetWorkerMachinesReadyCondition(t *testing.T) { + readyCondition := metav1.Condition{ + Type: clusterv1.MachineReadyV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: clusterv1.MachineReadyV1Beta2Reason, + } + + tests := []struct { + name string + cluster *clusterv1.Cluster + machines []*clusterv1.Machine + getDescendantsSucceeded bool + expectCondition metav1.Condition + }{ + { + name: "get descendant failed", + cluster: fakeCluster("c"), + machines: nil, + getDescendantsSucceeded: false, + expectCondition: metav1.Condition{ + Type: clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, + Status: metav1.ConditionUnknown, + Reason: clusterv1.ClusterWorkerMachinesReadyInternalErrorV1Beta2Reason, + Message: "Please check controller logs for errors", + }, + }, + { + name: "no machines", + cluster: fakeCluster("c"), + machines: []*clusterv1.Machine{}, + getDescendantsSucceeded: true, + expectCondition: metav1.Condition{ + Type: clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: clusterv1.ClusterWorkerMachinesReadyNoReplicasV1Beta2Reason, + }, + }, + { + name: "all machines are ready", + cluster: fakeCluster("c"), + machines: []*clusterv1.Machine{ + fakeMachine("machine-1", v1beta2Condition(readyCondition)), + fakeMachine("machine-2", v1beta2Condition(readyCondition)), + }, + getDescendantsSucceeded: true, + expectCondition: metav1.Condition{ + Type: clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: clusterv1.ClusterWorkerMachinesReadyV1Beta2Reason, + }, + }, + { + name: "one ready, one has nothing reported", + cluster: fakeCluster("c"), + machines: []*clusterv1.Machine{ + fakeMachine("machine-1", v1beta2Condition(readyCondition)), + fakeMachine("machine-2"), + }, + getDescendantsSucceeded: true, + expectCondition: metav1.Condition{ + Type: clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, + Status: metav1.ConditionUnknown, + Reason: clusterv1.ClusterWorkerMachinesReadyUnknownV1Beta2Reason, + Message: "* Machine machine-2: Condition Ready not yet reported", + }, + }, { name: "one ready, one reporting not ready, one reporting unknown, one reporting deleting", cluster: fakeCluster("c"), machines: []*clusterv1.Machine{ - fakeMachine("machine-1", controlPlane(true), v1beta2Condition(readyCondition)), - fakeMachine("machine-2", controlPlane(true), v1beta2Condition(metav1.Condition{ + fakeMachine("machine-1", v1beta2Condition(readyCondition)), + fakeMachine("machine-2", v1beta2Condition(metav1.Condition{ Type: clusterv1.MachineReadyV1Beta2Condition, Status: metav1.ConditionFalse, Reason: "SomeReason", @@ -913,11 +974,10 @@ func TestSetControlPlaneMachinesReadyCondition(t *testing.T) { })), }, getDescendantsSucceeded: true, - setter: controlPlaneSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Condition, + Type: clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, Status: metav1.ConditionFalse, - Reason: clusterv1.ClusterControlPlaneMachinesNotReadyV1Beta2Reason, + Reason: clusterv1.ClusterWorkerMachinesNotReadyV1Beta2Reason, Message: "* Machine machine-2: HealthCheckSucceeded: Some message\n" + "* Machine machine-4: Deleting: Machine deletion in progress, stage: DrainingNode\n" + "* Machine machine-3: Some unknown message", @@ -932,39 +992,21 @@ func TestSetControlPlaneMachinesReadyCondition(t *testing.T) { if tt.machines != nil { machines = collections.FromMachines(tt.machines...) } - tt.setter.setMachinesReadyCondition(ctx, tt.cluster, machines, tt.getDescendantsSucceeded) + setWorkerMachinesReadyCondition(ctx, tt.cluster, machines, tt.getDescendantsSucceeded) - condition := v1beta2conditions.Get(tt.cluster, tt.expectCondition.Type) + condition := v1beta2conditions.Get(tt.cluster, clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition) g.Expect(condition).ToNot(BeNil()) g.Expect(*condition).To(v1beta2conditions.MatchCondition(tt.expectCondition, v1beta2conditions.IgnoreLastTransitionTime(true))) }) } } -func TestSetMachinesUpToDateCondition(t *testing.T) { - controlPlaneSetter := machinesUpToDateSetter{ - condition: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition, - internalErrorReason: clusterv1.ClusterControlPlaneMachinesUpToDateInternalErrorV1Beta2Reason, - noReplicasReason: clusterv1.ClusterControlPlaneMachinesUpToDateNoReplicasV1Beta2Reason, - notUpToDateReason: clusterv1.ClusterControlPlaneMachinesNotUpToDateV1Beta2Reason, - upToDateUnknownReason: clusterv1.ClusterControlPlaneMachinesUpToDateUnknownV1Beta2Reason, - upToDateReason: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Reason, - } - workerSetter := machinesUpToDateSetter{ - condition: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, - internalErrorReason: clusterv1.ClusterWorkerMachinesUpToDateInternalErrorV1Beta2Reason, - noReplicasReason: clusterv1.ClusterWorkerMachinesUpToDateNoReplicasV1Beta2Reason, - notUpToDateReason: clusterv1.ClusterWorkerMachinesNotUpToDateV1Beta2Reason, - upToDateUnknownReason: clusterv1.ClusterWorkerMachinesUpToDateUnknownV1Beta2Reason, - upToDateReason: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Reason, - } - +func TestSetControlPlaneMachinesUpToDateCondition(t *testing.T) { tests := []struct { name string cluster *clusterv1.Cluster machines []*clusterv1.Machine getDescendantsSucceeded bool - setter machinesUpToDateSetter expectCondition metav1.Condition }{ { @@ -972,11 +1014,10 @@ func TestSetMachinesUpToDateCondition(t *testing.T) { cluster: fakeCluster("c"), machines: nil, getDescendantsSucceeded: false, - setter: workerSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + Type: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionUnknown, - Reason: clusterv1.ClusterWorkerMachinesUpToDateInternalErrorV1Beta2Reason, + Reason: clusterv1.ClusterControlPlaneMachinesUpToDateInternalErrorV1Beta2Reason, Message: "Please check controller logs for errors", }, }, @@ -985,11 +1026,10 @@ func TestSetMachinesUpToDateCondition(t *testing.T) { cluster: fakeCluster("c"), machines: []*clusterv1.Machine{}, getDescendantsSucceeded: true, - setter: workerSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + Type: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionTrue, - Reason: clusterv1.ClusterWorkerMachinesUpToDateNoReplicasV1Beta2Reason, + Reason: clusterv1.ClusterControlPlaneMachinesUpToDateNoReplicasV1Beta2Reason, Message: "", }, }, @@ -1004,11 +1044,10 @@ func TestSetMachinesUpToDateCondition(t *testing.T) { })), }, getDescendantsSucceeded: true, - setter: workerSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + Type: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionTrue, - Reason: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Reason, + Reason: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Reason, Message: "", }, }, @@ -1024,11 +1063,10 @@ func TestSetMachinesUpToDateCondition(t *testing.T) { })), }, getDescendantsSucceeded: true, - setter: workerSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + Type: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionUnknown, - Reason: clusterv1.ClusterWorkerMachinesUpToDateUnknownV1Beta2Reason, + Reason: clusterv1.ClusterControlPlaneMachinesUpToDateUnknownV1Beta2Reason, Message: "* Machine unknown-1: some unknown message", }, }, @@ -1044,11 +1082,10 @@ func TestSetMachinesUpToDateCondition(t *testing.T) { })), }, getDescendantsSucceeded: true, - setter: workerSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + Type: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionFalse, - Reason: clusterv1.ClusterWorkerMachinesNotUpToDateV1Beta2Reason, + Reason: clusterv1.ClusterControlPlaneMachinesNotUpToDateV1Beta2Reason, Message: "* Machine not-up-to-date-machine-1: some not up-to-date message", }, }, @@ -1060,11 +1097,10 @@ func TestSetMachinesUpToDateCondition(t *testing.T) { fakeMachine("no-condition-machine-2-new", creationTimestamp{Time: time.Now().Add(-5 * time.Second)}), // ignored because it's new }, getDescendantsSucceeded: true, - setter: workerSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + Type: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionUnknown, - Reason: clusterv1.ClusterWorkerMachinesUpToDateUnknownV1Beta2Reason, + Reason: clusterv1.ClusterControlPlaneMachinesUpToDateUnknownV1Beta2Reason, Message: "* Machine no-condition-machine-1: Condition UpToDate not yet reported", }, }, @@ -1098,36 +1134,155 @@ func TestSetMachinesUpToDateCondition(t *testing.T) { fakeMachine("no-condition-machine-2"), }, getDescendantsSucceeded: true, - setter: workerSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + Type: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionFalse, - Reason: clusterv1.ClusterWorkerMachinesNotUpToDateV1Beta2Reason, + Reason: clusterv1.ClusterControlPlaneMachinesNotUpToDateV1Beta2Reason, Message: "* Machines not-up-to-date-machine-1, not-up-to-date-machine-2: This is not up-to-date message\n" + "* Machines no-condition-machine-1, no-condition-machine-2: Condition UpToDate not yet reported", }, }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + var machines collections.Machines + if tt.machines != nil { + machines = collections.FromMachines(tt.machines...) + } + setControlPlaneMachinesUpToDateCondition(ctx, tt.cluster, machines, tt.getDescendantsSucceeded) + + condition := v1beta2conditions.Get(tt.cluster, clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition) + g.Expect(condition).ToNot(BeNil()) + g.Expect(*condition).To(v1beta2conditions.MatchCondition(tt.expectCondition, v1beta2conditions.IgnoreLastTransitionTime(true))) + }) + } +} +func TestSetWorkerMachinesUpToDateCondition(t *testing.T) { + tests := []struct { + name string + cluster *clusterv1.Cluster + machines []*clusterv1.Machine + getDescendantsSucceeded bool + expectCondition metav1.Condition + }{ + { + name: "get descendant failed", + cluster: fakeCluster("c"), + machines: nil, + getDescendantsSucceeded: false, + expectCondition: metav1.Condition{ + Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + Status: metav1.ConditionUnknown, + Reason: clusterv1.ClusterWorkerMachinesUpToDateInternalErrorV1Beta2Reason, + Message: "Please check controller logs for errors", + }, + }, + { + name: "no machines", + cluster: fakeCluster("c"), + machines: []*clusterv1.Machine{}, + getDescendantsSucceeded: true, + expectCondition: metav1.Condition{ + Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: clusterv1.ClusterWorkerMachinesUpToDateNoReplicasV1Beta2Reason, + Message: "", + }, + }, + { + name: "One machine up-to-date", + cluster: fakeCluster("c"), + machines: []*clusterv1.Machine{ + fakeMachine("up-to-date-1", v1beta2Condition(metav1.Condition{ + Type: clusterv1.MachineUpToDateV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: "some-reason-1", + })), + }, + getDescendantsSucceeded: true, + expectCondition: metav1.Condition{ + Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Reason, + Message: "", + }, + }, + { + name: "One machine unknown", + cluster: fakeCluster("c"), + machines: []*clusterv1.Machine{ + fakeMachine("unknown-1", v1beta2Condition(metav1.Condition{ + Type: clusterv1.MachineUpToDateV1Beta2Condition, + Status: metav1.ConditionUnknown, + Reason: "some-unknown-reason-1", + Message: "some unknown message", + })), + }, + getDescendantsSucceeded: true, + expectCondition: metav1.Condition{ + Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + Status: metav1.ConditionUnknown, + Reason: clusterv1.ClusterWorkerMachinesUpToDateUnknownV1Beta2Reason, + Message: "* Machine unknown-1: some unknown message", + }, + }, + { + name: "One machine not up-to-date", + cluster: fakeCluster("c"), + machines: []*clusterv1.Machine{ + fakeMachine("not-up-to-date-machine-1", v1beta2Condition(metav1.Condition{ + Type: clusterv1.MachineUpToDateV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: "some-not-up-to-date-reason", + Message: "some not up-to-date message", + })), + }, + getDescendantsSucceeded: true, + expectCondition: metav1.Condition{ + Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: clusterv1.ClusterWorkerMachinesNotUpToDateV1Beta2Reason, + Message: "* Machine not-up-to-date-machine-1: some not up-to-date message", + }, + }, + { + 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{ + Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, + Status: metav1.ConditionUnknown, + Reason: clusterv1.ClusterWorkerMachinesUpToDateUnknownV1Beta2Reason, + Message: "* Machine no-condition-machine-1: Condition UpToDate not yet reported", + }, + }, { name: "Two machines not up-to-date, two up-to-date, two not reported", cluster: fakeCluster("c"), machines: []*clusterv1.Machine{ - fakeMachine("up-to-date-1", controlPlane(true), v1beta2Condition(metav1.Condition{ + fakeMachine("up-to-date-1", v1beta2Condition(metav1.Condition{ Type: clusterv1.MachineUpToDateV1Beta2Condition, Status: metav1.ConditionTrue, Reason: "TestUpToDate", })), - fakeMachine("up-to-date-2", controlPlane(true), v1beta2Condition(metav1.Condition{ + fakeMachine("up-to-date-2", v1beta2Condition(metav1.Condition{ Type: clusterv1.MachineUpToDateV1Beta2Condition, Status: metav1.ConditionTrue, Reason: "TestUpToDate", })), - fakeMachine("not-up-to-date-machine-1", controlPlane(true), v1beta2Condition(metav1.Condition{ + fakeMachine("not-up-to-date-machine-1", v1beta2Condition(metav1.Condition{ Type: clusterv1.MachineUpToDateV1Beta2Condition, Status: metav1.ConditionFalse, Reason: "TestNotUpToDate", Message: "This is not up-to-date message", })), - fakeMachine("not-up-to-date-machine-2", controlPlane(true), v1beta2Condition(metav1.Condition{ + fakeMachine("not-up-to-date-machine-2", v1beta2Condition(metav1.Condition{ Type: clusterv1.MachineUpToDateV1Beta2Condition, Status: metav1.ConditionFalse, Reason: "TestNotUpToDate", @@ -1137,11 +1292,10 @@ func TestSetMachinesUpToDateCondition(t *testing.T) { fakeMachine("no-condition-machine-2"), }, getDescendantsSucceeded: true, - setter: controlPlaneSetter, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition, + Type: clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionFalse, - Reason: clusterv1.ClusterControlPlaneMachinesNotUpToDateV1Beta2Reason, + Reason: clusterv1.ClusterWorkerMachinesNotUpToDateV1Beta2Reason, Message: "* Machines not-up-to-date-machine-1, not-up-to-date-machine-2: This is not up-to-date message\n" + "* Machines no-condition-machine-1, no-condition-machine-2: Condition UpToDate not yet reported", }, @@ -1155,9 +1309,9 @@ func TestSetMachinesUpToDateCondition(t *testing.T) { if tt.machines != nil { machines = collections.FromMachines(tt.machines...) } - tt.setter.setMachinesUpToDateCondition(ctx, tt.cluster, machines, tt.getDescendantsSucceeded) + setWorkerMachinesUpToDateCondition(ctx, tt.cluster, machines, tt.getDescendantsSucceeded) - condition := v1beta2conditions.Get(tt.cluster, tt.expectCondition.Type) + condition := v1beta2conditions.Get(tt.cluster, clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition) g.Expect(condition).ToNot(BeNil()) g.Expect(*condition).To(v1beta2conditions.MatchCondition(tt.expectCondition, v1beta2conditions.IgnoreLastTransitionTime(true))) }) diff --git a/util/conditions/v1beta2/sort.go b/util/conditions/v1beta2/sort.go index 885dfdcadcc4..47db7339cbd0 100644 --- a/util/conditions/v1beta2/sort.go +++ b/util/conditions/v1beta2/sort.go @@ -80,8 +80,12 @@ func defaultSortLessFunc(i, j metav1.Condition) bool { // | ScalingDown | x | x | x | x | x | | // | ScalingUp | x | x | x | x | x | | // | -- Aggregated from Machines -- | | | | | | | -// | MachinesReady | x | x | x | x | x | | -// | MachinesUpToDate | x | x | x | x | x | | +// | MachinesReady | | x | x | x | x | | +// | ControlPlaneMachinesReady | x | | | | | | +// | WorkerMachinesReady | x | | | | | | +// | MachinesUpToDate | | x | x | x | x | | +// | ControlPlaneMachinesUpToDate | x | | | | | | +// | WorkerMachinesUpToDate | x | | | | | | // | -- From other controllers -- | | | | | | | // | Readiness/Availability gates | x | | | | | x | // | -- Misc -- | | | | | | | @@ -116,7 +120,11 @@ var order = []string{ clusterv1.ScalingDownV1Beta2Condition, clusterv1.ScalingUpV1Beta2Condition, clusterv1.MachinesReadyV1Beta2Condition, + clusterv1.ClusterControlPlaneMachinesReadyV1Beta2Condition, + clusterv1.ClusterWorkerMachinesReadyV1Beta2Condition, clusterv1.MachinesUpToDateV1Beta2Condition, + clusterv1.ClusterControlPlaneMachinesUpToDateV1Beta2Condition, + clusterv1.ClusterWorkerMachinesUpToDateV1Beta2Condition, readinessAndAvailabilityGates, clusterv1.PausedV1Beta2Condition, clusterv1.DeletingV1Beta2Condition,