Remove infraSelector in CAPD

kubernetes-sigs · Jun 28, 2023 · 7644767 · 7644767
1 parent 19467e8
commit 7644767
Show file tree

Hide file tree

Showing 9 changed files with 20 additions and 78 deletions.
diff --git a/exp/internal/controllers/machinepool_controller_phases.go b/exp/internal/controllers/machinepool_controller_phases.go
@@ -469,9 +469,10 @@ func (r *MachinePoolReconciler) ensureInfraMachineOnwerRefs(ctx context.Context,
 		if !util.HasOwnerRef(ownerRefs, *machineRef) {
 			log.V(2).Info("Setting ownerRef on infraMachine", "infraMachine", infraMachine.GetName(), "namespace", infraMachine.GetNamespace(), "machine", machine.GetName())
 			ownerRefs = util.EnsureOwnerRef(ownerRefs, *machineRef)
-			infraMachine.SetOwnerReferences(ownerRefs)
+			infraMachineCopy := infraMachine.DeepCopy()
+			infraMachineCopy.SetOwnerReferences(ownerRefs)
 
-			if err := r.Client.Update(ctx, infraMachine); err != nil {
+			if err := r.Client.Patch(ctx, infraMachineCopy, client.MergeFrom(infraMachine)); err != nil {
 				return errors.Wrapf(err, "failed to patch ownerRef for infraMachine %q", infraMachine.GetName())
 			}
 		}

diff --git a/...structure/docker/config/crd/bases/infrastructure.cluster.x-k8s.io_dockermachinepools.yaml b/...structure/docker/config/crd/bases/infrastructure.cluster.x-k8s.io_dockermachinepools.yaml
diff --git a/test/infrastructure/docker/exp/api/v1alpha3/conversion.go b/test/infrastructure/docker/exp/api/v1alpha3/conversion.go
@@ -38,7 +38,6 @@ func (src *DockerMachinePool) ConvertTo(dstRaw conversion.Hub) error {
 	}
 
 	dst.Status.InfrastructureMachineKind = restored.Status.InfrastructureMachineKind
-	dst.Status.InfrastructureMachineSelector = restored.Status.InfrastructureMachineSelector
 
 	return nil
 }

diff --git a/test/infrastructure/docker/exp/api/v1alpha3/zz_generated.conversion.go b/test/infrastructure/docker/exp/api/v1alpha3/zz_generated.conversion.go
diff --git a/test/infrastructure/docker/exp/api/v1alpha4/conversion.go b/test/infrastructure/docker/exp/api/v1alpha4/conversion.go
@@ -38,7 +38,6 @@ func (src *DockerMachinePool) ConvertTo(dstRaw conversion.Hub) error {
 	}
 
 	dst.Status.InfrastructureMachineKind = restored.Status.InfrastructureMachineKind
-	dst.Status.InfrastructureMachineSelector = restored.Status.InfrastructureMachineSelector
 
 	return nil
 }

diff --git a/test/infrastructure/docker/exp/api/v1alpha4/zz_generated.conversion.go b/test/infrastructure/docker/exp/api/v1alpha4/zz_generated.conversion.go
diff --git a/test/infrastructure/docker/exp/api/v1beta1/dockermachinepool_types.go b/test/infrastructure/docker/exp/api/v1beta1/dockermachinepool_types.go
@@ -26,9 +26,6 @@ import (
 const (
 	// MachinePoolFinalizer allows ReconcileDockerMachinePool to clean up resources.
 	MachinePoolFinalizer = "dockermachinepool.infrastructure.cluster.x-k8s.io"
-
-	// DockerMachinePoolNameLabel is the label indicating the name of the DockerMachinePool a DockerMachine is controleld by.
-	DockerMachinePoolNameLabel = "dockermachinepool.infrastructure.cluster.x-k8s.io/pool-name"
 )
 
 // DockerMachinePoolMachineTemplate defines the desired state of DockerMachine.
@@ -86,11 +83,6 @@ type DockerMachinePoolStatus struct {
 	// +optional
 	Conditions clusterv1.Conditions `json:"conditions,omitempty"`
 
-	// InfrastructureMachineSelector is a label query over the infrastructure resources behind MachinePool Machines.
-	// More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors
-	// +optional
-	InfrastructureMachineSelector metav1.LabelSelector `json:"infrastructureMachineSelector,omitempty"`
-
 	// InfrastructureMachineKind is the kind of the infrastructure resources behind MachinePool Machines.
 	// +optional
 	InfrastructureMachineKind string `json:"infrastructureMachineKind,omitempty"`

diff --git a/test/infrastructure/docker/exp/api/v1beta1/zz_generated.deepcopy.go b/test/infrastructure/docker/exp/api/v1beta1/zz_generated.deepcopy.go
diff --git a/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go b/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go
@@ -112,12 +112,6 @@ func (r *DockerMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		return ctrl.Result{}, err
 	}
 
-	dockerMachinePool.Status.InfrastructureMachineSelector = metav1.LabelSelector{
-		MatchLabels: map[string]string{
-			clusterv1.ClusterNameLabel:            cluster.Name,
-			infraexpv1.DockerMachinePoolNameLabel: dockerMachinePool.Name,
-		},
-	}
 	dockerMachinePool.Status.InfrastructureMachineKind = "DockerMachine"
 	// Patch now so that the status and selectors are available.
 	if err := patchHelper.Patch(ctx, dockerMachinePool); err != nil {
@@ -188,7 +182,7 @@ func (r *DockerMachinePoolReconciler) SetupWithManager(ctx context.Context, mgr
 func (r *DockerMachinePoolReconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Cluster, machinePool *expv1.MachinePool, dockerMachinePool *infraexpv1.DockerMachinePool) (ctrl.Result, error) {
 	_ = ctrl.LoggerFrom(ctx)
 
-	dockerMachineList, err := getDockerMachines(ctx, r.Client, *dockerMachinePool)
+	dockerMachineList, err := getDockerMachines(ctx, r.Client, *cluster, *machinePool, *dockerMachinePool)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -225,7 +219,7 @@ func (r *DockerMachinePoolReconciler) reconcileNormal(ctx context.Context, clust
 		machinePool.Spec.Replicas = pointer.Int32(1)
 	}
 
-	dockerMachineList, err := getDockerMachines(ctx, r.Client, *dockerMachinePool)
+	dockerMachineList, err := getDockerMachines(ctx, r.Client, *cluster, *machinePool, *dockerMachinePool)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -263,13 +257,13 @@ func (r *DockerMachinePoolReconciler) reconcileNormal(ctx context.Context, clust
 	}
 
 	// Delete all DockerMachines that are not in the list of instances returned by the node pool.
-	if err := r.DeleteOrphanedDockerMachines(ctx, dockerMachinePool, nodePoolInstancesResult); err != nil {
+	if err := r.DeleteOrphanedDockerMachines(ctx, cluster, machinePool, dockerMachinePool, nodePoolInstancesResult); err != nil {
 		conditions.MarkFalse(dockerMachinePool, clusterv1.ReadyCondition, "FailedToDeleteOrphanedMachines", clusterv1.ConditionSeverityWarning, err.Error())
 		return ctrl.Result{}, errors.Wrap(err, "failed to delete orphaned machines")
 	}
 
 	// Create a DockerMachine for each instance returned by the node pool if it doesn't exist.
-	if err := r.CreateDockerMachinesIfNotExists(ctx, machinePool, dockerMachinePool, nodePoolInstancesResult); err != nil {
+	if err := r.CreateDockerMachinesIfNotExists(ctx, cluster, machinePool, dockerMachinePool, nodePoolInstancesResult); err != nil {
 		conditions.MarkFalse(dockerMachinePool, clusterv1.ReadyCondition, "FailedToCreateNewMachines", clusterv1.ConditionSeverityWarning, err.Error())
 		return ctrl.Result{}, errors.Wrap(err, "failed to create missing machines")
 	}
@@ -301,9 +295,12 @@ func (r *DockerMachinePoolReconciler) reconcileNormal(ctx context.Context, clust
 	return res, nil
 }
 
-func getDockerMachines(ctx context.Context, c client.Client, dockerMachinePool infraexpv1.DockerMachinePool) (*infrav1.DockerMachineList, error) {
+func getDockerMachines(ctx context.Context, c client.Client, cluster clusterv1.Cluster, machinePool expv1.MachinePool, dockerMachinePool infraexpv1.DockerMachinePool) (*infrav1.DockerMachineList, error) {
 	dockerMachineList := &infrav1.DockerMachineList{}
-	labels := dockerMachinePool.Status.InfrastructureMachineSelector.MatchLabels
+	labels := map[string]string{
+		clusterv1.ClusterNameLabel:     cluster.Name,
+		clusterv1.MachinePoolNameLabel: machinePool.Name,
+	}
 	if err := c.List(ctx, dockerMachineList, client.InNamespace(dockerMachinePool.Namespace), client.MatchingLabels(labels)); err != nil {
 		return nil, err
 	}
@@ -312,10 +309,10 @@ func getDockerMachines(ctx context.Context, c client.Client, dockerMachinePool i
 }
 
 // DeleteOrphanedDockerMachines deletes any DockerMachines owned by the DockerMachinePool that reference an invalid providerID, i.e. not in the latest copy of the node pool instances.
-func (r *DockerMachinePoolReconciler) DeleteOrphanedDockerMachines(ctx context.Context, dockerMachinePool *infraexpv1.DockerMachinePool, instances []docker.NodePoolInstance) error {
+func (r *DockerMachinePoolReconciler) DeleteOrphanedDockerMachines(ctx context.Context, cluster *clusterv1.Cluster, machinePool *expv1.MachinePool, dockerMachinePool *infraexpv1.DockerMachinePool, instances []docker.NodePoolInstance) error {
 	log := ctrl.LoggerFrom(ctx)
 	log.V(2).Info("Deleting orphaned machines", "dockerMachinePool", dockerMachinePool.Name, "namespace", dockerMachinePool.Namespace, "instances", instances)
-	dockerMachineList, err := getDockerMachines(ctx, r.Client, *dockerMachinePool)
+	dockerMachineList, err := getDockerMachines(ctx, r.Client, *cluster, *machinePool, *dockerMachinePool)
 	if err != nil {
 		return err
 	}
@@ -343,12 +340,12 @@ func (r *DockerMachinePoolReconciler) DeleteOrphanedDockerMachines(ctx context.C
 }
 
 // CreateDockerMachinesIfNotExists creates a DockerMachine for each instance returned by the node pool if it doesn't exist.
-func (r *DockerMachinePoolReconciler) CreateDockerMachinesIfNotExists(ctx context.Context, machinePool *expv1.MachinePool, dockerMachinePool *infraexpv1.DockerMachinePool, instances []docker.NodePoolInstance) error {
+func (r *DockerMachinePoolReconciler) CreateDockerMachinesIfNotExists(ctx context.Context, cluster *clusterv1.Cluster, machinePool *expv1.MachinePool, dockerMachinePool *infraexpv1.DockerMachinePool, instances []docker.NodePoolInstance) error {
 	log := ctrl.LoggerFrom(ctx)
 
 	log.V(2).Info("Creating missing machines", "dockerMachinePool", dockerMachinePool.Name, "namespace", dockerMachinePool.Namespace, "instances", instances)
 
-	dockerMachineList, err := getDockerMachines(ctx, r.Client, *dockerMachinePool)
+	dockerMachineList, err := getDockerMachines(ctx, r.Client, *cluster, *machinePool, *dockerMachinePool)
 	if err != nil {
 		return err
 	}
@@ -363,7 +360,10 @@ func (r *DockerMachinePoolReconciler) CreateDockerMachinesIfNotExists(ctx contex
 			continue
 		}
 
-		labels := dockerMachinePool.Status.InfrastructureMachineSelector.MatchLabels
+		labels := map[string]string{
+			clusterv1.ClusterNameLabel:     cluster.Name,
+			clusterv1.MachinePoolNameLabel: machinePool.Name,
+		}
 		labels[clusterv1.MachinePoolNameLabel] = machinePool.Name
 		dockerMachine := &infrav1.DockerMachine{
 			ObjectMeta: metav1.ObjectMeta{