Add the WithOwnedV1Beta2Conditions option to the patch helper

api/v1beta1/machinedeployment_types.go

@@ -88,7 +88,7 @@ const (
 	// field of the MachineDeployment is not set.
 	MachineDeploymentAvailableWaitingForReplicasSetV1Beta2Reason = WaitingForReplicasSetV1Beta2Reason
 
-	// 	MachineDeploymentAvailableWaitingForAvailableReplicasSetV1Beta2Reason surfaces when the .status.v1beta2.availableReplicas
+	// MachineDeploymentAvailableWaitingForAvailableReplicasSetV1Beta2Reason surfaces when the .status.v1beta2.availableReplicas
 	// field of the MachineDeployment is not set.
 	MachineDeploymentAvailableWaitingForAvailableReplicasSetV1Beta2Reason = "WaitingForAvailableReplicasSet"
 
@@ -130,7 +130,7 @@ const (
 
 // MachineDeployment's ScalingUp condition and corresponding reasons that will be used in v1Beta2 API version.
 const (
-	// MachineDeploymentScalingUpV1Beta2Condition is true if available replicas < desired replicas.
+	// MachineDeploymentScalingUpV1Beta2Condition is true if actual replicas < desired replicas.
 	MachineDeploymentScalingUpV1Beta2Condition = ScalingUpV1Beta2Condition
 
 	// MachineDeploymentScalingUpV1Beta2Reason surfaces when actual replicas < desired replicas.
@@ -149,7 +149,7 @@ const (
 
 // MachineDeployment's ScalingDown condition and corresponding reasons that will be used in v1Beta2 API version.
 const (
-	// MachineDeploymentScalingDownV1Beta2Condition is true if replicas > desired replicas.
+	// MachineDeploymentScalingDownV1Beta2Condition is true if actual replicas > desired replicas.
 	MachineDeploymentScalingDownV1Beta2Condition = ScalingDownV1Beta2Condition
 
 	// MachineDeploymentScalingDownV1Beta2Reason surfaces when actual replicas > desired replicas.
@@ -168,7 +168,7 @@ const (
 
 // MachineDeployment's Remediating condition and corresponding reasons that will be used in v1Beta2 API version.
 const (
-	// MachineDeploymentRemediatingV1Beta2Condition details about ongoing remediation of the controlled machines, if any.
+	// MachineDeploymentRemediatingV1Beta2Condition surfaces details about ongoing remediation of the controlled machines, if any.
 	MachineDeploymentRemediatingV1Beta2Condition = RemediatingV1Beta2Condition
 
 	// MachineDeploymentRemediatingV1Beta2Reason surfaces when the MachineDeployment has at least one machine with HealthCheckSucceeded set to false

api/v1beta1/v1beta2_condition_consts.go

@@ -90,7 +90,7 @@ const (
 	// AvailableV1Beta2Reason applies to a condition surfacing object availability.
 	AvailableV1Beta2Reason = "Available"
 
-	// NotAvailableV1Beta2Reason applies to a condition surfacing object not satisfying availability per-requisites.
+	// NotAvailableV1Beta2Reason applies to a condition surfacing object not satisfying availability criteria.
 	NotAvailableV1Beta2Reason = "NotAvailable"
 
 	// ScalingUpV1Beta2Reason surfaces when an object is scaling up.

controlplane/kubeadm/internal/controllers/status.go

@@ -406,6 +406,10 @@ func setRemediatingCondition(ctx context.Context, kcp *controlplanev1.KubeadmCon
 }
 
 func aggregateStaleMachines(machines collections.Machines) string {
+	if len(machines) == 0 {
+		return ""
+	}
+
 	machineNames := []string{}
 	for _, machine := range machines {
 		if !machine.GetDeletionTimestamp().IsZero() && time.Since(machine.GetDeletionTimestamp().Time) > time.Minute*30 {
@@ -436,6 +440,10 @@ func aggregateStaleMachines(machines collections.Machines) string {
 }
 
 func aggregateUnhealthyMachines(machines collections.Machines) string {
+	if len(machines) == 0 {
+		return ""
+	}
+
 	machineNames := []string{}
 	for _, machine := range machines {
 		machineNames = append(machineNames, machine.GetName())

internal/controllers/machinedeployment/machinedeployment_controller.go

@@ -116,7 +116,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 	return nil
 }
 
-func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
+func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (retres ctrl.Result, reterr error) {
 	log := ctrl.LoggerFrom(ctx)
 
 	// Fetch the MachineDeployment instance.
@@ -173,6 +173,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 		if err := patchMachineDeployment(ctx, patchHelper, deployment, patchOpts...); err != nil {
 			reterr = kerrors.NewAggregate([]error{reterr, err})
 		}
+
+		if reterr != nil {
+			retres = ctrl.Result{}
+		}
 	}()
 
 	// Handle deletion reconciliation loop.
@@ -188,7 +192,9 @@ type scope struct {
 	cluster                                      *clusterv1.Cluster
 	machineSets                                  []*clusterv1.MachineSet
 	bootstrapTemplateNotFound                    bool
+	bootstrapTemplateExists                      bool
 	infrastructureTemplateNotFound               bool
+	infrastructureTemplateExists                 bool
 	getAndAdoptMachineSetsForDeploymentSucceeded bool
 }
 
@@ -206,6 +212,15 @@ func patchMachineDeployment(ctx context.Context, patchHelper *patch.Helper, md *
 			clusterv1.ReadyCondition,
 			clusterv1.MachineDeploymentAvailableCondition,
 		}},
+		patch.WithOwnedV1Beta2Conditions{Conditions: []string{
+			clusterv1.MachineDeploymentAvailableV1Beta2Condition,
+			clusterv1.MachineDeploymentMachinesReadyV1Beta2Condition,
+			clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition,
+			clusterv1.MachineDeploymentScalingDownV1Beta2Condition,
+			clusterv1.MachineDeploymentScalingUpV1Beta2Condition,
+			clusterv1.MachineDeploymentRemediatingV1Beta2Condition,
+			clusterv1.MachineDeploymentDeletingV1Beta2Condition,
+		}},
 	)
 	return patchHelper.Patch(ctx, md, options...)
 }
@@ -280,8 +295,10 @@ func (r *Reconciler) reconcile(ctx context.Context, s *scope) error {
 		}
 	}
 
+	templateExists := s.infrastructureTemplateExists && (md.Spec.Template.Spec.Bootstrap.ConfigRef == nil || s.bootstrapTemplateExists)
+
 	if md.Spec.Paused {
-		return r.sync(ctx, md, s.machineSets)
+		return r.sync(ctx, md, s.machineSets, templateExists)
 	}
 
 	if md.Spec.Strategy == nil {
@@ -292,11 +309,11 @@ func (r *Reconciler) reconcile(ctx context.Context, s *scope) error {
 		if md.Spec.Strategy.RollingUpdate == nil {
 			return errors.Errorf("missing MachineDeployment settings for strategy type: %s", md.Spec.Strategy.Type)
 		}
-		return r.rolloutRolling(ctx, md, s.machineSets)
+		return r.rolloutRolling(ctx, md, s.machineSets, templateExists)
 	}
 
 	if md.Spec.Strategy.Type == clusterv1.OnDeleteMachineDeploymentStrategyType {
-		return r.rolloutOnDelete(ctx, md, s.machineSets)
+		return r.rolloutOnDelete(ctx, md, s.machineSets, templateExists)
 	}
 
 	return errors.Errorf("unexpected deployment strategy type: %s", md.Spec.Strategy.Type)
@@ -314,8 +331,6 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) error {
 		return nil
 	}
 
-	log.Info("Waiting for MachineSets to be deleted", "MachineSets", clog.ObjNamesString(s.machineSets))
-
 	// else delete owned machinesets.
 	for _, ms := range s.machineSets {
 		if ms.DeletionTimestamp.IsZero() {
@@ -326,6 +341,7 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) error {
 		}
 	}
 
+	log.Info("Waiting for MachineSets to be deleted", "MachineSets", clog.ObjNamesString(s.machineSets))
 	return nil
 }
 
@@ -470,6 +486,8 @@ func (r *Reconciler) getTemplatesAndSetOwner(ctx context.Context, s *scope) erro
 			return err
 		}
 		s.infrastructureTemplateNotFound = true
+	} else {
+		s.infrastructureTemplateExists = true
 	}
 	// Make sure to reconcile the external bootstrap reference, if any.
 	if md.Spec.Template.Spec.Bootstrap.ConfigRef != nil {
@@ -478,6 +496,8 @@ func (r *Reconciler) getTemplatesAndSetOwner(ctx context.Context, s *scope) erro
 				return err
 			}
 			s.bootstrapTemplateNotFound = true
+		} else {
+			s.bootstrapTemplateExists = true
 		}
 	}
 	return nil

internal/controllers/machinedeployment/machinedeployment_rolling.go

@@ -29,8 +29,8 @@ import (
 )
 
 // rolloutRolling implements the logic for rolling a new MachineSet.
-func (r *Reconciler) rolloutRolling(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet) error {
-	newMS, oldMSs, err := r.getAllMachineSetsAndSyncRevision(ctx, md, msList, true)
+func (r *Reconciler) rolloutRolling(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet, templateExists bool) error {
+	newMS, oldMSs, err := r.getAllMachineSetsAndSyncRevision(ctx, md, msList, true, templateExists)
 	if err != nil {
 		return err
 	}

internal/controllers/machinedeployment/machinedeployment_rollout_ondelete.go

@@ -32,8 +32,8 @@ import (
 )
 
 // rolloutOnDelete implements the logic for the OnDelete MachineDeploymentStrategyType.
-func (r *Reconciler) rolloutOnDelete(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet) error {
-	newMS, oldMSs, err := r.getAllMachineSetsAndSyncRevision(ctx, md, msList, true)
+func (r *Reconciler) rolloutOnDelete(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet, templateExists bool) error {
+	newMS, oldMSs, err := r.getAllMachineSetsAndSyncRevision(ctx, md, msList, true, templateExists)
 	if err != nil {
 		return err
 	}

internal/controllers/machinedeployment/machinedeployment_status.go

@@ -37,35 +37,38 @@ import (
 
 func (r *Reconciler) updateStatus(ctx context.Context, s *scope) (retErr error) {
 	// Get all Machines controlled by this MachineDeployment.
-	var machines, machinesToBeRemediated, unHealthyMachines collections.Machines
+	var machines, machinesToBeRemediated, unhealthyMachines collections.Machines
+	var getMachinesSucceeded bool
 	if selectorMap, err := metav1.LabelSelectorAsMap(&s.machineDeployment.Spec.Selector); err == nil {
 		machineList := &clusterv1.MachineList{}
 		if err := r.Client.List(ctx, machineList, client.InNamespace(s.machineDeployment.Namespace), client.MatchingLabels(selectorMap)); err != nil {
 			retErr = errors.Wrap(err, "failed to list machines")
+		} else {
+			getMachinesSucceeded = true
+			machines = collections.FromMachineList(machineList)
+			machinesToBeRemediated = machines.Filter(collections.IsUnhealthyAndOwnerRemediated)
+			unhealthyMachines = machines.Filter(collections.IsUnhealthy)
 		}
-		machines = collections.FromMachineList(machineList)
-		machinesToBeRemediated = machines.Filter(collections.IsUnhealthyAndOwnerRemediated)
-		unHealthyMachines = machines.Filter(collections.IsUnhealthy)
 	} else {
 		retErr = errors.Wrap(err, "failed to convert label selector to a map")
 	}
 
-	// If the controller failed to read MachineSets, do not update replica counters.
-	if !s.getAndAdoptMachineSetsForDeploymentSucceeded {
+	// If the controller could read MachineSets, update replica counters.
+	if s.getAndAdoptMachineSetsForDeploymentSucceeded {
 		setReplicas(s.machineDeployment, s.machineSets)
 	}
 
 	setAvailableCondition(ctx, s.machineDeployment, s.getAndAdoptMachineSetsForDeploymentSucceeded)
 
 	setScalingUpCondition(ctx, s.machineDeployment, s.machineSets, s.bootstrapTemplateNotFound, s.infrastructureTemplateNotFound, s.getAndAdoptMachineSetsForDeploymentSucceeded)
-	setScalingDownCondition(ctx, s.machineDeployment, s.machineSets, machines, s.getAndAdoptMachineSetsForDeploymentSucceeded)
+	setScalingDownCondition(ctx, s.machineDeployment, s.machineSets, machines, s.getAndAdoptMachineSetsForDeploymentSucceeded, getMachinesSucceeded)
 
-	setMachinesReadyCondition(ctx, s.machineDeployment, machines)
-	setMachinesUpToDateCondition(ctx, s.machineDeployment, machines)
+	setMachinesReadyCondition(ctx, s.machineDeployment, machines, getMachinesSucceeded)
+	setMachinesUpToDateCondition(ctx, s.machineDeployment, machines, getMachinesSucceeded)
 
-	setRemediatingCondition(ctx, s.machineDeployment, machinesToBeRemediated, unHealthyMachines)
+	setRemediatingCondition(ctx, s.machineDeployment, machinesToBeRemediated, unhealthyMachines, getMachinesSucceeded)
 
-	setDeletingCondition(ctx, s.machineDeployment, s.machineSets, machines, s.getAndAdoptMachineSetsForDeploymentSucceeded)
+	setDeletingCondition(ctx, s.machineDeployment, s.machineSets, machines, s.getAndAdoptMachineSetsForDeploymentSucceeded, getMachinesSucceeded)
 
 	return retErr
 }
@@ -132,7 +135,7 @@ func setAvailableCondition(_ context.Context, machineDeployment *clusterv1.Machi
 
 	message := fmt.Sprintf("%d available replicas, at least %d required", *machineDeployment.Status.V1Beta2.AvailableReplicas, minReplicasNeeded)
 	if machineDeployment.Spec.Strategy != nil && mdutil.IsRollingUpdate(machineDeployment) && machineDeployment.Spec.Strategy.RollingUpdate != nil {
-		message += fmt.Sprintf(" (spec.strategy.rollout.maxUnavailable is %s)", machineDeployment.Spec.Strategy.RollingUpdate.MaxUnavailable)
+		message += fmt.Sprintf(" (spec.strategy.rollout.maxUnavailable is %s, spec.replicas is %d)", machineDeployment.Spec.Strategy.RollingUpdate.MaxUnavailable, *machineDeployment.Spec.Replicas)
 	}
 	v1beta2conditions.Set(machineDeployment, metav1.Condition{
 		Type:    clusterv1.MachineDeploymentAvailableV1Beta2Condition,
@@ -168,7 +171,7 @@ func setScalingUpCondition(_ context.Context, machineDeployment *clusterv1.Machi
 	if !machineDeployment.DeletionTimestamp.IsZero() {
 		desiredReplicas = 0
 	}
-	currentReplicas := mdutil.GetReplicaCountForMachineSets(machineSets)
+	currentReplicas := mdutil.GetActualReplicaCountForMachineSets(machineSets)
 
 	missingReferencesMessage := calculateMissingReferencesMessage(machineDeployment, bootstrapObjectNotFound, infrastructureObjectNotFound)
 
@@ -199,7 +202,7 @@ func setScalingUpCondition(_ context.Context, machineDeployment *clusterv1.Machi
 	})
 }
 
-func setScalingDownCondition(_ context.Context, machineDeployment *clusterv1.MachineDeployment, machineSets []*clusterv1.MachineSet, machines collections.Machines, getAndAdoptMachineSetsForDeploymentSucceeded bool) {
+func setScalingDownCondition(_ context.Context, machineDeployment *clusterv1.MachineDeployment, machineSets []*clusterv1.MachineSet, machines collections.Machines, getAndAdoptMachineSetsForDeploymentSucceeded, getMachinesSucceeded bool) {
 	// If we got unexpected errors in listing the machines sets (this should never happen), surface them.
 	if !getAndAdoptMachineSetsForDeploymentSucceeded {
 		v1beta2conditions.Set(machineDeployment, metav1.Condition{
@@ -225,14 +228,16 @@ func setScalingDownCondition(_ context.Context, machineDeployment *clusterv1.Mac
 	if !machineDeployment.DeletionTimestamp.IsZero() {
 		desiredReplicas = 0
 	}
-	currentReplicas := mdutil.GetReplicaCountForMachineSets(machineSets)
+	currentReplicas := mdutil.GetActualReplicaCountForMachineSets(machineSets)
 
 	// Scaling down.
 	if currentReplicas > desiredReplicas {
 		message := fmt.Sprintf("Scaling down from %d to %d replicas", currentReplicas, desiredReplicas)
-		staleMessage := aggregateStaleMachines(machines)
-		if staleMessage != "" {
-			message += fmt.Sprintf(" and %s", staleMessage)
+		if getMachinesSucceeded {
+			staleMessage := aggregateStaleMachines(machines)
+			if staleMessage != "" {
+				message += fmt.Sprintf(" and %s", staleMessage)
+			}
 		}
 		v1beta2conditions.Set(machineDeployment, metav1.Condition{
 			Type:    clusterv1.MachineDeploymentScalingDownV1Beta2Condition,
@@ -251,10 +256,10 @@ func setScalingDownCondition(_ context.Context, machineDeployment *clusterv1.Mac
 	})
 }
 
-func setMachinesReadyCondition(ctx context.Context, machineDeployment *clusterv1.MachineDeployment, machines collections.Machines) {
+func setMachinesReadyCondition(ctx context.Context, machineDeployment *clusterv1.MachineDeployment, machines collections.Machines, getMachinesSucceeded bool) {
 	log := ctrl.LoggerFrom(ctx)
 	// If we got unexpected errors in listing the machines (this should never happen), surface them.
-	if machines == nil {
+	if !getMachinesSucceeded {
 		v1beta2conditions.Set(machineDeployment, metav1.Condition{
 			Type:    clusterv1.MachineDeploymentMachinesReadyV1Beta2Condition,
 			Status:  metav1.ConditionUnknown,
@@ -291,10 +296,10 @@ func setMachinesReadyCondition(ctx context.Context, machineDeployment *clusterv1
 	v1beta2conditions.Set(machineDeployment, *readyCondition)
 }
 
-func setMachinesUpToDateCondition(ctx context.Context, machineDeployment *clusterv1.MachineDeployment, machines collections.Machines) {
+func setMachinesUpToDateCondition(ctx context.Context, machineDeployment *clusterv1.MachineDeployment, machines collections.Machines, getMachinesSucceeded bool) {
 	log := ctrl.LoggerFrom(ctx)
 	// If we got unexpected errors in listing the machines (this should never happen), surface them.
-	if machines == nil {
+	if !getMachinesSucceeded {
 		v1beta2conditions.Set(machineDeployment, metav1.Condition{
 			Type:    clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition,
 			Status:  metav1.ConditionUnknown,
@@ -331,8 +336,8 @@ func setMachinesUpToDateCondition(ctx context.Context, machineDeployment *cluste
 	v1beta2conditions.Set(machineDeployment, *upToDateCondition)
 }
 
-func setRemediatingCondition(ctx context.Context, machineDeployment *clusterv1.MachineDeployment, machinesToBeRemediated, unhealthyMachines collections.Machines) {
-	if machinesToBeRemediated == nil || unhealthyMachines == nil {
+func setRemediatingCondition(ctx context.Context, machineDeployment *clusterv1.MachineDeployment, machinesToBeRemediated, unhealthyMachines collections.Machines, getMachinesSucceeded bool) {
+	if !getMachinesSucceeded {
 		v1beta2conditions.Set(machineDeployment, metav1.Condition{
 			Type:    clusterv1.MachineDeploymentRemediatingV1Beta2Condition,
 			Status:  metav1.ConditionUnknown,
@@ -378,9 +383,9 @@ func setRemediatingCondition(ctx context.Context, machineDeployment *clusterv1.M
 	})
 }
 
-func setDeletingCondition(_ context.Context, machineDeployment *clusterv1.MachineDeployment, machineSets []*clusterv1.MachineSet, machines collections.Machines, getAndAdoptMachineSetsForDeploymentSucceeded bool) {
+func setDeletingCondition(_ context.Context, machineDeployment *clusterv1.MachineDeployment, machineSets []*clusterv1.MachineSet, machines collections.Machines, getAndAdoptMachineSetsForDeploymentSucceeded, getMachinesSucceeded bool) {
 	// If we got unexpected errors in listing the machines sets or machines (this should never happen), surface them.
-	if !getAndAdoptMachineSetsForDeploymentSucceeded || machines == nil {
+	if !getAndAdoptMachineSetsForDeploymentSucceeded || !getMachinesSucceeded {
 		v1beta2conditions.Set(machineDeployment, metav1.Condition{
 			Type:    clusterv1.MachineDeploymentDeletingV1Beta2Condition,
 			Status:  metav1.ConditionUnknown,
@@ -401,7 +406,11 @@ func setDeletingCondition(_ context.Context, machineDeployment *clusterv1.Machin
 
 	message := ""
 	if len(machines) > 0 {
-		message = fmt.Sprintf("Deleting %d replicas", len(machines))
+		if len(machines) == 1 {
+			message = fmt.Sprintf("Deleting %d Machine", len(machines))
+		} else {
+			message = fmt.Sprintf("Deleting %d Machines", len(machines))
+		}
 		staleMessage := aggregateStaleMachines(machines)
 		if staleMessage != "" {
 			message += fmt.Sprintf(" and %s", staleMessage)
@@ -413,7 +422,7 @@ func setDeletingCondition(_ context.Context, machineDeployment *clusterv1.Machin
 	}
 	v1beta2conditions.Set(machineDeployment, metav1.Condition{
 		Type:    clusterv1.MachineDeploymentDeletingV1Beta2Condition,
-		Status:  metav1.ConditionFalse,
+		Status:  metav1.ConditionTrue,
 		Reason:  clusterv1.MachineDeploymentDeletingDeletionTimestampSetV1Beta2Reason,
 		Message: message,
 	})
@@ -474,6 +483,10 @@ func aggregateStaleMachines(machines collections.Machines) string {
 }
 
 func aggregateUnhealthyMachines(machines collections.Machines) string {
+	if len(machines) == 0 {
+		return ""
+	}
+
 	machineNames := []string{}
 	for _, machine := range machines {
 		machineNames = append(machineNames, machine.GetName())
@@ -496,7 +509,7 @@ func aggregateUnhealthyMachines(machines collections.Machines) string {
 	} else {
 		message += " are "
 	}
-	message += "not healthy (not to be remediated by KCP)"
+	message += "not healthy (not to be remediated by MachineDeployment/MachineSet)"
 
 	return message
 }