Consider updating condition when computing Machine's ready condition

Author: fabriziopandini

api/core/v1beta2/machine_types.go

@@ -120,7 +120,7 @@ const (
 // Machine's Ready condition and corresponding reasons.
 const (
 	// MachineReadyCondition is true if the Machine's deletionTimestamp is not set, Machine's BootstrapConfigReady, InfrastructureReady,
-	// NodeHealthy and HealthCheckSucceeded (if present) conditions are true; if other conditions are defined in spec.readinessGates,
+	// NodeHealthy and HealthCheckSucceeded (if present) conditions are true, Updating condition is false; if other conditions are defined in spec.readinessGates,
 	// these conditions must be true as well.
 	// Note:
 	// - When summarizing the Deleting condition:

internal/controllers/machine/machine_controller_status.go

@@ -657,12 +657,13 @@ func setReadyCondition(ctx context.Context, machine *clusterv1.Machine) {
 
 	forConditionTypes := conditions.ForConditionTypes{
 		clusterv1.MachineDeletingCondition,
+		clusterv1.MachineUpdatingCondition,
 		clusterv1.MachineBootstrapConfigReadyCondition,
 		clusterv1.MachineInfrastructureReadyCondition,
 		clusterv1.MachineNodeHealthyCondition,
 		clusterv1.MachineHealthCheckSucceededCondition,
 	}
-	negativePolarityConditionTypes := []string{clusterv1.MachineDeletingCondition}
+	negativePolarityConditionTypes := []string{clusterv1.MachineDeletingCondition, clusterv1.MachineUpdatingCondition}
 	for _, g := range machine.Spec.ReadinessGates {
 		forConditionTypes = append(forConditionTypes, g.ConditionType)
 		if g.Polarity == clusterv1.NegativePolarityCondition {
@@ -685,9 +686,10 @@ func setReadyCondition(ctx context.Context, machine *clusterv1.Machine) {
 				negativePolarityConditionTypes: negativePolarityConditionTypes,
 			},
 		},
-		// Instruct summary to consider Deleting condition with negative polarity.
+		// Instruct summary to consider Deleting and updating condition with negative polarity.
 		conditions.NegativePolarityConditionTypes{
 			clusterv1.MachineDeletingCondition,
+			clusterv1.MachineUpdatingCondition,
 		},
 	}
 

internal/controllers/machine/machine_controller_status_test.go

@@ -35,7 +35,7 @@ import (
 	"sigs.k8s.io/cluster-api/util/conditions"
 	v1beta1conditions "sigs.k8s.io/cluster-api/util/conditions/deprecated/v1beta1"
 	"sigs.k8s.io/cluster-api/util/kubeconfig"
-	builder "sigs.k8s.io/cluster-api/util/test/builder"
+	"sigs.k8s.io/cluster-api/util/test/builder"
 )
 
 func TestSetBootstrapReadyCondition(t *testing.T) {
@@ -1439,6 +1439,11 @@ func TestSetReadyCondition(t *testing.T) {
 							Status: metav1.ConditionFalse,
 							Reason: clusterv1.MachineNotDeletingReason,
 						},
+						{
+							Type:   clusterv1.MachineUpdatingCondition,
+							Status: metav1.ConditionFalse,
+							Reason: clusterv1.MachineNotUpdatingReason,
+						},
 					},
 				},
 			},
@@ -1479,6 +1484,11 @@ func TestSetReadyCondition(t *testing.T) {
 							Reason:  clusterv1.MachineDeletingWaitingForPreDrainHookReason,
 							Message: "Waiting for pre-drain hooks to succeed (hooks: test-hook)",
 						},
+						{
+							Type:   clusterv1.MachineUpdatingCondition,
+							Status: metav1.ConditionFalse,
+							Reason: clusterv1.MachineNotUpdatingReason,
+						},
 					},
 				},
 			},
@@ -1489,6 +1499,51 @@ func TestSetReadyCondition(t *testing.T) {
 				Message: "* Deleting: Machine deletion in progress, stage: WaitingForPreDrainHook",
 			},
 		},
+		{
+			name: "Aggregates Ready condition correctly while the machine is updating",
+			machine: &clusterv1.Machine{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "machine-test",
+					Namespace: metav1.NamespaceDefault,
+				},
+				Status: clusterv1.MachineStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:   clusterv1.MachineBootstrapConfigReadyCondition,
+							Status: metav1.ConditionTrue,
+							Reason: "Foo",
+						},
+						{
+							Type:   clusterv1.InfrastructureReadyCondition,
+							Status: metav1.ConditionTrue,
+							Reason: "Foo",
+						},
+						{
+							Type:   clusterv1.MachineNodeHealthyCondition,
+							Status: metav1.ConditionTrue,
+							Reason: "Foo",
+						},
+						{
+							Type:   clusterv1.MachineDeletingCondition,
+							Status: metav1.ConditionFalse,
+							Reason: clusterv1.MachineNotDeletingReason,
+						},
+						{
+							Type:    clusterv1.MachineUpdatingCondition,
+							Status:  metav1.ConditionTrue,
+							Reason:  clusterv1.MachineInPlaceUpdatingReason,
+							Message: "In place update in progress",
+						},
+					},
+				},
+			},
+			expectCondition: metav1.Condition{
+				Type:    clusterv1.MachineReadyCondition,
+				Status:  metav1.ConditionFalse,
+				Reason:  clusterv1.MachineNotReadyReason,
+				Message: "* Updating: In place update in progress",
+			},
+		},
 		{
 			name: "Aggregates Ready condition correctly while the machine is deleting (waiting for Node drain)",
 			machine: &clusterv1.Machine{
@@ -1528,6 +1583,11 @@ func TestSetReadyCondition(t *testing.T) {
   * some other error 4: pod-9-to-trigger-eviction-some-other-error
   * ... (1 more error applying to 1 Pod)`,
 						},
+						{
+							Type:   clusterv1.MachineUpdatingCondition,
+							Status: metav1.ConditionFalse,
+							Reason: clusterv1.MachineNotUpdatingReason,
+						},
 					},
 				},
 			},
@@ -1573,6 +1633,11 @@ func TestSetReadyCondition(t *testing.T) {
 							Status: metav1.ConditionFalse,
 							Reason: clusterv1.MachineNotDeletingReason,
 						},
+						{
+							Type:   clusterv1.MachineUpdatingCondition,
+							Status: metav1.ConditionFalse,
+							Reason: clusterv1.MachineNotUpdatingReason,
+						},
 					},
 				},
 			},
@@ -1640,6 +1705,11 @@ func TestSetReadyCondition(t *testing.T) {
 							Status: metav1.ConditionFalse,
 							Reason: clusterv1.MachineNotDeletingReason,
 						},
+						{
+							Type:   clusterv1.MachineUpdatingCondition,
+							Status: metav1.ConditionFalse,
+							Reason: clusterv1.MachineNotUpdatingReason,
+						},
 					},
 				},
 			},
@@ -1710,6 +1780,11 @@ func TestSetReadyCondition(t *testing.T) {
 							Status: metav1.ConditionFalse,
 							Reason: clusterv1.MachineNotDeletingReason,
 						},
+						{
+							Type:   clusterv1.MachineUpdatingCondition,
+							Status: metav1.ConditionFalse,
+							Reason: clusterv1.MachineNotUpdatingReason,
+						},
 					},
 				},
 			},

util/conditions/sort.go

@@ -76,6 +76,7 @@ func defaultSortLessFunc(i, j metav1.Condition) bool {
 // | OwnerRemediated                |         |     |    |    |    | x       |
 // | -- Operations --               |         |     |    |    |    |         |
 // | TopologyReconciled             | x       |     |    |    |    |         |
+// | Updating                       |         |     |    |    |    | x       |
 // | RollingOut                     | x       | x   | x  |    | x  |         |
 // | Remediating                    | x       | x   | x  | x  | x  |         |
 // | ScalingDown                    | x       | x   | x  | x  | x  |         |
@@ -117,6 +118,7 @@ var order = []string{
 	clusterv1.MachineHealthCheckSucceededCondition,
 	clusterv1.MachineOwnerRemediatedCondition,
 	clusterv1.ClusterTopologyReconciledCondition,
+	clusterv1.MachineUpdatingCondition,
 	clusterv1.RollingOutCondition,
 	clusterv1.RemediatingCondition,
 	clusterv1.ScalingDownCondition,