Skip to content

Commit

Permalink
MachinesCreated condition on MachineSet should show failed preflight …
Browse files Browse the repository at this point in the history
…checks
  • Loading branch information
Yuvaraj Kakaraparthi committed May 20, 2023
1 parent c21ccfb commit 1fdd045
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 6 deletions.
4 changes: 4 additions & 0 deletions api/v1beta1/condition_consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
23 changes: 17 additions & 6 deletions internal/controllers/machineset/machineset_preflight.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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)
}
Expand All @@ -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)
}
}
Expand All @@ -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())

Expand All @@ -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
}

Expand All @@ -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.
Expand All @@ -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}
}

Expand Down Expand Up @@ -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
}
}
Expand Down
10 changes: 10 additions & 0 deletions internal/controllers/machineset/machineset_preflight_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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())
}
})
}
})
Expand Down

0 comments on commit 1fdd045

Please sign in to comment.