Skip to content

Commit

Permalink
fix comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Yuvaraj Kakaraparthi committed May 14, 2023
1 parent 6b6231a commit 397a2d7
Showing 1 changed file with 8 additions and 9 deletions.
17 changes: 8 additions & 9 deletions internal/controllers/machineset/machineset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,10 +318,9 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster,
if !machine.DeletionTimestamp.IsZero() {
continue
}
// TODO(ykakarap): Consider running preflight checks before deleting the machines so that we dont end up deleting the machines without being able to create new ones.
if conditions.IsFalse(machine, clusterv1.MachineOwnerRemediatedCondition) {
// Delete the machines only if the pre-flight checks have passed. Do not delete machine when we cannot
// create new machines.
// Delete the machines only if the pre-flight checks have passed. Do not delete machines if we cannot
// guarantee creating new machines.
if !preFlightChecksResult.IsZero() {
result = util.LowestNonZeroResult(result, preFlightChecksResult)
break
Expand Down Expand Up @@ -484,12 +483,12 @@ func (r *Reconciler) syncReplicas(ctx context.Context, cluster *clusterv1.Cluste
}
}

// TODO(ykakarap): Run pre-flight checks to ensure that we are good to create a new machine.
// TODO(ykakarap): Drop this comment.
// Question: Would it be better to simply return and not throw an error? Throwing an error would cause the MS to go into exponential backoff. Is that okay?
// Prior Art: We do not throw error if pre-flight checks fail in KCP. We requeue.
result, err := r.runPreFlightChecks(ctx, cluster, ms)
if err != nil {
return ctrl.Result{}, errors.Wrap(err, "pre-flight checks failed")
return ctrl.Result{}, err
}
if !result.IsZero() {
return result, nil
Expand Down Expand Up @@ -1042,7 +1041,7 @@ func (r *Reconciler) runControlPlanePreFlightChecks(ctx context.Context, cluster
return ctrl.Result{}, errors.Wrapf(err, "failed to check if ControlPlane %s is provisioning", cpKlogRef)
}
if isProvisioning {
log.Info(fmt.Sprintf("ControlPlane %s is provisioning", cpKlogRef))
log.Info(fmt.Sprintf("Pre-flight check failed: ControlPlane %s is provisioning", cpKlogRef))
return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, nil
}

Expand All @@ -1052,7 +1051,7 @@ func (r *Reconciler) runControlPlanePreFlightChecks(ctx context.Context, cluster
return ctrl.Result{}, errors.Wrapf(err, "failed to check if the ControlPlane %s is upgrading", cpKlogRef)
}
if isUpgrading {
log.Info(fmt.Sprintf("ControlPlane %s is upgrading", cpKlogRef))
log.Info(fmt.Sprintf("Pre-flight check failed: ControlPlane %s is upgrading", cpKlogRef))
return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, nil
}

Expand All @@ -1076,7 +1075,7 @@ func (r *Reconciler) runControlPlanePreFlightChecks(ctx context.Context, cluster
// => MS minor version cannot be greater than the Control Plane minor version.
// Kubernetes skew policy: https://kubernetes.io/releases/version-skew-policy/#kubelet
if msSemver.Major > cpSemver.Major || msSemver.Minor > cpSemver.Minor {
log.Info(fmt.Sprintf("MachineSet version (%s) and ControlPlane version (%s) do not conform to the kubernetes version skew policy", msVersion, *cpVersion))
log.Info(fmt.Sprintf("Pre-flight check failed: MachineSet version (%s) and ControlPlane version (%s) do not conform to the kubernetes version skew policy", msVersion, *cpVersion))
return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, nil
}

Expand All @@ -1088,7 +1087,7 @@ func (r *Reconciler) runControlPlanePreFlightChecks(ctx context.Context, cluster
ms.Spec.Template.Spec.Bootstrap.ConfigRef.Kind == "KubeadmConfigTemplate"
if kubeadmBootstrapProviderUsed {
if *cpVersion != msVersion {
log.Info(fmt.Sprintf("MachineSet version (%s) and ControlPlane version (%s) do not conform to kubeadm version skew policy: kubeadm bootstrap provider only supports joining with the same version as the control plane", msVersion, *cpVersion))
log.Info(fmt.Sprintf("Pre-flight check 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 version as the control plane", msVersion, *cpVersion))
return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, nil
}
}
Expand Down

0 comments on commit 397a2d7

Please sign in to comment.