diff --git a/config/crd/bases/cluster.x-k8s.io_machinepools.yaml b/config/crd/bases/cluster.x-k8s.io_machinepools.yaml index bd06d84e53cf..9fcd47760f3f 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinepools.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinepools.yaml @@ -1053,52 +1053,6 @@ spec: pointer to distinguish between explicit zero and not specified. format: int32 type: integer - selector: - description: 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. - properties: - matchExpressions: - description: matchExpressions is a list of label selector requirements. - The requirements are ANDed. - items: - description: A label selector requirement is a selector that - contains values, a key, and an operator that relates the key - and values. - properties: - key: - description: key is the label key that the selector applies - to. - type: string - operator: - description: operator represents a key's relationship to - a set of values. Valid operators are In, NotIn, Exists - and DoesNotExist. - type: string - values: - description: values is an array of string values. If the - operator is In or NotIn, the values array must be non-empty. - If the operator is Exists or DoesNotExist, the values - array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - required: - - key - - operator - type: object - type: array - matchLabels: - additionalProperties: - type: string - description: matchLabels is a map of {key,value} pairs. A single - {key,value} in the matchLabels map is equivalent to an element - of matchExpressions, whose key field is "key", the operator - is "In", and the values array contains only "value". The requirements - are ANDed. - type: object - type: object template: description: Template describes the machines that will be created. properties: @@ -1279,7 +1233,6 @@ spec: type: object required: - clusterName - - selector - template type: object status: diff --git a/exp/api/v1alpha3/conversion.go b/exp/api/v1alpha3/conversion.go index b1eccab89345..8f6903b81abc 100644 --- a/exp/api/v1alpha3/conversion.go +++ b/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 } diff --git a/exp/api/v1alpha3/zz_generated.conversion.go b/exp/api/v1alpha3/zz_generated.conversion.go index ba418bd028a6..19ae36da3b85 100644 --- a/exp/api/v1alpha3/zz_generated.conversion.go +++ b/exp/api/v1alpha3/zz_generated.conversion.go @@ -163,7 +163,6 @@ func autoConvert_v1alpha3_MachinePoolSpec_To_v1beta1_MachinePoolSpec(in *Machine func autoConvert_v1beta1_MachinePoolSpec_To_v1alpha3_MachinePoolSpec(in *v1beta1.MachinePoolSpec, out *MachinePoolSpec, s conversion.Scope) error { out.ClusterName = in.ClusterName out.Replicas = (*int32)(unsafe.Pointer(in.Replicas)) - // WARNING: in.Selector requires manual conversion: does not exist in peer-type if err := apiv1alpha3.Convert_v1beta1_MachineTemplateSpec_To_v1alpha3_MachineTemplateSpec(&in.Template, &out.Template, s); err != nil { return err } diff --git a/exp/api/v1alpha4/conversion.go b/exp/api/v1alpha4/conversion.go index d66548e85829..fa9d49368d8d 100644 --- a/exp/api/v1alpha4/conversion.go +++ b/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 } diff --git a/exp/api/v1alpha4/zz_generated.conversion.go b/exp/api/v1alpha4/zz_generated.conversion.go index 8e2afb6e08d5..1c2ef9cc44f4 100644 --- a/exp/api/v1alpha4/zz_generated.conversion.go +++ b/exp/api/v1alpha4/zz_generated.conversion.go @@ -177,7 +177,6 @@ func Convert_v1alpha4_MachinePoolSpec_To_v1beta1_MachinePoolSpec(in *MachinePool func autoConvert_v1beta1_MachinePoolSpec_To_v1alpha4_MachinePoolSpec(in *v1beta1.MachinePoolSpec, out *MachinePoolSpec, s conversion.Scope) error { out.ClusterName = in.ClusterName out.Replicas = (*int32)(unsafe.Pointer(in.Replicas)) - // WARNING: in.Selector requires manual conversion: does not exist in peer-type if err := apiv1alpha4.Convert_v1beta1_MachineTemplateSpec_To_v1alpha4_MachineTemplateSpec(&in.Template, &out.Template, s); err != nil { return err } diff --git a/exp/api/v1beta1/machinepool_types.go b/exp/api/v1beta1/machinepool_types.go index 7f4d0a1792e2..3d08de8260be 100644 --- a/exp/api/v1beta1/machinepool_types.go +++ b/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"` diff --git a/exp/api/v1beta1/zz_generated.deepcopy.go b/exp/api/v1beta1/zz_generated.deepcopy.go index 12427e28af55..164680e08999 100644 --- a/exp/api/v1beta1/zz_generated.deepcopy.go +++ b/exp/api/v1beta1/zz_generated.deepcopy.go @@ -95,7 +95,6 @@ func (in *MachinePoolSpec) DeepCopyInto(out *MachinePoolSpec) { *out = new(int32) **out = **in } - in.Selector.DeepCopyInto(&out.Selector) in.Template.DeepCopyInto(&out.Template) if in.MinReadySeconds != nil { in, out := &in.MinReadySeconds, &out.MinReadySeconds diff --git a/internal/controllers/topology/cluster/blueprint.go b/internal/controllers/topology/cluster/blueprint.go index f2d27db62331..e853d418aaae 100644 --- a/internal/controllers/topology/cluster/blueprint.go +++ b/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) } diff --git a/internal/controllers/topology/cluster/current_state.go b/internal/controllers/topology/cluster/current_state.go index 4b77cd225076..8a35a5c6f4ad 100644 --- a/internal/controllers/topology/cluster/current_state.go +++ b/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})) } diff --git a/internal/controllers/topology/cluster/desired_state.go b/internal/controllers/topology/cluster/desired_state.go index 365f459fdac7..8bee67755d03 100644 --- a/internal/controllers/topology/cluster/desired_state.go +++ b/internal/controllers/topology/cluster/desired_state.go @@ -955,8 +955,8 @@ func computeMachinePool(_ context.Context, s *scope.Scope, desiredControlPlaneSt currentInfraMachineTemplateRef = ¤tMachinePool.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 diff --git a/internal/controllers/topology/cluster/patches/engine.go b/internal/controllers/topology/cluster/patches/engine.go index 36731df5b04b..54bfe9cb0b55 100644 --- a/internal/controllers/topology/cluster/patches/engine.go +++ b/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 } } diff --git a/internal/controllers/topology/cluster/patches/inline/json_patch_generator.go b/internal/controllers/topology/cluster/patches/inline/json_patch_generator.go index 33b13037c0e7..c924265e1114 100644 --- a/internal/controllers/topology/cluster/patches/inline/json_patch_generator.go +++ b/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") { diff --git a/internal/controllers/topology/cluster/patches/variables/variables.go b/internal/controllers/topology/cluster/patches/variables/variables.go index 6b467cc81d2b..ae16173d7d10 100644 --- a/internal/controllers/topology/cluster/patches/variables/variables.go +++ b/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"` } diff --git a/internal/controllers/topology/cluster/scope/blueprint.go b/internal/controllers/topology/cluster/scope/blueprint.go index d5c32c3af23c..1f93ba84258e 100644 --- a/internal/controllers/topology/cluster/scope/blueprint.go +++ b/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. diff --git a/internal/controllers/topology/cluster/scope/scope.go b/internal/controllers/topology/cluster/scope/scope.go index 93b5d579b720..9ef41844d0f7 100644 --- a/internal/controllers/topology/cluster/scope/scope.go +++ b/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(), } diff --git a/internal/controllers/topology/cluster/scope/state.go b/internal/controllers/topology/cluster/scope/state.go index 5356ef69b667..245e032fec76 100644 --- a/internal/controllers/topology/cluster/scope/state.go +++ b/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 }