MachinesCreated condition on MachineSet should show failed preflight …

…checks
kubernetes-sigs · May 26, 2023 · ee60a55 · ee60a55
1 parent 174e166
commit ee60a55
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 2 deletions.
diff --git a/api/v1beta1/condition_consts.go b/api/v1beta1/condition_consts.go
@@ -243,6 +243,10 @@ const (
 	// MachinesReadyCondition reports an aggregate of current status of the machines controlled by the MachineSet.
 	MachinesReadyCondition ConditionType = "MachinesReady"
 
+	// PreflightCheckFailedReason (Severity=Error) documents a MachineSet failing pre-flight checks
+	// to create machine(s).
+	PreflightCheckFailedReason = "PreflightCheckFailed"
+
 	// BootstrapTemplateCloningFailedReason (Severity=Error) documents a MachineSet failing to
 	// clone the bootstrap template.
 	BootstrapTemplateCloningFailedReason = "BootstrapTemplateCloningFailed"

diff --git a/internal/controllers/machineset/machineset_preflight.go b/internal/controllers/machineset/machineset_preflight.go
@@ -37,6 +37,7 @@ import (
 	"sigs.k8s.io/cluster-api/controllers/external"
 	"sigs.k8s.io/cluster-api/feature"
 	"sigs.k8s.io/cluster-api/internal/contract"
+	"sigs.k8s.io/cluster-api/util/conditions"
 )
 
 type preflightCheckError *string
@@ -45,7 +46,7 @@ type preflightCheckError *string
 // the preflight checks fail.
 const preflightFailedRequeueAfter = 15 * time.Second
 
-func (r *Reconciler) runPreflightChecks(ctx context.Context, cluster *clusterv1.Cluster, ms *clusterv1.MachineSet, action string) (ctrl.Result, error) {
+func (r *Reconciler) runPreflightChecks(ctx context.Context, cluster *clusterv1.Cluster, ms *clusterv1.MachineSet, action string) (_ ctrl.Result, retErr error) {
 	log := ctrl.LoggerFrom(ctx)
 	// If the MachineSetPreflightChecks feature gate is disabled return early.
 	if !feature.Gates.Enabled(feature.MachineSetPreflightChecks) {
@@ -63,6 +64,13 @@ func (r *Reconciler) runPreflightChecks(ctx context.Context, cluster *clusterv1.
 		return ctrl.Result{}, nil
 	}
 
+	defer func() {
+		// Mark the condition as false if failed to perform preflight checks.
+		if retErr != nil {
+			conditions.MarkFalse(ms, clusterv1.MachinesCreatedCondition, clusterv1.PreflightCheckFailedReason, clusterv1.ConditionSeverityError, retErr.Error())
+		}
+	}()
+
 	// Get the control plane object.
 	controlPlane, err := external.Get(ctx, r.Client, cluster.Spec.ControlPlaneRef, cluster.Namespace)
 	if err != nil {
@@ -134,7 +142,9 @@ func (r *Reconciler) runPreflightChecks(ctx context.Context, cluster *clusterv1.
 		for _, v := range preflightCheckErrs {
 			preflightCheckErrStrings = append(preflightCheckErrStrings, *v)
 		}
-		log.Info(fmt.Sprintf("While performing %q preflight checks failed: %s", action, strings.Join(preflightCheckErrStrings, ",")))
+		msg := fmt.Sprintf("While performing %q preflight checks failed: %s", action, strings.Join(preflightCheckErrStrings, ", "))
+		log.Info(msg)
+		conditions.MarkFalse(ms, clusterv1.MachinesCreatedCondition, clusterv1.PreflightCheckFailedReason, clusterv1.ConditionSeverityError, msg)
 		return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, nil
 	}
 	return ctrl.Result{}, nil

diff --git a/internal/controllers/machineset/machineset_preflight_test.go b/internal/controllers/machineset/machineset_preflight_test.go
@@ -33,6 +33,7 @@ import (
 	"sigs.k8s.io/cluster-api/feature"
 	"sigs.k8s.io/cluster-api/internal/contract"
 	"sigs.k8s.io/cluster-api/internal/test/builder"
+	"sigs.k8s.io/cluster-api/util/conditions"
 )
 
 func TestMachineSetReconciler_runPreflightChecks(t *testing.T) {
@@ -502,9 +503,21 @@ func TestMachineSetReconciler_runPreflightChecks(t *testing.T) {
 				result, err := r.runPreflightChecks(ctx, tt.cluster, tt.machineSet, "")
 				if tt.wantErr {
 					g.Expect(err).To(HaveOccurred())
+					g.Expect(conditions.Has(tt.machineSet, clusterv1.MachinesCreatedCondition)).To(BeTrue())
+					machineCreatedCondition := conditions.Get(tt.machineSet, clusterv1.MachinesCreatedCondition)
+					g.Expect(conditions.IsFalse(tt.machineSet, clusterv1.MachinesCreatedCondition)).To(BeTrue())
+					g.Expect(machineCreatedCondition.Reason).To(Equal(clusterv1.PreflightCheckFailedReason))
 				} else {
 					g.Expect(err).NotTo(HaveOccurred())
 					g.Expect(result.IsZero()).To(Equal(tt.wantPass))
+					if !tt.wantPass {
+						g.Expect(conditions.Has(tt.machineSet, clusterv1.MachinesCreatedCondition)).To(BeTrue())
+						machineCreatedCondition := conditions.Get(tt.machineSet, clusterv1.MachinesCreatedCondition)
+						g.Expect(conditions.IsFalse(tt.machineSet, clusterv1.MachinesCreatedCondition)).To(BeTrue())
+						g.Expect(machineCreatedCondition.Reason).To(Equal(clusterv1.PreflightCheckFailedReason))
+					} else {
+						g.Expect(conditions.Has(tt.machineSet, clusterv1.MachinesCreatedCondition)).To(BeFalse())
+					}
 				}
 			})
 		}