Various review fixes

exp/api/v1alpha3/conversion.go

@@ -76,7 +76,6 @@ func (src *MachinePool) ConvertTo(dstRaw conversion.Hub) error {
 	}
 	dst.Spec.Template.Spec.NodeDeletionTimeout = restored.Spec.Template.Spec.NodeDeletionTimeout
 	dst.Spec.Template.Spec.NodeVolumeDetachTimeout = restored.Spec.Template.Spec.NodeVolumeDetachTimeout
-	dst.Spec.Selector = restored.Spec.Selector
 	return nil
 }
 

exp/api/v1alpha4/conversion.go

@@ -42,7 +42,6 @@ func (src *MachinePool) ConvertTo(dstRaw conversion.Hub) error {
 	}
 	dst.Spec.Template.Spec.NodeDeletionTimeout = restored.Spec.Template.Spec.NodeDeletionTimeout
 	dst.Spec.Template.Spec.NodeVolumeDetachTimeout = restored.Spec.Template.Spec.NodeVolumeDetachTimeout
-	dst.Spec.Selector = restored.Spec.Selector
 	return nil
 }
 

exp/api/v1beta1/machinepool_types.go

@@ -42,11 +42,6 @@ type MachinePoolSpec struct {
 	// +optional
 	Replicas *int32 `json:"replicas,omitempty"`
 
-	// Label selector for machines. Existing MachineSets whose machines are
-	// selected by this will be the ones affected by this deployment.
-	// It must match the machine template's labels.
-	Selector metav1.LabelSelector `json:"selector"`
-
 	// Template describes the machines that will be created.
 	Template clusterv1.MachineTemplateSpec `json:"template"`
 

internal/controllers/topology/cluster/blueprint.go

@@ -104,7 +104,7 @@ func (r *Reconciler) getBlueprint(ctx context.Context, cluster *clusterv1.Cluste
 		machinePoolClass.Template.Metadata.DeepCopyInto(&machinePoolBlueprint.Metadata)
 
 		// Get the infrastructure machine template.
-		machinePoolBlueprint.InfrastructureMachineTemplate, err = r.getReference(ctx, machinePoolClass.Template.Infrastructure.Ref)
+		machinePoolBlueprint.InfrastructureMachinePoolTemplate, err = r.getReference(ctx, machinePoolClass.Template.Infrastructure.Ref)
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to get infrastructure machine template for %s, MachinePool class %q", tlog.KObj{Obj: blueprint.ClusterClass}, machinePoolClass.Class)
 		}

internal/controllers/topology/cluster/current_state.go

@@ -344,7 +344,7 @@ func (r *Reconciler) getCurrentMachinePoolState(ctx context.Context, blueprintMa
 			if err != nil {
 				return nil, errors.Wrap(err, fmt.Sprintf("%s Bootstrap reference could not be retrieved", tlog.KObj{Obj: m}))
 			}
-			infraRef, err = alignRefAPIVersion(mpBluePrint.InfrastructureMachineTemplate, infraRef)
+			infraRef, err = alignRefAPIVersion(mpBluePrint.InfrastructureMachinePoolTemplate, infraRef)
 			if err != nil {
 				return nil, errors.Wrap(err, fmt.Sprintf("%s Infrastructure reference could not be retrieved", tlog.KObj{Obj: m}))
 			}

internal/controllers/topology/cluster/desired_state.go

@@ -955,8 +955,8 @@ func computeMachinePool(_ context.Context, s *scope.Scope, desiredControlPlaneSt
 		currentInfraMachineTemplateRef = &currentMachinePool.Object.Spec.Template.Spec.InfrastructureRef
 	}
 	desiredMachinePool.InfrastructureMachinePoolObject, err = templateToObject(templateToInput{
-		template:              machinePoolBlueprint.InfrastructureMachineTemplate,
-		templateClonedFromRef: contract.ObjToRef(machinePoolBlueprint.InfrastructureMachineTemplate),
+		template:              machinePoolBlueprint.InfrastructureMachinePoolTemplate,
+		templateClonedFromRef: contract.ObjToRef(machinePoolBlueprint.InfrastructureMachinePoolTemplate),
 		cluster:               s.Current.Cluster,
 		namePrefix:            infrastructureMachineTemplateNamePrefix(s.Current.Cluster.Name, machinePoolTopology.Name),
 		currentObjectRef:      currentInfraMachineTemplateRef,
@@ -1059,14 +1059,6 @@ func computeMachinePool(_ context.Context, s *scope.Scope, desiredControlPlaneSt
 	// Note: the labels in MachineSet are used to properly cleanup templates when the MachineSet is deleted.
 	desiredMachinePoolObj.Spec.Template.Labels = machinePoolLabels
 
-	// Set the selector with the subset of labels identifying controlled machines.
-	// NOTE: this prevents the web hook to add cluster.x-k8s.io/pool-name label, that is
-	// redundant for managed MachinePool given that we already have topology.cluster.x-k8s.io/pool-name.
-	desiredMachinePoolObj.Spec.Selector.MatchLabels = map[string]string{}
-	desiredMachinePoolObj.Spec.Selector.MatchLabels[clusterv1.ClusterNameLabel] = s.Current.Cluster.Name
-	desiredMachinePoolObj.Spec.Selector.MatchLabels[clusterv1.ClusterTopologyOwnedLabel] = ""
-	desiredMachinePoolObj.Spec.Selector.MatchLabels[clusterv1.ClusterTopologyMachinePoolNameLabel] = machinePoolTopology.Name
-
 	// Set the desired replicas.
 	desiredMachinePoolObj.Spec.Replicas = machinePoolTopology.Replicas
 

internal/controllers/topology/cluster/patches/engine.go

@@ -174,6 +174,10 @@ func addVariablesForPatch(blueprint *scope.ClusterBlueprint, desired *scope.Clus
 	for _, md := range desired.MachineDeployments {
 		mdStateIndex[md.Object.Name] = md
 	}
+	mpStateIndex := map[string]*scope.MachinePoolState{}
+	for _, mp := range desired.MachinePools {
+		mpStateIndex[mp.Object.Name] = mp
+	}
 	for i, item := range req.Items {
 		// If the item is a Control Plane add the Control Plane variables.
 		if item.HolderReference.FieldPath == "spec.controlPlaneRef" {
@@ -202,27 +206,7 @@ func addVariablesForPatch(blueprint *scope.ClusterBlueprint, desired *scope.Clus
 				return errors.Wrapf(err, "failed to calculate variables for %s", klog.KObj(md.Object))
 			}
 			item.Variables = mdVariables
-		}
-		req.Items[i] = item
-	}
-
-	mpStateIndex := map[string]*scope.MachinePoolState{}
-	for _, mp := range desired.MachinePools {
-		mpStateIndex[mp.Object.Name] = mp
-	}
-	for i, item := range req.Items {
-		// If the item is a Control Plane add the Control Plane variables.
-		if item.HolderReference.FieldPath == "spec.controlPlaneRef" {
-			item.Variables = controlPlaneVariables
-		}
-		// If the item holder reference is a Control Plane machine add the Control Plane variables.
-		if blueprint.HasControlPlaneInfrastructureMachine() &&
-			item.HolderReference.FieldPath == strings.Join(contract.ControlPlane().MachineTemplate().InfrastructureRef().Path(), ".") {
-			item.Variables = controlPlaneVariables
-		}
-		// If the item holder reference is a MachinePool calculate the variables for each MachinePoolTopology
-		// and add them to the variables for the MachinePool.
-		if item.HolderReference.Kind == "MachinePool" {
+		} else if item.HolderReference.Kind == "MachinePool" {
 			mp, ok := mpStateIndex[item.HolderReference.Name]
 			if !ok {
 				return errors.Errorf("could not find desired state for MachinePool %s", klog.KObj(mp.Object))
@@ -241,7 +225,6 @@ func addVariablesForPatch(blueprint *scope.ClusterBlueprint, desired *scope.Clus
 		}
 		req.Items[i] = item
 	}
-
 	return nil
 }
 
@@ -312,12 +295,11 @@ func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterStat
 	}
 
 	// Add BootstrapConfigTemplate and InfrastructureMachine template for all MachineDeploymentTopologies
-	// and MachinePoolTopologies in the Cluster.
+	// in the Cluster.
 	// NOTE: We intentionally iterate over MachineDeployment in the Cluster instead of over
 	// MachineDeploymentClasses in the ClusterClass because each MachineDeployment in a topology
 	// has its own state, e.g. version or replicas. This state is used to calculate builtin variables,
 	// which can then be used e.g. to compute the machine image for a specific Kubernetes version.
-	// The same applies to MachinePools.
 	for mdTopologyName, md := range desired.MachineDeployments {
 		// Lookup MachineDeploymentTopology definition from cluster.spec.topology.
 		mdTopology, err := lookupMDTopology(blueprint.Topology, mdTopologyName)
@@ -352,7 +334,7 @@ func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterStat
 		req.Items = append(req.Items, *t)
 	}
 
-	// Add BootstrapConfigTemplate and InfrastructureMachine template for all MachinePoolTopologies
+	// Add BootstrapConfigTemplate and InfrastructureMachinePoolTemplate for all MachinePoolTopologies
 	// in the Cluster.
 	// NOTE: We intentionally iterate over MachinePool in the Cluster instead of over
 	// MachinePoolClasses in the ClusterClass because each MachinePool in a topology
@@ -382,12 +364,12 @@ func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterStat
 		req.Items = append(req.Items, *t)
 
 		// Add the InfrastructureMachineTemplate.
-		t, err = newRequestItemBuilder(mpClass.InfrastructureMachineTemplate).
+		t, err = newRequestItemBuilder(mpClass.InfrastructureMachinePoolTemplate).
 			WithHolder(mp.Object, "spec.template.spec.infrastructureRef").
 			Build()
 		if err != nil {
-			return nil, errors.Wrapf(err, "failed to prepare InfrastructureMachine template %s for MachinePool topology %s for patching",
-				tlog.KObj{Obj: mpClass.InfrastructureMachineTemplate}, mpTopologyName)
+			return nil, errors.Wrapf(err, "failed to prepare InfrastructureMachinePoolTemplate %s for MachinePool topology %s for patching",
+				tlog.KObj{Obj: mpClass.InfrastructureMachinePoolTemplate}, mpTopologyName)
 		}
 		req.Items = append(req.Items, *t)
 	}
@@ -590,7 +572,7 @@ func updateDesiredState(ctx context.Context, req *runtimehooksv1.GeneratePatches
 	// Update the templates for all MachinePools.
 	for mpTopologyName, mp := range desired.MachinePools {
 		topologyName := requestTopologyName{mpTopologyName: mpTopologyName}
-		// Update the BootstrapConfigTemplate.
+		// Update the BootstrapConfig.
 		bootstrapTemplate, err := getTemplateAsUnstructured(req, "MachinePool", "spec.template.spec.bootstrap.configRef", topologyName)
 		if err != nil {
 			return err
@@ -599,12 +581,12 @@ func updateDesiredState(ctx context.Context, req *runtimehooksv1.GeneratePatches
 			return err
 		}
 
-		// Update the InfrastructureMachineTemplate.
-		infrastructureMachineTemplate, err := getTemplateAsUnstructured(req, "MachinePool", "spec.template.spec.infrastructureRef", topologyName)
+		// Update the InfrastructureMachinePool.
+		infrastructureMachinePoolTemplate, err := getTemplateAsUnstructured(req, "MachinePool", "spec.template.spec.infrastructureRef", topologyName)
 		if err != nil {
 			return err
 		}
-		if err := patchObject(ctx, mp.InfrastructureMachinePoolObject, infrastructureMachineTemplate); err != nil {
+		if err := patchObject(ctx, mp.InfrastructureMachinePoolObject, infrastructureMachinePoolTemplate); err != nil {
 			return err
 		}
 	}

internal/controllers/topology/cluster/patches/inline/json_patch_generator.go

@@ -155,8 +155,8 @@ func matchesSelector(req *runtimehooksv1.GeneratePatchesRequestItem, templateVar
 	}
 
 	// Check if the request is for a BootstrapConfigTemplate or an InfrastructureMachineTemplate
-	// of one of the configured MachineDeploymentClasses or MachinePoolClasses.
-	if selector.MatchResources.MachineDeploymentClass != nil || selector.MatchResources.MachinePoolClass != nil {
+	// of one of the configured MachineDeploymentClasses.
+	if selector.MatchResources.MachineDeploymentClass != nil {
 		// MachineDeployment.spec.template.spec.bootstrap.configRef or
 		// MachineDeployment.spec.template.spec.infrastructureRef holds the BootstrapConfigTemplate or
 		// InfrastructureMachineTemplate.
@@ -184,7 +184,11 @@ func matchesSelector(req *runtimehooksv1.GeneratePatchesRequestItem, templateVar
 				}
 			}
 		}
+	}
 
+	// Check if the request is for a BootstrapConfigTemplate or an InfrastructureMachineTemplate
+	// of one of the configured MachinePoolClasses.
+	if selector.MatchResources.MachinePoolClass != nil {
 		if req.HolderReference.Kind == "MachinePool" &&
 			(req.HolderReference.FieldPath == "spec.template.spec.bootstrap.configRef" ||
 				req.HolderReference.FieldPath == "spec.template.spec.infrastructureRef") {

internal/controllers/topology/cluster/patches/variables/variables.go

@@ -148,7 +148,7 @@ type MachineDeploymentBuiltins struct {
 	// Bootstrap is the value of the .spec.template.spec.bootstrap field of the MachineDeployment.
 	Bootstrap *MachineBootstrapBuiltins `json:"bootstrap,omitempty"`
 
-	// InfrastructureRef is the value of the .spec.template.spec.bootstrap field of the MachineDeployment.
+	// InfrastructureRef is the value of the .spec.template.spec.infrastructureRef field of the MachineDeployment.
 	InfrastructureRef *MachineInfrastructureRefBuiltins `json:"infrastructureRef,omitempty"`
 }
 
@@ -181,7 +181,7 @@ type MachinePoolBuiltins struct {
 	// Bootstrap is the value of the .spec.template.spec.bootstrap field of the MachinePool.
 	Bootstrap *MachineBootstrapBuiltins `json:"bootstrap,omitempty"`
 
-	// InfrastructureRef is the value of the .spec.template.spec.bootstrap field of the MachinePool.
+	// InfrastructureRef is the value of the .spec.template.spec.infrastructureRef field of the MachinePool.
 	InfrastructureRef *MachineInfrastructureRefBuiltins `json:"infrastructureRef,omitempty"`
 }
 

internal/controllers/topology/cluster/scope/blueprint.go

@@ -40,7 +40,7 @@ type ClusterBlueprint struct {
 	// MachineDeployments holds the MachineDeploymentBlueprints derived from ClusterClass.
 	MachineDeployments map[string]*MachineDeploymentBlueprint
 
-	// MachinePools holds the MachinePoolsBlueprints derived from ClusterClass.
+	// MachinePools holds the MachinePoolBlueprints derived from ClusterClass.
 	MachinePools map[string]*MachinePoolBlueprint
 }
 
@@ -76,7 +76,7 @@ type MachineDeploymentBlueprint struct {
 	MachineHealthCheck *clusterv1.MachineHealthCheckClass
 }
 
-// MachinePoolBlueprint holds the templates required for computing the desired state of a managed MachinePools;
+// MachinePoolBlueprint holds the templates required for computing the desired state of a managed MachinePool;
 // it also holds a copy of the MachinePool metadata from Cluster.Topology, thus providing all the required info
 // in a single place.
 type MachinePoolBlueprint struct {
@@ -87,8 +87,8 @@ type MachinePoolBlueprint struct {
 	// BootstrapTemplate holds the bootstrap template for a MachinePool referenced from ClusterClass.
 	BootstrapTemplate *unstructured.Unstructured
 
-	// InfrastructureMachineTemplate holds the infrastructure machine template for a MachinePool referenced from ClusterClass.
-	InfrastructureMachineTemplate *unstructured.Unstructured
+	// InfrastructureMachinePoolTemplate holds the infrastructure machine template for a MachinePool referenced from ClusterClass.
+	InfrastructureMachinePoolTemplate *unstructured.Unstructured
 }
 
 // HasControlPlaneInfrastructureMachine checks whether the clusterClass mandates the controlPlane has infrastructureMachines.

internal/controllers/topology/cluster/scope/scope.go

@@ -63,7 +63,7 @@ func New(cluster *clusterv1.Cluster) *Scope {
 		},
 		UpgradeTracker: NewUpgradeTracker(
 			MaxMDUpgradeConcurrency(maxMDUpgradeConcurrency),
-			MaxMDUpgradeConcurrency(maxMPUpgradeConcurrency),
+			MaxMPUpgradeConcurrency(maxMPUpgradeConcurrency),
 		),
 		HookResponseTracker: NewHookResponseTracker(),
 	}

internal/controllers/topology/cluster/scope/state.go

@@ -45,7 +45,7 @@ type ClusterState struct {
 	// MachineDeployments holds the machine deployments in the Cluster.
 	MachineDeployments MachineDeploymentsStateMap
 
-	// MachinePools holds the machine deployments in the Cluster.
+	// MachinePools holds the MachinePools in the Cluster.
 	MachinePools MachinePoolsStateMap
 }