More comments

Author: fabriziopandini

api/v1beta1/machine_types.go

@@ -104,7 +104,7 @@ const (
 
 	// MachineAvailableV1Beta2Reason surfaces when a machine is ready for at least MinReadySeconds.
 	// Note: MinReadySeconds is assumed 0 until it will be implemented in v1beta2 API.
-	MachineAvailableV1Beta2Reason = "MachineAvailable"
+	MachineAvailableV1Beta2Reason = AvailableV1Beta2Condition
 )
 
 // Machine's Ready condition and corresponding reasons that will be used in v1Beta2 API version.
@@ -124,7 +124,7 @@ const (
 // Note: UpToDate condition is set by the controller owning the machine.
 const (
 	// MachineUpToDateV1Beta2Condition is true if the Machine spec matches the spec of the Machine's owner resource, e.g. KubeadmControlPlane or MachineDeployment.
-	// The Machine's owner (e.g MachineDeployment) is authoritative to set their owned Machine's UpToDate conditions based on its current spec.
+	// The Machine's owner (e.g. MachineDeployment) is authoritative to set their owned Machine's UpToDate conditions based on its current spec.
 	MachineUpToDateV1Beta2Condition = "UpToDate"
 )
 
@@ -139,11 +139,11 @@ const (
 	MachineBootstrapDataSecretProvidedV1Beta2Reason = "DataSecretProvided"
 
 	// MachineBootstrapConfigInvalidConditionReportedV1Beta2Reason surfaces a BootstrapConfig Ready condition (read from a bootstrap config object) which is invalid.
-	// (e.g. it is status is missing).
-	MachineBootstrapConfigInvalidConditionReportedV1Beta2Reason = InvalidConditionReported
+	// (e.g. its status is missing).
+	MachineBootstrapConfigInvalidConditionReportedV1Beta2Reason = InvalidConditionReportedV1Beta2Reason
 
-	// MachineBootstrapConfigReadyNoV1Beta2ReasonReported applies to a BootstrapConfig Ready condition (read from a bootstrap config object) that reports no reason.
-	MachineBootstrapConfigReadyNoV1Beta2ReasonReported = NoV1Beta2ReasonReported
+	// MachineBootstrapConfigReadyNoReasonReportedV1Beta2Reason applies to a BootstrapConfig Ready condition (read from a bootstrap config object) that reports no reason.
+	MachineBootstrapConfigReadyNoReasonReportedV1Beta2Reason = NoV1Beta2ReasonReported
 
 	// MachineBootstrapConfigInternalErrorV1Beta2Reason surfaces unexpected failures when reading a BootstrapConfig object.
 	MachineBootstrapConfigInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason
@@ -165,11 +165,11 @@ const (
 	MachineInfrastructureReadyV1Beta2Condition = InfrastructureReadyV1Beta2Condition
 
 	// MachineInfrastructureInvalidConditionReportedV1Beta2Reason surfaces a infrastructure Ready condition (read from an infra machine object) which is invalid.
-	// (e.g. it is status is missing).
-	MachineInfrastructureInvalidConditionReportedV1Beta2Reason = InvalidConditionReported
+	// (e.g. its status is missing).
+	MachineInfrastructureInvalidConditionReportedV1Beta2Reason = InvalidConditionReportedV1Beta2Reason
 
-	// MachineInfrastructureReadyNoV1Beta2ReasonReported applies to a infrastructure Ready condition (read from an infra machine object) that reports no reason.
-	MachineInfrastructureReadyNoV1Beta2ReasonReported = NoV1Beta2ReasonReported
+	// MachineInfrastructureReadyNoReasonReportedV1Beta2Reason applies to a infrastructure Ready condition (read from an infra machine object) that reports no reason.
+	MachineInfrastructureReadyNoReasonReportedV1Beta2Reason = NoV1Beta2ReasonReported
 
 	// MachineInfrastructureInternalErrorV1Beta2Reason surfaces unexpected failures when reading a infra machine object.
 	MachineInfrastructureInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason
@@ -196,14 +196,14 @@ const (
 	// MachineNodeConditionNotYetReportedV1Beta2Reason surfaces when a Machine's Node doesn't have a condition reported yet.
 	MachineNodeConditionNotYetReportedV1Beta2Reason = "NodeConditionNotYetReported"
 
-	// MachineNodeNotFoundV1Beta2Reason surfaces when the node hosted on the machine cannot be found.
+	// MachineNodeDoesNotExistV1Beta2Reason surfaces when the node hosted on the machine does not exist.
 	// Note: this could happen when creating the machine. However, this state should be treated as an error if it lasts indefinitely.
-	MachineNodeNotFoundV1Beta2Reason = "NodeNotFound"
+	MachineNodeDoesNotExistV1Beta2Reason = ObjectDoesNotExistV1Beta2Reason
 
 	// MachineNodeDeletedV1Beta2Reason surfaces when  the node hosted on the machine has been deleted.
 	// Note: controllers can't identify if the Node was deleted by the controller itself, e.g.
 	// during the deletion workflow, or by a users.
-	MachineNodeDeletedV1Beta2Reason = "NodeDeleted"
+	MachineNodeDeletedV1Beta2Reason = ObjectDeletedV1Beta2Reason
 )
 
 // Machine's HealthCheckSucceeded and OwnerRemediated conditions and corresponding reasons that will be used in v1Beta2 API version.
@@ -228,12 +228,6 @@ const (
 const (
 	// MachinePausedV1Beta2Condition is true if the Machine or the Cluster it belongs to are paused.
 	MachinePausedV1Beta2Condition = PausedV1Beta2Condition
-
-	// MachineNotPausedV1Beta2Reason surfaces when a Machine is not paused.
-	MachineNotPausedV1Beta2Reason = NotPausedV1Beta2Reason
-
-	// MachineObjectPausedV1Beta2Reason surfaces when a Machine is paused.
-	MachineObjectPausedV1Beta2Reason = ObjectPausedV1Beta2Reason
 )
 
 // ANCHOR: MachineSpec

api/v1beta1/v1beta2_condition_consts.go

@@ -87,16 +87,16 @@ const (
 
 // Reasons that are used across different objects.
 const (
-	// InvalidConditionReported applies to a condition, usually read from an external object, that is invalid
-	// (e.g. it is status is missing).
-	InvalidConditionReported = "InvalidConditionReported"
+	// InvalidConditionReportedV1Beta2Reason applies to a condition, usually read from an external object, that is invalid
+	// (e.g. its status is missing).
+	InvalidConditionReportedV1Beta2Reason = "InvalidConditionReported"
 
 	// NoV1Beta2ReasonReported applies to a condition, usually read from an external object, that reports no reason.
 	// Note: this could happen e.g. when an external object still uses Cluster API v1beta1 Conditions.
 	NoV1Beta2ReasonReported = "NoReasonReported"
 
 	// InternalErrorV1Beta2Reason surfaces unexpected errors reporting by controllers.
-	// In most cases, it will be required to look at controllers logs to proper triage those issues.
+	// In most cases, it will be required to look at controllers logs to properly triage those issues.
 	InternalErrorV1Beta2Reason = "InternalError"
 
 	// ObjectDoesNotExistV1Beta2Reason surfaces when a referenced object does not exist.
@@ -110,11 +110,8 @@ const (
 	// NotPausedV1Beta2Reason surfaces when an object is not paused.
 	NotPausedV1Beta2Reason = "NotPaused"
 
-	// ClusterPausedV1Beta2Reason surfaces when an Cluster is paused.
-	ClusterPausedV1Beta2Reason = "ClusterPaused"
-
-	// ObjectPausedV1Beta2Reason surfaces when an object is paused.
-	ObjectPausedV1Beta2Reason = "ObjectPaused"
+	// PausedV1Beta2Reason surfaces when an object is paused.
+	PausedV1Beta2Reason = "Paused"
 )
 
 // Conditions that will be used for the MachineSet object in v1Beta2 API version.

internal/controllers/machine/machine_controller.go

@@ -878,7 +878,7 @@ func (r *Reconciler) reconcileDeleteBootstrap(ctx context.Context, s *scope) (bo
 		return true, nil
 	}
 
-	if s.bootstrapConfig != nil {
+	if s.bootstrapConfig != nil && s.bootstrapConfig.GetDeletionTimestamp().IsZero() {
 		if err := r.Client.Delete(ctx, s.bootstrapConfig); err != nil && !apierrors.IsNotFound(err) {
 			return false, errors.Wrapf(err,
 				"failed to delete %v %q for Machine %q in namespace %q",
@@ -895,7 +895,7 @@ func (r *Reconciler) reconcileDeleteInfrastructure(ctx context.Context, s *scope
 		return true, nil
 	}
 
-	if s.infraMachine != nil {
+	if s.infraMachine != nil && s.infraMachine.GetDeletionTimestamp().IsZero() {
 		if err := r.Client.Delete(ctx, s.infraMachine); err != nil && !apierrors.IsNotFound(err) {
 			return false, errors.Wrapf(err,
 				"failed to delete %v %q for Machine %q in namespace %q",

internal/controllers/machine/machine_controller_phases.go

@@ -49,7 +49,7 @@ var externalReadyWait = 30 * time.Second
 func (r *Reconciler) reconcileExternal(ctx context.Context, cluster *clusterv1.Cluster, m *clusterv1.Machine, ref *corev1.ObjectReference) (*unstructured.Unstructured, error) {
 	if err := utilconversion.UpdateReferenceAPIContract(ctx, r.Client, ref); err != nil {
 		if apierrors.IsNotFound(err) {
-			// We want to surface IsNotFound only for the referenced object, so we use a generic error in case CRD is not found.
+			// We want to surface the NotFound error only for the referenced object, so we use a generic error in case CRD is not found.
 			return nil, errors.New(err.Error())
 		}
 		return nil, err
@@ -180,6 +180,10 @@ func (r *Reconciler) reconcileBootstrap(ctx context.Context, s *scope) (ctrl.Res
 		fallBack,
 	)
 
+	if !s.bootstrapConfig.GetDeletionTimestamp().IsZero() {
+		return ctrl.Result{}, nil
+	}
+
 	// If the bootstrap provider is not ready, return.
 	if !ready {
 		log.Info("Waiting for bootstrap provider to generate data secret and report status.ready", s.bootstrapConfig.GetKind(), klog.KObj(s.bootstrapConfig))
@@ -233,10 +237,6 @@ func (r *Reconciler) reconcileInfrastructure(ctx context.Context, s *scope) (ctr
 	}
 	s.infraMachine = obj
 
-	if !s.infraMachine.GetDeletionTimestamp().IsZero() {
-		return ctrl.Result{}, nil
-	}
-
 	// Determine if the infrastructure provider is ready.
 	ready, err := external.IsReady(s.infraMachine)
 	if err != nil {
@@ -256,6 +256,10 @@ func (r *Reconciler) reconcileInfrastructure(ctx context.Context, s *scope) (ctr
 		fallBack,
 	)
 
+	if !s.infraMachine.GetDeletionTimestamp().IsZero() {
+		return ctrl.Result{}, nil
+	}
+
 	// If the infrastructure provider is not ready (and it wasn't ready before), return early.
 	if !ready && !m.Status.InfrastructureReady {
 		log.Info("Waiting for infrastructure provider to create machine infrastructure and report status.ready", s.infraMachine.GetKind(), klog.KObj(s.infraMachine))