Remove unecessary ensureOwnerRef code on infraMachines

kubernetes-sigs · Oct 24, 2023 · c9e9e2d · c9e9e2d
1 parent 6928512
commit c9e9e2d
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 42 deletions.
diff --git a/exp/internal/controllers/machinepool_controller_phases.go b/exp/internal/controllers/machinepool_controller_phases.go
@@ -376,20 +376,16 @@ func (r *MachinePoolReconciler) reconcileMachines(ctx context.Context, mp *expv1
 		return err
 	}
 
-	updatedMachines, err := r.createOrUpdateMachines(ctx, mp, machineList.Items, infraMachineList.Items)
+	err := r.createOrUpdateMachines(ctx, mp, machineList.Items, infraMachineList.Items)
 	if err != nil {
 		return errors.Wrapf(err, "failed to create machines for MachinePool %q in namespace %q", mp.Name, mp.Namespace)
 	}
 
-	if err := r.ensureInfraMachineOnwerRefs(ctx, updatedMachines, infraMachineList.Items); err != nil {
-		return errors.Wrapf(err, "failed to create machines for MachinePool %q in namespace %q", mp.Name, mp.Namespace)
-	}
-
 	return nil
 }
 
 // createOrUpdateMachines creates a MachinePool Machine for each infraMachine if it doesn't already exist and sets the owner reference and infraRef.
-func (r *MachinePoolReconciler) createOrUpdateMachines(ctx context.Context, mp *expv1.MachinePool, machines []clusterv1.Machine, infraMachines []unstructured.Unstructured) ([]clusterv1.Machine, error) {
+func (r *MachinePoolReconciler) createOrUpdateMachines(ctx context.Context, mp *expv1.MachinePool, machines []clusterv1.Machine, infraMachines []unstructured.Unstructured) error {
 	log := ctrl.LoggerFrom(ctx)
 
 	// Construct a set of names of infraMachines that already have a Machine.
@@ -400,7 +396,6 @@ func (r *MachinePoolReconciler) createOrUpdateMachines(ctx context.Context, mp *
 	}
 
 	createdMachines := []clusterv1.Machine{}
-	existingMachines := []clusterv1.Machine{}
 	var errs []error
 	for i := range infraMachines {
 		infraMachine := &infraMachines[i]
@@ -414,7 +409,6 @@ func (r *MachinePoolReconciler) createOrUpdateMachines(ctx context.Context, mp *
 				errs = append(errs, errors.Wrapf(err, "failed to update Machine %q", klog.KObj(desiredMachine)))
 			}
 
-			existingMachines = append(existingMachines, *desiredMachine)
 		} else {
 			// Otherwise create a new Machine for the infraMachine.
 			log.Info("Creating new Machine for infraMachine", infraMachine.GroupVersionKind().Kind, klog.KObj(infraMachine))
@@ -429,39 +423,11 @@ func (r *MachinePoolReconciler) createOrUpdateMachines(ctx context.Context, mp *
 			createdMachines = append(createdMachines, *machine)
 		}
 	}
-	existingMachines = append(existingMachines, createdMachines...)
 	if err := r.waitForMachineCreation(ctx, createdMachines); err != nil {
 		errs = append(errs, errors.Wrapf(err, "failed to wait for machines to be created"))
 	}
 	if len(errs) > 0 {
-		return nil, kerrors.NewAggregate(errs)
-	}
-
-	return existingMachines, nil
-}
-
-// ensureInfraMachineOnwerRefs sets the ownerReferences on the each infraMachine to its associated MachinePool Machine if it hasn't already been done.
-func (r *MachinePoolReconciler) ensureInfraMachineOnwerRefs(ctx context.Context, updatedMachines []clusterv1.Machine, infraMachines []unstructured.Unstructured) error {
-	log := ctrl.LoggerFrom(ctx)
-	infraMachineNameToMachine := make(map[string]clusterv1.Machine)
-	for _, machine := range updatedMachines {
-		infraRef := machine.Spec.InfrastructureRef
-		infraMachineNameToMachine[infraRef.Name] = machine
-	}
-
-	for i := range infraMachines {
-		infraMachine := &infraMachines[i]
-		machine, ok := infraMachineNameToMachine[infraMachine.GetName()]
-		if !ok {
-			return errors.Errorf("failed to patch ownerRef for infraMachine %q because no Machine has an infraRef pointing to it", infraMachine.GetName())
-		}
-
-		log.V(2).Info("Setting ownerRef on infraMachine", "infraMachine", infraMachine.GetName(), "namespace", infraMachine.GetNamespace(), "machine", machine.GetName())
-		if err := controllerutil.SetControllerReference(&machine, infraMachine, r.Client.Scheme()); err != nil {
-			return err
-		}
-
-		log.V(4).Info("Successfully set ownerRef on infraMachine", "infraMachine", infraMachine.GetName(), "namespace", infraMachine.GetNamespace(), "machine", machine.GetName())
+		return kerrors.NewAggregate(errs)
 	}
 
 	return nil

diff --git a/exp/internal/controllers/machinepool_controller_phases_test.go b/exp/internal/controllers/machinepool_controller_phases_test.go
@@ -39,7 +39,7 @@ import (
 	"sigs.k8s.io/cluster-api/controllers/remote"
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
 	"sigs.k8s.io/cluster-api/internal/test/builder"
-	"sigs.k8s.io/cluster-api/util"
+	"sigs.k8s.io/cluster-api/internal/util/ssa"
 	"sigs.k8s.io/cluster-api/util/kubeconfig"
 	"sigs.k8s.io/cluster-api/util/labels/format"
 )
@@ -1312,7 +1312,8 @@ func TestReconcileMachinePoolMachines(t *testing.T) {
 			}
 
 			r := &MachinePoolReconciler{
-				Client: fake.NewClientBuilder().WithObjects(objs...).Build(),
+				Client:   fake.NewClientBuilder().WithObjects(objs...).Build(),
+				ssaCache: ssa.NewCache(),
 			}
 
 			err := r.reconcileMachines(ctx, tc.machinepool, infraConfig)
@@ -1335,10 +1336,8 @@ func TestReconcileMachinePoolMachines(t *testing.T) {
 					g.Expect(machineList.Items).To(HaveLen(len(tc.infraMachines)))
 					for i := range machineList.Items {
 						machine := &machineList.Items[i]
-						infraMachine, err := external.Get(ctx, r.Client, &machine.Spec.InfrastructureRef, machine.Namespace)
+						_, err := external.Get(ctx, r.Client, &machine.Spec.InfrastructureRef, machine.Namespace)
 						g.Expect(err).ToNot(HaveOccurred())
-
-						g.Expect(util.IsControlledBy(infraMachine, machine)).To(BeTrue())
 					}
 				} else {
 					g.Expect(machineList.Items).To(BeEmpty())