Merge pull request #12528 from richardcase/mp_v1beta2_replicas

✨ feat: set new replica fields for machine pools

Author: k8s-ci-robot

exp/internal/controllers/machinepool_controller.go

@@ -49,6 +49,7 @@ import (
 	"sigs.k8s.io/cluster-api/util/conditions"
 	v1beta1conditions "sigs.k8s.io/cluster-api/util/conditions/deprecated/v1beta1"
 	"sigs.k8s.io/cluster-api/util/finalizers"
+	"sigs.k8s.io/cluster-api/util/labels/format"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/cluster-api/util/paused"
 	"sigs.k8s.io/cluster-api/util/predicates"
@@ -62,10 +63,8 @@ import (
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io;bootstrap.cluster.x-k8s.io,resources=*,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinepools;machinepools/status;machinepools/finalizers,verbs=get;list;watch;create;update;patch;delete
 
-var (
-	// machinePoolKind contains the schema.GroupVersionKind for the MachinePool type.
-	machinePoolKind = clusterv1.GroupVersion.WithKind("MachinePool")
-)
+// machinePoolKind contains the schema.GroupVersionKind for the MachinePool type.
+var machinePoolKind = clusterv1.GroupVersion.WithKind("MachinePool")
 
 const (
 	// MachinePoolControllerName defines the controller used when creating clients.
@@ -89,21 +88,6 @@ type MachinePoolReconciler struct {
 	predicateLog *logr.Logger
 }
 
-// scope holds the different objects that are read and used during the reconcile.
-type scope struct {
-	// cluster is the Cluster object the Machine belongs to.
-	// It is set at the beginning of the reconcile function.
-	cluster *clusterv1.Cluster
-
-	// machinePool is the MachinePool object. It is set at the beginning
-	// of the reconcile function.
-	machinePool *clusterv1.MachinePool
-
-	// nodeRefMapResult is a map of providerIDs to Nodes that are associated with the Cluster.
-	// It is set after reconcileInfrastructure is called.
-	nodeRefMap map[string]*corev1.Node
-}
-
 func (r *MachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
 	if r.Client == nil || r.APIReader == nil || r.ClusterCache == nil {
 		return errors.New("Client, APIReader and ClusterCache must not be nil")
@@ -189,7 +173,17 @@ func (r *MachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, err
 	}
 
+	// Handle normal reconciliation loop.
+	scope := &scope{
+		cluster:     cluster,
+		machinePool: mp,
+	}
+
 	defer func() {
+		if err := r.updateStatus(ctx, scope); err != nil {
+			reterr = kerrors.NewAggregate([]error{reterr, err})
+		}
+
 		r.reconcilePhase(mp)
 		// TODO(jpang): add support for metrics.
 
@@ -223,79 +217,69 @@ func (r *MachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		}
 	}()
 
-	// Reconcile labels.
-	if mp.Labels == nil {
-		mp.Labels = make(map[string]string)
+	alwaysReconcile := []machinePoolReconcileFunc{
+		wrapErrMachinePoolReconcileFunc(r.reconcileSetOwnerAndLabels, "failed to set MachinePool owner and labels"),
 	}
-	mp.Labels[clusterv1.ClusterNameLabel] = mp.Spec.ClusterName
 
 	// Handle deletion reconciliation loop.
 	if !mp.DeletionTimestamp.IsZero() {
-		return ctrl.Result{}, r.reconcileDelete(ctx, cluster, mp)
+		reconcileDelete := append(
+			alwaysReconcile,
+			wrapErrMachinePoolReconcileFunc(r.reconcileDelete, "failed to reconcile delete"),
+		)
+		return doReconcile(ctx, scope, reconcileDelete)
 	}
 
-	// Handle normal reconciliation loop.
-	scope := &scope{
-		cluster:     cluster,
-		machinePool: mp,
-	}
-	return r.reconcile(ctx, scope)
+	reconcileNormal := append(alwaysReconcile,
+		wrapErrMachinePoolReconcileFunc(r.reconcileBootstrap, "failed to reconcile bootstrap config"),
+		wrapErrMachinePoolReconcileFunc(r.reconcileInfrastructure, "failed to reconcile infrastructure"),
+		wrapErrMachinePoolReconcileFunc(r.getMachinesForMachinePool, "failed to get Machines for MachinePool"),
+		wrapErrMachinePoolReconcileFunc(r.reconcileNodeRefs, "failed to reconcile nodeRefs"),
+		wrapErrMachinePoolReconcileFunc(r.setMachinesUptoDate, "failed to set machines up to date"),
+	)
+
+	return doReconcile(ctx, scope, reconcileNormal)
 }
 
-func (r *MachinePoolReconciler) reconcile(ctx context.Context, s *scope) (ctrl.Result, error) {
-	// Ensure the MachinePool is owned by the Cluster it belongs to.
+func (r *MachinePoolReconciler) reconcileSetOwnerAndLabels(_ context.Context, s *scope) (ctrl.Result, error) {
 	cluster := s.cluster
 	mp := s.machinePool
+
+	if mp.Labels == nil {
+		mp.Labels = make(map[string]string)
+	}
+	mp.Labels[clusterv1.ClusterNameLabel] = mp.Spec.ClusterName
+
 	mp.SetOwnerReferences(util.EnsureOwnerRef(mp.GetOwnerReferences(), metav1.OwnerReference{
 		APIVersion: clusterv1.GroupVersion.String(),
 		Kind:       "Cluster",
 		Name:       cluster.Name,
 		UID:        cluster.UID,
 	}))
 
-	phases := []func(context.Context, *scope) (ctrl.Result, error){
-		r.reconcileBootstrap,
-		r.reconcileInfrastructure,
-		r.reconcileNodeRefs,
-	}
-
-	res := ctrl.Result{}
-	errs := []error{}
-	for _, phase := range phases {
-		// Call the inner reconciliation methods.
-		phaseResult, err := phase(ctx, s)
-		if err != nil {
-			errs = append(errs, err)
-		}
-		if len(errs) > 0 {
-			continue
-		}
-
-		res = util.LowestNonZeroResult(res, phaseResult)
-	}
-	return res, kerrors.NewAggregate(errs)
+	return ctrl.Result{}, nil
 }
 
 // reconcileDelete delete machinePool related resources.
-func (r *MachinePoolReconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Cluster, machinePool *clusterv1.MachinePool) error {
-	if ok, err := r.reconcileDeleteExternal(ctx, machinePool); !ok || err != nil {
+func (r *MachinePoolReconciler) reconcileDelete(ctx context.Context, s *scope) (ctrl.Result, error) {
+	if ok, err := r.reconcileDeleteExternal(ctx, s.machinePool); !ok || err != nil {
 		// Return early and don't remove the finalizer if we got an error or
 		// the external reconciliation deletion isn't ready.
-		return err
+		return ctrl.Result{}, fmt.Errorf("failed deleting external references: %s", err)
 	}
 
 	// check nodes delete timeout passed.
-	if !r.isMachinePoolNodeDeleteTimeoutPassed(machinePool) {
-		if err := r.reconcileDeleteNodes(ctx, cluster, machinePool); err != nil {
+	if !r.isMachinePoolNodeDeleteTimeoutPassed(s.machinePool) {
+		if err := r.reconcileDeleteNodes(ctx, s.cluster, s.machinePool); err != nil {
 			// Return early and don't remove the finalizer if we got an error.
-			return err
+			return ctrl.Result{}, fmt.Errorf("failed deleting nodes: %w", err)
 		}
 	} else {
 		ctrl.LoggerFrom(ctx).Info("NodeDeleteTimeout passed, skipping Nodes deletion")
 	}
 
-	controllerutil.RemoveFinalizer(machinePool, clusterv1.MachinePoolFinalizer)
-	return nil
+	controllerutil.RemoveFinalizer(s.machinePool, clusterv1.MachinePoolFinalizer)
+	return ctrl.Result{}, nil
 }
 
 // reconcileDeleteNodes delete the cluster nodes.
@@ -430,3 +414,110 @@ func (r *MachinePoolReconciler) nodeToMachinePool(ctx context.Context, o client.
 
 	return nil
 }
+
+func (r *MachinePoolReconciler) getMachinesForMachinePool(ctx context.Context, s *scope) (ctrl.Result, error) {
+	infraMachineSelector := metav1.LabelSelector{
+		MatchLabels: map[string]string{
+			clusterv1.MachinePoolNameLabel: format.MustFormatValue(s.machinePool.Name),
+			clusterv1.ClusterNameLabel:     s.machinePool.Spec.ClusterName,
+		},
+	}
+
+	allMachines := &clusterv1.MachineList{}
+	if err := r.Client.List(ctx,
+		allMachines,
+		client.InNamespace(s.machinePool.Namespace),
+		client.MatchingLabels(infraMachineSelector.MatchLabels)); err != nil {
+		return ctrl.Result{}, err
+	}
+
+	filteredMachines := make([]*clusterv1.Machine, 0, len(allMachines.Items))
+	for idx := range allMachines.Items {
+		machine := &allMachines.Items[idx]
+		if shouldExcludeMachine(s.machinePool, machine) {
+			continue
+		}
+		filteredMachines = append(filteredMachines, machine)
+	}
+
+	s.machines = filteredMachines
+
+	return ctrl.Result{}, nil
+}
+
+func (r *MachinePoolReconciler) setMachinesUptoDate(ctx context.Context, s *scope) (ctrl.Result, error) {
+	var errs []error
+	for _, machine := range s.machines {
+		patchHelper, err := patch.NewHelper(machine, r.Client)
+		if err != nil {
+			errs = append(errs, err)
+			continue
+		}
+
+		upToDateCondition := &metav1.Condition{
+			Type: clusterv1.MachineUpToDateCondition,
+		}
+
+		if machine.DeletionTimestamp.IsZero() {
+			upToDateCondition.Status = metav1.ConditionTrue
+			upToDateCondition.Reason = clusterv1.MachineUpToDateReason
+		} else {
+			upToDateCondition.Status = metav1.ConditionFalse
+			upToDateCondition.Reason = clusterv1.MachineNotUpToDateReason
+			upToDateCondition.Message = "Machine is being deleted"
+		}
+		conditions.Set(machine, *upToDateCondition)
+
+		if err := patchHelper.Patch(ctx, machine, patch.WithOwnedConditions{Conditions: []string{
+			clusterv1.MachineUpToDateCondition,
+		}}); err != nil {
+			errs = append(errs, err)
+			continue
+		}
+	}
+
+	if len(errs) > 0 {
+		return ctrl.Result{}, kerrors.NewAggregate(errs)
+	}
+
+	return ctrl.Result{}, nil
+}
+
+type machinePoolReconcileFunc func(ctx context.Context, s *scope) (ctrl.Result, error)
+
+func wrapErrMachinePoolReconcileFunc(f machinePoolReconcileFunc, msg string) machinePoolReconcileFunc {
+	return func(ctx context.Context, s *scope) (ctrl.Result, error) {
+		res, err := f(ctx, s)
+		return res, errors.Wrap(err, msg)
+	}
+}
+
+func doReconcile(ctx context.Context, s *scope, phases []machinePoolReconcileFunc) (ctrl.Result, kerrors.Aggregate) {
+	res := ctrl.Result{}
+	errs := []error{}
+	for _, phase := range phases {
+		// Call the inner reconciliation methods.
+		phaseResult, err := phase(ctx, s)
+		if err != nil {
+			errs = append(errs, err)
+		}
+		if len(errs) > 0 {
+			continue
+		}
+		res = util.LowestNonZeroResult(res, phaseResult)
+	}
+
+	if len(errs) > 0 {
+		return ctrl.Result{}, kerrors.NewAggregate(errs)
+	}
+
+	return res, nil
+}
+
+func shouldExcludeMachine(machinePool *clusterv1.MachinePool, machine *clusterv1.Machine) bool {
+	if metav1.GetControllerOf(machine) != nil && !metav1.IsControlledBy(machine, machinePool) {
+		return true
+	}
+
+	return false
+}

exp/internal/controllers/machinepool_controller_phases.go

@@ -276,6 +276,7 @@ func (r *MachinePoolReconciler) reconcileInfrastructure(ctx context.Context, s *
 		return ctrl.Result{}, err
 	}
 	infraConfig := infraReconcileResult.Result
+	s.infraMachinePool = infraConfig
 
 	if !infraConfig.GetDeletionTimestamp().IsZero() {
 		return ctrl.Result{}, nil