From 1fdd04589ff91e3d0a57d01e63bb0ef803f5ada8 Mon Sep 17 00:00:00 2001 From: Yuvaraj Kakaraparthi Date: Sat, 20 May 2023 00:59:15 -0700 Subject: [PATCH] MachinesCreated condition on MachineSet should show failed preflight checks --- api/v1beta1/condition_consts.go | 4 ++++ .../machineset/machineset_preflight.go | 23 ++++++++++++++----- .../machineset/machineset_preflight_test.go | 10 ++++++++ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/api/v1beta1/condition_consts.go b/api/v1beta1/condition_consts.go index c0c5de162efa..e2b00062d29e 100644 --- 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 index 2e42e18e5828..3e73b169326a 100644 --- a/internal/controllers/machineset/machineset_preflight.go +++ b/internal/controllers/machineset/machineset_preflight.go @@ -37,13 +37,14 @@ import ( "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/contract" "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/cluster-api/util/conditions" ) // preflightFailedRequeueAfter is used to requeue the MachineSet to re-verify the preflight checks if // the preflight checks fail. const preflightFailedRequeueAfter = 15 * time.Second -func (r *Reconciler) runPreflightChecks(ctx context.Context, cluster *clusterv1.Cluster, ms *clusterv1.MachineSet) (ctrl.Result, error) { +func (r *Reconciler) runPreflightChecks(ctx context.Context, cluster *clusterv1.Cluster, ms *clusterv1.MachineSet) (result ctrl.Result, retErr error) { // If the MachineSetPreflightChecks feature gate is disabled return early. if !feature.Gates.Enabled(feature.MachineSetPreflightChecks) { return ctrl.Result{}, nil @@ -60,6 +61,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 { @@ -82,12 +90,11 @@ func (r *Reconciler) runPreflightChecks(ctx context.Context, cluster *clusterv1. return ctrl.Result{}, errors.Wrapf(err, "failed to perform preflight checks: failed to parse ControlPlane version %s", *cpVersion) } - result := ctrl.Result{} errList := []error{} // Run the control-plane-stable preflight check. if !skipped.Has(clusterv1.MachineSetPreflightCheckControlPlaneStable) { - if checkResult, err := r.controlPlaneStablePreflightCheck(ctx, controlPlane); err != nil || !checkResult.IsZero() { + if checkResult, err := r.controlPlaneStablePreflightCheck(ctx, controlPlane, ms); err != nil || !checkResult.IsZero() { result = util.LowestNonZeroResult(result, checkResult) errList = append(errList, err) } @@ -103,7 +110,7 @@ func (r *Reconciler) runPreflightChecks(ctx context.Context, cluster *clusterv1. // Run the kubernetes-version skew preflight check. if !skipped.Has(clusterv1.MachineSetPreflightCheckKubernetesVersion) { - if checkResult := r.kubernetesVersionPreflightCheck(ctx, cpSemver, msSemver); !checkResult.IsZero() { + if checkResult := r.kubernetesVersionPreflightCheck(ctx, cpSemver, msSemver, ms); !checkResult.IsZero() { result = util.LowestNonZeroResult(result, checkResult) } } @@ -120,7 +127,7 @@ func (r *Reconciler) runPreflightChecks(ctx context.Context, cluster *clusterv1. return result, errors.Wrap(kerrors.NewAggregate(errList), "failed to perform preflight checks") } -func (r *Reconciler) controlPlaneStablePreflightCheck(ctx context.Context, controlPlane *unstructured.Unstructured) (ctrl.Result, error) { +func (r *Reconciler) controlPlaneStablePreflightCheck(ctx context.Context, controlPlane *unstructured.Unstructured, ms *clusterv1.MachineSet) (ctrl.Result, error) { log := ctrl.LoggerFrom(ctx) cpKlogRef := klog.KRef(controlPlane.GetNamespace(), controlPlane.GetName()) @@ -131,6 +138,7 @@ func (r *Reconciler) controlPlaneStablePreflightCheck(ctx context.Context, contr } if isProvisioning { log.Info(fmt.Sprintf("Preflight check %q failed: ControlPlane %s is provisioning", clusterv1.MachineSetPreflightCheckControlPlaneStable, cpKlogRef)) + conditions.MarkFalse(ms, clusterv1.MachinesCreatedCondition, clusterv1.PreflightCheckFailedReason, clusterv1.ConditionSeverityError, "Control plane is provisioning") return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, nil } @@ -141,13 +149,14 @@ func (r *Reconciler) controlPlaneStablePreflightCheck(ctx context.Context, contr } if isUpgrading { log.Info(fmt.Sprintf("Preflight check %q failed: ControlPlane %s is upgrading", clusterv1.MachineSetPreflightCheckControlPlaneStable, cpKlogRef)) + conditions.MarkFalse(ms, clusterv1.MachinesCreatedCondition, clusterv1.PreflightCheckFailedReason, clusterv1.ConditionSeverityError, "Control plane is upgrading") return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, nil } return ctrl.Result{}, nil } -func (r *Reconciler) kubernetesVersionPreflightCheck(ctx context.Context, cpSemver semver.Version, msSemver semver.Version) ctrl.Result { +func (r *Reconciler) kubernetesVersionPreflightCheck(ctx context.Context, cpSemver semver.Version, msSemver semver.Version, ms *clusterv1.MachineSet) ctrl.Result { log := ctrl.LoggerFrom(ctx) // Check the Kubernetes version skew policy. @@ -161,6 +170,7 @@ func (r *Reconciler) kubernetesVersionPreflightCheck(ctx context.Context, cpSemv } if msSemver.GT(cpSemver) || msSemver.LT(floorVersion) { log.Info(fmt.Sprintf("Preflight check %q failed: MachineSet version (%s) and ControlPlane version (%s) do not conform to the kubernetes version skew policy", clusterv1.MachineSetPreflightCheckKubernetesVersion, msSemver.String(), cpSemver.String())) + conditions.MarkFalse(ms, clusterv1.MachinesCreatedCondition, clusterv1.PreflightCheckFailedReason, clusterv1.ConditionSeverityError, "Version does not conform to kubernetes version skew policy") return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter} } @@ -188,6 +198,7 @@ func (r *Reconciler) kubeadmVersionPreflightCheck(ctx context.Context, cpSemver if kubeadmBootstrapProviderUsed { if cpSemver.Major != msSemver.Major || cpSemver.Minor != msSemver.Minor { log.Info(fmt.Sprintf("Preflight check %q failed: MachineSet version (%s) and ControlPlane version (%s) do not conform to kubeadm version skew policy: kubeadm bootstrap provider only supports joining with the same (major+minor) version as the control plane", clusterv1.MachineSetPreflightCheckKubeadmVersion, msSemver.String(), cpSemver.String())) + conditions.MarkFalse(ms, clusterv1.MachinesCreatedCondition, clusterv1.PreflightCheckFailedReason, clusterv1.ConditionSeverityError, "Version does not conform to kubeadm version skew policy") return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, nil } } diff --git a/internal/controllers/machineset/machineset_preflight_test.go b/internal/controllers/machineset/machineset_preflight_test.go index 30d7350c3a97..db68e6ad08ea 100644 --- 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) { @@ -430,6 +431,15 @@ func TestMachineSetReconciler_runPreflightChecks(t *testing.T) { result, err := r.runPreflightChecks(ctx, tt.cluster, tt.machineSet) g.Expect(err).NotTo(HaveOccurred()) g.Expect(result.IsZero()).To(Equal(tt.wantPass)) + // Verify the MachineCreatedCondition is updated accordingly. + 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()) + } }) } })