diff --git a/.golangci.yml b/.golangci.yml index 91ff591f8d7e..4ec860df78a1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -173,6 +173,8 @@ linters-settings: # CAPI utils - pkg: sigs.k8s.io/cluster-api/util/conditions/v1beta2 alias: v1beta2conditions + - pkg: sigs.k8s.io/cluster-api/internal/topology/names + alias: topologynames # CAPD - pkg: sigs.k8s.io/cluster-api/test/infrastructure/docker/api/v1alpha3 alias: infrav1alpha3 diff --git a/api/v1beta1/machinedeployment_types.go b/api/v1beta1/machinedeployment_types.go index 2828550efd6e..bc02d4d445e1 100644 --- a/api/v1beta1/machinedeployment_types.go +++ b/api/v1beta1/machinedeployment_types.go @@ -286,6 +286,11 @@ type MachineDeploymentSpec struct { // +optional Strategy *MachineDeploymentStrategy `json:"strategy,omitempty"` + // machineNamingStrategy allows changing the naming pattern used when creating Machines. + // Note: InfraMachines & BootstrapConfigs will use the same name as the corresponding Machines. + // +optional + MachineNamingStrategy *MachineNamingStrategy `json:"machineNamingStrategy,omitempty"` + // minReadySeconds is the minimum number of seconds for which a Node for a newly created machine should be ready before considering the replica available. // Defaults to 0 (machine will be considered available as soon as the Node is ready) // +optional @@ -412,6 +417,31 @@ type RemediationStrategy struct { // ANCHOR_END: RemediationStrategy +// MachineNamingStrategy allows changing the naming pattern used when creating +// Machines. +// Note: InfraMachines will use the same name as the corresponding Machines. +type MachineNamingStrategy struct { + // Template defines the template to use for generating the names of the + // Machine objects. + // If not defined, it will fallback to `{{ .machineSet.name }}-{{ .random }}`. + // If the generated name string exceeds 63 characters, it will be trimmed to + // 58 characters and will + // get concatenated with a random suffix of length 5. + // Length of the template string must not exceed 256 characters. + // The template allows the following variables `.cluster.name`, + // `.machineSet.name` and `.random`. + // The variable `.cluster.name` retrieves the name of the cluster object + // that owns the Machines being created. + // The variable `.machineSet.name` retrieves the name of the MachineSet + // object that owns the Machines being created. + // The variable `.random` is substituted with random alphanumeric string, + // without vowels, of length 5. This variable is required part of the + // template. If not provided, validation will fail. + // +optional + // +kubebuilder:validation:MaxLength=256 + Template string `json:"template,omitempty"` +} + // ANCHOR: MachineDeploymentStatus // MachineDeploymentStatus defines the observed state of MachineDeployment. diff --git a/api/v1beta1/machineset_types.go b/api/v1beta1/machineset_types.go index 1abbd906cd45..4d29aba02377 100644 --- a/api/v1beta1/machineset_types.go +++ b/api/v1beta1/machineset_types.go @@ -85,6 +85,11 @@ type MachineSetSpec struct { // Object references to custom resources are treated as templates. // +optional Template MachineTemplateSpec `json:"template,omitempty"` + + // machineNamingStrategy allows changing the naming pattern used when creating Machines. + // Note: InfraMachines & BootstrapConfigs will use the same name as the corresponding Machines. + // +optional + MachineNamingStrategy *MachineNamingStrategy `json:"machineNamingStrategy,omitempty"` } // MachineSet's ScalingUp condition and corresponding reasons that will be used in v1Beta2 API version. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 2f1db80c4688..2bbda4d4933f 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -1318,6 +1318,11 @@ func (in *MachineDeploymentSpec) DeepCopyInto(out *MachineDeploymentSpec) { *out = new(MachineDeploymentStrategy) (*in).DeepCopyInto(*out) } + if in.MachineNamingStrategy != nil { + in, out := &in.MachineNamingStrategy, &out.MachineNamingStrategy + *out = new(MachineNamingStrategy) + **out = **in + } if in.MinReadySeconds != nil { in, out := &in.MinReadySeconds, &out.MinReadySeconds *out = new(int32) @@ -1922,6 +1927,21 @@ func (in *MachineList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MachineNamingStrategy) DeepCopyInto(out *MachineNamingStrategy) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineNamingStrategy. +func (in *MachineNamingStrategy) DeepCopy() *MachineNamingStrategy { + if in == nil { + return nil + } + out := new(MachineNamingStrategy) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachinePoolClass) DeepCopyInto(out *MachinePoolClass) { *out = *in @@ -2193,6 +2213,11 @@ func (in *MachineSetSpec) DeepCopyInto(out *MachineSetSpec) { } in.Selector.DeepCopyInto(&out.Selector) in.Template.DeepCopyInto(&out.Template) + if in.MachineNamingStrategy != nil { + in, out := &in.MachineNamingStrategy, &out.MachineNamingStrategy + *out = new(MachineNamingStrategy) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineSetSpec. diff --git a/api/v1beta1/zz_generated.openapi.go b/api/v1beta1/zz_generated.openapi.go index 3167c70da4e7..13f3792cbab2 100644 --- a/api/v1beta1/zz_generated.openapi.go +++ b/api/v1beta1/zz_generated.openapi.go @@ -88,6 +88,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "sigs.k8s.io/cluster-api/api/v1beta1.MachineHealthCheckTopology": schema_sigsk8sio_cluster_api_api_v1beta1_MachineHealthCheckTopology(ref), "sigs.k8s.io/cluster-api/api/v1beta1.MachineHealthCheckV1Beta2Status": schema_sigsk8sio_cluster_api_api_v1beta1_MachineHealthCheckV1Beta2Status(ref), "sigs.k8s.io/cluster-api/api/v1beta1.MachineList": schema_sigsk8sio_cluster_api_api_v1beta1_MachineList(ref), + "sigs.k8s.io/cluster-api/api/v1beta1.MachineNamingStrategy": schema_sigsk8sio_cluster_api_api_v1beta1_MachineNamingStrategy(ref), "sigs.k8s.io/cluster-api/api/v1beta1.MachinePoolClass": schema_sigsk8sio_cluster_api_api_v1beta1_MachinePoolClass(ref), "sigs.k8s.io/cluster-api/api/v1beta1.MachinePoolClassNamingStrategy": schema_sigsk8sio_cluster_api_api_v1beta1_MachinePoolClassNamingStrategy(ref), "sigs.k8s.io/cluster-api/api/v1beta1.MachinePoolClassTemplate": schema_sigsk8sio_cluster_api_api_v1beta1_MachinePoolClassTemplate(ref), @@ -2219,6 +2220,12 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_MachineDeploymentSpec(ref common.R Ref: ref("sigs.k8s.io/cluster-api/api/v1beta1.MachineDeploymentStrategy"), }, }, + "machineNamingStrategy": { + SchemaProps: spec.SchemaProps{ + Description: "machineNamingStrategy allows changing the naming pattern used when creating Machines. Note: InfraMachines & BootstrapConfigs will use the same name as the corresponding Machines.", + Ref: ref("sigs.k8s.io/cluster-api/api/v1beta1.MachineNamingStrategy"), + }, + }, "minReadySeconds": { SchemaProps: spec.SchemaProps{ Description: "minReadySeconds is the minimum number of seconds for which a Node for a newly created machine should be ready before considering the replica available. Defaults to 0 (machine will be considered available as soon as the Node is ready)", @@ -2252,7 +2259,7 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_MachineDeploymentSpec(ref common.R }, }, Dependencies: []string{ - "k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelector", "k8s.io/apimachinery/pkg/apis/meta/v1.Time", "sigs.k8s.io/cluster-api/api/v1beta1.MachineDeploymentStrategy", "sigs.k8s.io/cluster-api/api/v1beta1.MachineTemplateSpec"}, + "k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelector", "k8s.io/apimachinery/pkg/apis/meta/v1.Time", "sigs.k8s.io/cluster-api/api/v1beta1.MachineDeploymentStrategy", "sigs.k8s.io/cluster-api/api/v1beta1.MachineNamingStrategy", "sigs.k8s.io/cluster-api/api/v1beta1.MachineTemplateSpec"}, } } @@ -3264,6 +3271,26 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_MachineList(ref common.ReferenceCa } } +func schema_sigsk8sio_cluster_api_api_v1beta1_MachineNamingStrategy(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "MachineNamingStrategy allows changing the naming pattern used when creating Machines. Note: InfraMachines will use the same name as the corresponding Machines.", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "template": { + SchemaProps: spec.SchemaProps{ + Description: "Template defines the template to use for generating the names of the Machine objects. If not defined, it will fallback to `{{ .machineSet.name }}-{{ .random }}`. If the generated name string exceeds 63 characters, it will be trimmed to 58 characters and will get concatenated with a random suffix of length 5. Length of the template string must not exceed 256 characters. The template allows the following variables `.cluster.name`, `.machineSet.name` and `.random`. The variable `.cluster.name` retrieves the name of the cluster object that owns the Machines being created. The variable `.machineSet.name` retrieves the name of the MachineSet object that owns the Machines being created. The variable `.random` is substituted with random alphanumeric string, without vowels, of length 5. This variable is required part of the template. If not provided, validation will fail.", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + } +} + func schema_sigsk8sio_cluster_api_api_v1beta1_MachinePoolClass(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -3729,12 +3756,18 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_MachineSetSpec(ref common.Referenc Ref: ref("sigs.k8s.io/cluster-api/api/v1beta1.MachineTemplateSpec"), }, }, + "machineNamingStrategy": { + SchemaProps: spec.SchemaProps{ + Description: "machineNamingStrategy allows changing the naming pattern used when creating Machines. Note: InfraMachines & BootstrapConfigs will use the same name as the corresponding Machines.", + Ref: ref("sigs.k8s.io/cluster-api/api/v1beta1.MachineNamingStrategy"), + }, + }, }, Required: []string{"clusterName", "selector"}, }, }, Dependencies: []string{ - "k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelector", "sigs.k8s.io/cluster-api/api/v1beta1.MachineTemplateSpec"}, + "k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelector", "sigs.k8s.io/cluster-api/api/v1beta1.MachineNamingStrategy", "sigs.k8s.io/cluster-api/api/v1beta1.MachineTemplateSpec"}, } } diff --git a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml index 4ebcaf5c24c3..98f8da537a7d 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml @@ -1120,6 +1120,32 @@ spec: to. minLength: 1 type: string + machineNamingStrategy: + description: |- + machineNamingStrategy allows changing the naming pattern used when creating Machines. + Note: InfraMachines & BootstrapConfigs will use the same name as the corresponding Machines. + properties: + template: + description: |- + Template defines the template to use for generating the names of the + Machine objects. + If not defined, it will fallback to `{{ .machineSet.name }}-{{ .random }}`. + If the generated name string exceeds 63 characters, it will be trimmed to + 58 characters and will + get concatenated with a random suffix of length 5. + Length of the template string must not exceed 256 characters. + The template allows the following variables `.cluster.name`, + `.machineSet.name` and `.random`. + The variable `.cluster.name` retrieves the name of the cluster object + that owns the Machines being created. + The variable `.machineSet.name` retrieves the name of the MachineSet + object that owns the Machines being created. + The variable `.random` is substituted with random alphanumeric string, + without vowels, of length 5. This variable is required part of the + template. If not provided, validation will fail. + maxLength: 256 + type: string + type: object minReadySeconds: description: |- minReadySeconds is the minimum number of seconds for which a Node for a newly created machine should be ready before considering the replica available. diff --git a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml index 04c517dee597..889bc369dfb9 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml @@ -980,6 +980,32 @@ spec: - Newest - Oldest type: string + machineNamingStrategy: + description: |- + machineNamingStrategy allows changing the naming pattern used when creating Machines. + Note: InfraMachines & BootstrapConfigs will use the same name as the corresponding Machines. + properties: + template: + description: |- + Template defines the template to use for generating the names of the + Machine objects. + If not defined, it will fallback to `{{ .machineSet.name }}-{{ .random }}`. + If the generated name string exceeds 63 characters, it will be trimmed to + 58 characters and will + get concatenated with a random suffix of length 5. + Length of the template string must not exceed 256 characters. + The template allows the following variables `.cluster.name`, + `.machineSet.name` and `.random`. + The variable `.cluster.name` retrieves the name of the cluster object + that owns the Machines being created. + The variable `.machineSet.name` retrieves the name of the MachineSet + object that owns the Machines being created. + The variable `.random` is substituted with random alphanumeric string, + without vowels, of length 5. This variable is required part of the + template. If not provided, validation will fail. + maxLength: 256 + type: string + type: object minReadySeconds: description: |- minReadySeconds is the minimum number of seconds for which a Node for a newly created machine should be ready before considering the replica available. diff --git a/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go b/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go index df85bb946bee..f9903e361b4d 100644 --- a/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go +++ b/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go @@ -38,7 +38,7 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" - "sigs.k8s.io/cluster-api/internal/topology/names" + topologynames "sigs.k8s.io/cluster-api/internal/topology/names" "sigs.k8s.io/cluster-api/internal/util/kubeadm" "sigs.k8s.io/cluster-api/util/container" "sigs.k8s.io/cluster-api/util/version" @@ -475,7 +475,7 @@ func validateNamingStrategy(machineNamingStrategy *controlplanev1.MachineNamingS )) return allErrs } - name, err := names.KCPMachineNameGenerator(machineNamingStrategy.Template, "cluster", "kubeadmcontrolplane").GenerateName() + name, err := topologynames.KCPMachineNameGenerator(machineNamingStrategy.Template, "cluster", "kubeadmcontrolplane").GenerateName() if err != nil { allErrs = append(allErrs, field.Invalid( diff --git a/controlplane/kubeadm/main.go b/controlplane/kubeadm/main.go index dd4e2ff3b65a..1353f6f3dd91 100644 --- a/controlplane/kubeadm/main.go +++ b/controlplane/kubeadm/main.go @@ -186,7 +186,7 @@ func InitFlags(fs *pflag.FlagSet) { fs.BoolVar(&useDeprecatedInfraMachineNaming, "use-deprecated-infra-machine-naming", false, "Use the deprecated naming convention for infra machines where they are named after the InfraMachineTemplate.") - _ = fs.MarkDeprecated("use-deprecated-infra-machine-naming", "This flag will be removed in v1.9.") + _ = fs.MarkDeprecated("use-deprecated-infra-machine-naming", "This flag will be removed in v1.10.") flags.AddManagerOptions(fs, &managerOptions) diff --git a/exp/topology/desiredstate/desired_state_test.go b/exp/topology/desiredstate/desired_state_test.go index 466a19f6b1c4..27cd4e405775 100644 --- a/exp/topology/desiredstate/desired_state_test.go +++ b/exp/topology/desiredstate/desired_state_test.go @@ -46,7 +46,7 @@ import ( "sigs.k8s.io/cluster-api/internal/hooks" fakeruntimeclient "sigs.k8s.io/cluster-api/internal/runtime/client/fake" "sigs.k8s.io/cluster-api/internal/topology/clustershim" - "sigs.k8s.io/cluster-api/internal/topology/names" + topologynames "sigs.k8s.io/cluster-api/internal/topology/names" "sigs.k8s.io/cluster-api/internal/topology/ownerrefs" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/test/builder" @@ -2689,7 +2689,7 @@ func TestTemplateToObject(t *testing.T) { template: template, templateClonedFromRef: fakeRef1, cluster: cluster, - nameGenerator: names.SimpleNameGenerator(cluster.Name), + nameGenerator: topologynames.SimpleNameGenerator(cluster.Name), currentObjectRef: nil, }) g.Expect(err).ToNot(HaveOccurred()) @@ -2709,7 +2709,7 @@ func TestTemplateToObject(t *testing.T) { template: template, templateClonedFromRef: fakeRef1, cluster: cluster, - nameGenerator: names.SimpleNameGenerator(cluster.Name), + nameGenerator: topologynames.SimpleNameGenerator(cluster.Name), currentObjectRef: fakeRef2, }) g.Expect(err).ToNot(HaveOccurred()) @@ -2750,7 +2750,7 @@ func TestTemplateToTemplate(t *testing.T) { template: template, templateClonedFromRef: fakeRef1, cluster: cluster, - nameGenerator: names.SimpleNameGenerator(cluster.Name), + nameGenerator: topologynames.SimpleNameGenerator(cluster.Name), currentObjectRef: nil, }) g.Expect(err).ToNot(HaveOccurred()) @@ -2769,7 +2769,7 @@ func TestTemplateToTemplate(t *testing.T) { template: template, templateClonedFromRef: fakeRef1, cluster: cluster, - nameGenerator: names.SimpleNameGenerator(cluster.Name), + nameGenerator: topologynames.SimpleNameGenerator(cluster.Name), currentObjectRef: fakeRef2, }) g.Expect(err).ToNot(HaveOccurred()) diff --git a/internal/apis/core/v1alpha3/conversion.go b/internal/apis/core/v1alpha3/conversion.go index ae90b81459c8..00f4f210e13f 100644 --- a/internal/apis/core/v1alpha3/conversion.go +++ b/internal/apis/core/v1alpha3/conversion.go @@ -154,6 +154,9 @@ func (src *MachineSet) ConvertTo(dstRaw conversion.Hub) error { dst.Status.Conditions = restored.Status.Conditions dst.Status.V1Beta2 = restored.Status.V1Beta2 + if restored.Spec.MachineNamingStrategy != nil { + dst.Spec.MachineNamingStrategy = restored.Spec.MachineNamingStrategy + } return nil } @@ -209,6 +212,10 @@ func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Strategy.Remediation = restored.Spec.Strategy.Remediation } + if restored.Spec.MachineNamingStrategy != nil { + dst.Spec.MachineNamingStrategy = restored.Spec.MachineNamingStrategy + } + dst.Spec.Template.Spec.ReadinessGates = restored.Spec.Template.Spec.ReadinessGates dst.Spec.Template.Spec.NodeDeletionTimeout = restored.Spec.Template.Spec.NodeDeletionTimeout dst.Spec.Template.Spec.NodeVolumeDetachTimeout = restored.Spec.Template.Spec.NodeVolumeDetachTimeout diff --git a/internal/apis/core/v1alpha3/zz_generated.conversion.go b/internal/apis/core/v1alpha3/zz_generated.conversion.go index 23d631ab59e9..fe6ba54d7c03 100644 --- a/internal/apis/core/v1alpha3/zz_generated.conversion.go +++ b/internal/apis/core/v1alpha3/zz_generated.conversion.go @@ -783,6 +783,7 @@ func autoConvert_v1beta1_MachineDeploymentSpec_To_v1alpha3_MachineDeploymentSpec } else { out.Strategy = nil } + // WARNING: in.MachineNamingStrategy requires manual conversion: does not exist in peer-type out.MinReadySeconds = (*int32)(unsafe.Pointer(in.MinReadySeconds)) out.RevisionHistoryLimit = (*int32)(unsafe.Pointer(in.RevisionHistoryLimit)) out.Paused = in.Paused @@ -1141,6 +1142,7 @@ func autoConvert_v1beta1_MachineSetSpec_To_v1alpha3_MachineSetSpec(in *v1beta1.M if err := Convert_v1beta1_MachineTemplateSpec_To_v1alpha3_MachineTemplateSpec(&in.Template, &out.Template, s); err != nil { return err } + // WARNING: in.MachineNamingStrategy requires manual conversion: does not exist in peer-type return nil } diff --git a/internal/apis/core/v1alpha4/conversion.go b/internal/apis/core/v1alpha4/conversion.go index cd69ca35d2ba..e51e3abc89b7 100644 --- a/internal/apis/core/v1alpha4/conversion.go +++ b/internal/apis/core/v1alpha4/conversion.go @@ -243,6 +243,10 @@ func (src *MachineSet) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Template.Spec.NodeVolumeDetachTimeout = restored.Spec.Template.Spec.NodeVolumeDetachTimeout dst.Status.V1Beta2 = restored.Status.V1Beta2 + if restored.Spec.MachineNamingStrategy != nil { + dst.Spec.MachineNamingStrategy = restored.Spec.MachineNamingStrategy + } + return nil } @@ -293,6 +297,11 @@ func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error { } dst.Spec.Strategy.Remediation = restored.Spec.Strategy.Remediation } + + if restored.Spec.MachineNamingStrategy != nil { + dst.Spec.MachineNamingStrategy = restored.Spec.MachineNamingStrategy + } + dst.Status.V1Beta2 = restored.Status.V1Beta2 return nil diff --git a/internal/apis/core/v1alpha4/zz_generated.conversion.go b/internal/apis/core/v1alpha4/zz_generated.conversion.go index 14df0dfc9112..a39473e6b376 100644 --- a/internal/apis/core/v1alpha4/zz_generated.conversion.go +++ b/internal/apis/core/v1alpha4/zz_generated.conversion.go @@ -1168,6 +1168,7 @@ func autoConvert_v1beta1_MachineDeploymentSpec_To_v1alpha4_MachineDeploymentSpec } else { out.Strategy = nil } + // WARNING: in.MachineNamingStrategy requires manual conversion: does not exist in peer-type out.MinReadySeconds = (*int32)(unsafe.Pointer(in.MinReadySeconds)) out.RevisionHistoryLimit = (*int32)(unsafe.Pointer(in.RevisionHistoryLimit)) out.Paused = in.Paused @@ -1556,6 +1557,7 @@ func autoConvert_v1beta1_MachineSetSpec_To_v1alpha4_MachineSetSpec(in *v1beta1.M if err := Convert_v1beta1_MachineTemplateSpec_To_v1alpha4_MachineTemplateSpec(&in.Template, &out.Template, s); err != nil { return err } + // WARNING: in.MachineNamingStrategy requires manual conversion: does not exist in peer-type return nil } diff --git a/internal/controllers/machinedeployment/machinedeployment_sync.go b/internal/controllers/machinedeployment/machinedeployment_sync.go index a2da7824b233..1d156c96e574 100644 --- a/internal/controllers/machinedeployment/machinedeployment_sync.go +++ b/internal/controllers/machinedeployment/machinedeployment_sync.go @@ -332,6 +332,11 @@ func (r *Reconciler) computeDesiredMachineSet(ctx context.Context, deployment *c desiredMS.Spec.Template.Spec.NodeDrainTimeout = deployment.Spec.Template.Spec.NodeDrainTimeout desiredMS.Spec.Template.Spec.NodeDeletionTimeout = deployment.Spec.Template.Spec.NodeDeletionTimeout desiredMS.Spec.Template.Spec.NodeVolumeDetachTimeout = deployment.Spec.Template.Spec.NodeVolumeDetachTimeout + if deployment.Spec.MachineNamingStrategy != nil && deployment.Spec.MachineNamingStrategy.Template != "" { + desiredMS.Spec.MachineNamingStrategy = &clusterv1.MachineNamingStrategy{ + Template: deployment.Spec.MachineNamingStrategy.Template, + } + } return desiredMS, nil } diff --git a/internal/controllers/machinedeployment/machinedeployment_sync_test.go b/internal/controllers/machinedeployment/machinedeployment_sync_test.go index 3904fb676894..595bae800fe7 100644 --- a/internal/controllers/machinedeployment/machinedeployment_sync_test.go +++ b/internal/controllers/machinedeployment/machinedeployment_sync_test.go @@ -538,6 +538,7 @@ func TestSyncDeploymentStatus(t *testing.T) { func TestComputeDesiredMachineSet(t *testing.T) { duration5s := &metav1.Duration{Duration: 5 * time.Second} duration10s := &metav1.Duration{Duration: 10 * time.Second} + namingTemplateKey := "test" infraRef := corev1.ObjectReference{ Kind: "GenericInfrastructureMachineTemplate", @@ -568,6 +569,9 @@ func TestComputeDesiredMachineSet(t *testing.T) { MaxUnavailable: intOrStrPtr(0), }, }, + MachineNamingStrategy: &clusterv1.MachineNamingStrategy{ + Template: "{{ .machineSet.name }}" + namingTemplateKey + "-{{ .random }}", + }, Selector: metav1.LabelSelector{ MatchLabels: map[string]string{"k1": "v1"}, }, @@ -604,6 +608,9 @@ func TestComputeDesiredMachineSet(t *testing.T) { DeletePolicy: string(clusterv1.RandomMachineSetDeletePolicy), Selector: metav1.LabelSelector{MatchLabels: map[string]string{"k1": "v1"}}, Template: *deployment.Spec.Template.DeepCopy(), + MachineNamingStrategy: &clusterv1.MachineNamingStrategy{ + Template: "{{ .machineSet.name }}" + namingTemplateKey + "-{{ .random }}", + }, }, } @@ -816,6 +823,11 @@ func assertMachineSet(g *WithT, actualMS *clusterv1.MachineSet, expectedMS *clus // Check MachineTemplateSpec g.Expect(actualMS.Spec.Template.Spec).Should(BeComparableTo(expectedMS.Spec.Template.Spec)) + + // Check MachineNamingStrategy + if actualMS.Spec.MachineNamingStrategy != nil { + g.Expect(actualMS.Spec.MachineNamingStrategy.Template).Should(BeComparableTo(expectedMS.Spec.MachineNamingStrategy.Template)) + } } // asserts the conditions set on the Getter object. diff --git a/internal/controllers/machineset/machineset_controller.go b/internal/controllers/machineset/machineset_controller.go index 4ee0f88af938..5c919c11cb6c 100644 --- a/internal/controllers/machineset/machineset_controller.go +++ b/internal/controllers/machineset/machineset_controller.go @@ -52,6 +52,7 @@ import ( "sigs.k8s.io/cluster-api/internal/contract" "sigs.k8s.io/cluster-api/internal/controllers/machine" "sigs.k8s.io/cluster-api/internal/controllers/machinedeployment/mdutil" + topologynames "sigs.k8s.io/cluster-api/internal/topology/names" "sigs.k8s.io/cluster-api/internal/util/ssa" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/collections" @@ -557,8 +558,11 @@ func (r *Reconciler) syncMachines(ctx context.Context, s *scope) (ctrl.Result, e } // Update Machine to propagate in-place mutable fields from the MachineSet. - updatedMachine := r.computeDesiredMachine(machineSet, m) - err := ssa.Patch(ctx, r.Client, machineSetManagerName, updatedMachine, ssa.WithCachingProxy{Cache: r.ssaCache, Original: m}) + updatedMachine, err := r.computeDesiredMachine(machineSet, m) + if err != nil { + return ctrl.Result{}, errors.Wrap(err, "failed to update Machine: failed to compute desired Machine") + } + err = ssa.Patch(ctx, r.Client, machineSetManagerName, updatedMachine, ssa.WithCachingProxy{Cache: r.ssaCache, Original: m}) if err != nil { log.Error(err, "Failed to update Machine", "Machine", klog.KObj(updatedMachine)) return ctrl.Result{}, errors.Wrapf(err, "failed to update Machine %q", klog.KObj(updatedMachine)) @@ -711,7 +715,10 @@ func (r *Reconciler) syncReplicas(ctx context.Context, s *scope) (ctrl.Result, e for i := range diff { // Create a new logger so the global logger is not modified. log := log - machine := r.computeDesiredMachine(ms, nil) + machine, computeMachineErr := r.computeDesiredMachine(ms, nil) + if computeMachineErr != nil { + return ctrl.Result{}, errors.Wrap(computeMachineErr, "failed to create Machine: failed to compute desired Machine") + } // Clone and set the infrastructure and bootstrap references. var ( infraRef, bootstrapRef *corev1.ObjectReference @@ -846,14 +853,28 @@ func (r *Reconciler) syncReplicas(ctx context.Context, s *scope) (ctrl.Result, e // There are small differences in how we calculate the Machine depending on if it // is a create or update. Example: for a new Machine we have to calculate a new name, // while for an existing Machine we have to use the name of the existing Machine. -func (r *Reconciler) computeDesiredMachine(machineSet *clusterv1.MachineSet, existingMachine *clusterv1.Machine) *clusterv1.Machine { +func (r *Reconciler) computeDesiredMachine(machineSet *clusterv1.MachineSet, existingMachine *clusterv1.Machine) (*clusterv1.Machine, error) { + nameTemplate := "{{ .machineSet.name }}-{{ .random }}" + if machineSet.Spec.MachineNamingStrategy != nil && machineSet.Spec.MachineNamingStrategy.Template != "" { + nameTemplate = machineSet.Spec.MachineNamingStrategy.Template + // This should never happen as this is validated on admission. + if !strings.Contains(nameTemplate, "{{ .random }}") { + return nil, errors.New("cannot generate MachineDeployment machine name: {{ .random }} is missing in machineNamingStrategy.template") + } + } + + generatedMachineName, err := topologynames.MachineSetMachineNameGenerator(nameTemplate, machineSet.Spec.ClusterName, machineSet.Name).GenerateName() + if err != nil { + return nil, errors.Wrap(err, "failed to generate name for MachineSet machine") + } + desiredMachine := &clusterv1.Machine{ TypeMeta: metav1.TypeMeta{ APIVersion: clusterv1.GroupVersion.String(), Kind: "Machine", }, ObjectMeta: metav1.ObjectMeta{ - Name: names.SimpleNameGenerator.GenerateName(fmt.Sprintf("%s-", machineSet.Name)), + Name: generatedMachineName, Namespace: machineSet.Namespace, // Note: By setting the ownerRef on creation we signal to the Machine controller that this is not a stand-alone Machine. OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(machineSet, machineSetKind)}, @@ -911,7 +932,7 @@ func (r *Reconciler) computeDesiredMachine(machineSet *clusterv1.MachineSet, exi desiredMachine.Spec.NodeDeletionTimeout = machineSet.Spec.Template.Spec.NodeDeletionTimeout desiredMachine.Spec.NodeVolumeDetachTimeout = machineSet.Spec.Template.Spec.NodeVolumeDetachTimeout - return desiredMachine + return desiredMachine, nil } // updateExternalObject updates the external object passed in with the diff --git a/internal/controllers/machineset/machineset_controller_test.go b/internal/controllers/machineset/machineset_controller_test.go index 9f587655cd55..6ce92e060293 100644 --- a/internal/controllers/machineset/machineset_controller_test.go +++ b/internal/controllers/machineset/machineset_controller_test.go @@ -22,6 +22,7 @@ import ( "time" . "github.com/onsi/gomega" + gomegatypes "github.com/onsi/gomega/types" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -2346,10 +2347,28 @@ func TestMachineSetReconciler_syncReplicas(t *testing.T) { }) } +type computeDesiredMachineTestCase struct { + name string + ms *clusterv1.MachineSet + existingMachine *clusterv1.Machine + wantMachine *clusterv1.Machine + wantName []gomegatypes.GomegaMatcher +} + func TestComputeDesiredMachine(t *testing.T) { duration5s := &metav1.Duration{Duration: 5 * time.Second} duration10s := &metav1.Duration{Duration: 10 * time.Second} + namingTemplateKey := "-md" + mdName := "testmd" + msName := "ms1" + testCluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: metav1.NamespaceDefault, + }, + } + infraRef := corev1.ObjectReference{ Kind: "GenericInfrastructureMachineTemplate", Name: "infra-template-1", @@ -2361,53 +2380,37 @@ func TestComputeDesiredMachine(t *testing.T) { APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1", } - ms := &clusterv1.MachineSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "ms1", - Labels: map[string]string{ - clusterv1.MachineDeploymentNameLabel: "md1", - }, + machineTemplateSpec := clusterv1.MachineTemplateSpec{ + ObjectMeta: clusterv1.ObjectMeta{ + Labels: map[string]string{"machine-label1": "machine-value1"}, + Annotations: map[string]string{"machine-annotation1": "machine-value1"}, }, - Spec: clusterv1.MachineSetSpec{ - ClusterName: "test-cluster", - Replicas: ptr.To[int32](3), - MinReadySeconds: 10, - Selector: metav1.LabelSelector{ - MatchLabels: map[string]string{"k1": "v1"}, - }, - Template: clusterv1.MachineTemplateSpec{ - ObjectMeta: clusterv1.ObjectMeta{ - Labels: map[string]string{"machine-label1": "machine-value1"}, - Annotations: map[string]string{"machine-annotation1": "machine-value1"}, - }, - Spec: clusterv1.MachineSpec{ - Version: ptr.To("v1.25.3"), - InfrastructureRef: infraRef, - Bootstrap: clusterv1.Bootstrap{ - ConfigRef: &bootstrapRef, - }, - NodeDrainTimeout: duration10s, - NodeVolumeDetachTimeout: duration10s, - NodeDeletionTimeout: duration10s, - }, + Spec: clusterv1.MachineSpec{ + ClusterName: testClusterName, + Version: ptr.To("v1.25.3"), + InfrastructureRef: infraRef, + Bootstrap: clusterv1.Bootstrap{ + ConfigRef: &bootstrapRef, }, + NodeDrainTimeout: duration10s, + NodeVolumeDetachTimeout: duration10s, + NodeDeletionTimeout: duration10s, }, } skeletonMachine := &clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", + Namespace: metav1.NamespaceDefault, Labels: map[string]string{ "machine-label1": "machine-value1", - clusterv1.MachineSetNameLabel: "ms1", - clusterv1.MachineDeploymentNameLabel: "md1", + clusterv1.MachineSetNameLabel: msName, + clusterv1.MachineDeploymentNameLabel: mdName, }, Annotations: map[string]string{"machine-annotation1": "machine-value1"}, Finalizers: []string{clusterv1.MachineFinalizer}, }, Spec: clusterv1.MachineSpec{ - ClusterName: "test-cluster", + ClusterName: testClusterName, Version: ptr.To("v1.25.3"), NodeDrainTimeout: duration10s, NodeVolumeDetachTimeout: duration10s, @@ -2448,34 +2451,200 @@ func TestComputeDesiredMachine(t *testing.T) { expectedUpdatedMachine.Spec.InfrastructureRef = *existingMachine.Spec.InfrastructureRef.DeepCopy() expectedUpdatedMachine.Spec.Bootstrap.ConfigRef = existingMachine.Spec.Bootstrap.ConfigRef.DeepCopy() - tests := []struct { - name string - existingMachine *clusterv1.Machine - want *clusterv1.Machine - }{ + tests := []computeDesiredMachineTestCase{ + { + name: "should return the correct Machine object when creating a new Machine", + ms: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: msName, + Labels: map[string]string{ + clusterv1.MachineDeploymentNameLabel: mdName, + }, + }, + Spec: clusterv1.MachineSetSpec{ + ClusterName: testClusterName, + Replicas: ptr.To[int32](3), + MinReadySeconds: 10, + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"k1": "v1"}, + }, + MachineNamingStrategy: &clusterv1.MachineNamingStrategy{ + Template: "{{ .machineSet.name }}" + namingTemplateKey + "-{{ .random }}", + }, + Template: machineTemplateSpec, + }, + }, + existingMachine: nil, + wantMachine: expectedNewMachine, + wantName: []gomegatypes.GomegaMatcher{ + HavePrefix(msName + namingTemplateKey + "-"), + Not(HaveSuffix("-")), + }, + }, + { + name: "should return error when creating a new Machine when '.random' is not added in template", + ms: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: msName, + Labels: map[string]string{ + clusterv1.MachineDeploymentNameLabel: mdName, + }, + }, + Spec: clusterv1.MachineSetSpec{ + ClusterName: testClusterName, + Replicas: ptr.To[int32](3), + MinReadySeconds: 10, + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"k1": "v1"}, + }, + MachineNamingStrategy: &clusterv1.MachineNamingStrategy{ + Template: "{{ .machineSet.name }}" + namingTemplateKey, + }, + Template: machineTemplateSpec, + }, + }, + existingMachine: nil, + wantMachine: nil, + }, + { + name: "should not return error when creating a new Machine when the generated name exceeds 63", + ms: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: msName, + Labels: map[string]string{ + clusterv1.MachineDeploymentNameLabel: mdName, + }, + }, + Spec: clusterv1.MachineSetSpec{ + ClusterName: testClusterName, + Replicas: ptr.To[int32](3), + MinReadySeconds: 10, + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"k1": "v1"}, + }, + MachineNamingStrategy: &clusterv1.MachineNamingStrategy{ + Template: "{{ .random }}" + fmt.Sprintf("%059d", 0), + }, + Template: machineTemplateSpec, + }, + }, + existingMachine: nil, + wantMachine: expectedNewMachine, + wantName: []gomegatypes.GomegaMatcher{ + ContainSubstring(fmt.Sprintf("%053d", 0)), + Not(HaveSuffix("00000")), + }, + }, + { + name: "should return error when creating a new Machine", + ms: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: msName, + Labels: map[string]string{ + clusterv1.MachineDeploymentNameLabel: mdName, + }, + }, + Spec: clusterv1.MachineSetSpec{ + ClusterName: testClusterName, + Replicas: ptr.To[int32](3), + MinReadySeconds: 10, + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"k1": "v1"}, + }, + MachineNamingStrategy: &clusterv1.MachineNamingStrategy{ + Template: "some-hardcoded-name-{{ .doesnotexistindata }}-{{ .random }}", // invalid template + }, + Template: machineTemplateSpec, + }, + }, + existingMachine: nil, + wantMachine: nil, + }, { - name: "creating a new Machine", + name: "should return the correct Machine object when creating a new Machine with default templated name", + ms: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: msName, + Labels: map[string]string{ + clusterv1.MachineDeploymentNameLabel: mdName, + }, + }, + Spec: clusterv1.MachineSetSpec{ + ClusterName: testClusterName, + Replicas: ptr.To[int32](3), + MinReadySeconds: 10, + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"k1": "v1"}, + }, + Template: machineTemplateSpec, + }, + }, existingMachine: nil, - want: expectedNewMachine, + wantMachine: expectedNewMachine, + wantName: []gomegatypes.GomegaMatcher{ + HavePrefix(msName), + }, }, { - name: "updating an existing Machine", + name: "updating an existing Machine", + ms: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: msName, + Labels: map[string]string{ + clusterv1.MachineDeploymentNameLabel: mdName, + }, + }, + Spec: clusterv1.MachineSetSpec{ + ClusterName: testClusterName, + Replicas: ptr.To[int32](3), + MinReadySeconds: 10, + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"k1": "v1"}, + }, + Template: machineTemplateSpec, + }, + }, existingMachine: existingMachine, - want: expectedUpdatedMachine, + wantMachine: expectedUpdatedMachine, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - got := (&Reconciler{}).computeDesiredMachine(ms, tt.existingMachine) - assertMachine(g, got, tt.want) + var got *clusterv1.Machine + var err error + msr := &Reconciler{ + Client: fake.NewClientBuilder().WithObjects( + testCluster, + tt.ms, + ).WithStatusSubresource(&clusterv1.MachineSet{}).Build(), + recorder: record.NewFakeRecorder(32), + } + got, err = msr.computeDesiredMachine(tt.ms, tt.existingMachine) + + if tt.wantMachine == nil { + g.Expect(err).To(HaveOccurred()) + return + } + assertMachine(g, got, tt.wantMachine, tt.existingMachine, tt.wantName) }) } } -func assertMachine(g *WithT, actualMachine *clusterv1.Machine, expectedMachine *clusterv1.Machine) { +func assertMachine(g *WithT, actualMachine *clusterv1.Machine, expectedMachine *clusterv1.Machine, existingMachine *clusterv1.Machine, nameMatches []gomegatypes.GomegaMatcher) { // Check Name + if existingMachine == nil { + for _, matcher := range nameMatches { + g.Expect(actualMachine.Name).To(matcher) + } + } if expectedMachine.Name != "" { g.Expect(actualMachine.Name).Should(Equal(expectedMachine.Name)) } diff --git a/internal/controllers/topology/cluster/reconcile_state_test.go b/internal/controllers/topology/cluster/reconcile_state_test.go index ac0f2eae4fbd..99794597db7e 100644 --- a/internal/controllers/topology/cluster/reconcile_state_test.go +++ b/internal/controllers/topology/cluster/reconcile_state_test.go @@ -51,7 +51,7 @@ import ( "sigs.k8s.io/cluster-api/internal/hooks" fakeruntimeclient "sigs.k8s.io/cluster-api/internal/runtime/client/fake" "sigs.k8s.io/cluster-api/internal/topology/clustershim" - "sigs.k8s.io/cluster-api/internal/topology/names" + topologynames "sigs.k8s.io/cluster-api/internal/topology/names" "sigs.k8s.io/cluster-api/internal/topology/ownerrefs" "sigs.k8s.io/cluster-api/internal/topology/selectors" "sigs.k8s.io/cluster-api/internal/util/ssa" @@ -1621,7 +1621,7 @@ func TestReconcileControlPlane(t *testing.T) { // This check is just for the naming format uses by generated templates - here it's templateName-* // This check is only performed when we had an initial template that has been changed if gotRotation { - pattern := fmt.Sprintf("%s.*", names.ControlPlaneInfrastructureMachineTemplateNamePrefix(s.Current.Cluster.Name)) + pattern := fmt.Sprintf("%s.*", topologynames.ControlPlaneInfrastructureMachineTemplateNamePrefix(s.Current.Cluster.Name)) ok, err := regexp.Match(pattern, []byte(gotInfrastructureMachineRef.Name)) g.Expect(err).ToNot(HaveOccurred()) g.Expect(ok).To(BeTrue()) diff --git a/internal/topology/names/names.go b/internal/topology/names/names.go index 0796d94debe4..f6d4259b7e8f 100644 --- a/internal/topology/names/names.go +++ b/internal/topology/names/names.go @@ -96,6 +96,16 @@ func KCPMachineNameGenerator(templateString, clusterName, kubeadmControlPlaneNam }) } +// MachineSetMachineNameGenerator returns a generator for creating a machineSet machine name. +func MachineSetMachineNameGenerator(templateString, clusterName, machineSetName string) NameGenerator { + return newTemplateGenerator(templateString, clusterName, + map[string]interface{}{ + "machineSet": map[string]interface{}{ + "name": machineSetName, + }, + }) +} + // templateGenerator parses the template string as text/template and executes it using // the passed data to generate a name. type templateGenerator struct { diff --git a/internal/webhooks/clusterclass.go b/internal/webhooks/clusterclass.go index 65fa7a2d69cd..4b16c110d499 100644 --- a/internal/webhooks/clusterclass.go +++ b/internal/webhooks/clusterclass.go @@ -38,7 +38,7 @@ import ( "sigs.k8s.io/cluster-api/api/v1beta1/index" "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/topology/check" - "sigs.k8s.io/cluster-api/internal/topology/names" + topologynames "sigs.k8s.io/cluster-api/internal/topology/names" "sigs.k8s.io/cluster-api/internal/topology/variables" ) @@ -434,7 +434,7 @@ func validateNamingStrategies(clusterClass *clusterv1.ClusterClass) field.ErrorL var allErrs field.ErrorList if clusterClass.Spec.ControlPlane.NamingStrategy != nil && clusterClass.Spec.ControlPlane.NamingStrategy.Template != nil { - name, err := names.ControlPlaneNameGenerator(*clusterClass.Spec.ControlPlane.NamingStrategy.Template, "cluster").GenerateName() + name, err := topologynames.ControlPlaneNameGenerator(*clusterClass.Spec.ControlPlane.NamingStrategy.Template, "cluster").GenerateName() templateFldPath := field.NewPath("spec", "controlPlane", "namingStrategy", "template") if err != nil { allErrs = append(allErrs, @@ -454,7 +454,7 @@ func validateNamingStrategies(clusterClass *clusterv1.ClusterClass) field.ErrorL if md.NamingStrategy == nil || md.NamingStrategy.Template == nil { continue } - name, err := names.MachineDeploymentNameGenerator(*md.NamingStrategy.Template, "cluster", "mdtopology").GenerateName() + name, err := topologynames.MachineDeploymentNameGenerator(*md.NamingStrategy.Template, "cluster", "mdtopology").GenerateName() templateFldPath := field.NewPath("spec", "workers", "machineDeployments").Key(md.Class).Child("namingStrategy", "template") if err != nil { allErrs = append(allErrs, @@ -474,7 +474,7 @@ func validateNamingStrategies(clusterClass *clusterv1.ClusterClass) field.ErrorL if mp.NamingStrategy == nil || mp.NamingStrategy.Template == nil { continue } - name, err := names.MachinePoolNameGenerator(*mp.NamingStrategy.Template, "cluster", "mptopology").GenerateName() + name, err := topologynames.MachinePoolNameGenerator(*mp.NamingStrategy.Template, "cluster", "mptopology").GenerateName() templateFldPath := field.NewPath("spec", "workers", "machinePools").Key(mp.Class).Child("namingStrategy", "template") if err != nil { allErrs = append(allErrs, diff --git a/internal/webhooks/machinedeployment.go b/internal/webhooks/machinedeployment.go index 4e03f589830c..17e62eb8a0fa 100644 --- a/internal/webhooks/machinedeployment.go +++ b/internal/webhooks/machinedeployment.go @@ -38,6 +38,7 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/feature" + topologynames "sigs.k8s.io/cluster-api/internal/topology/names" "sigs.k8s.io/cluster-api/util/version" ) @@ -300,6 +301,10 @@ func (webhook *MachineDeployment) validate(oldMD, newMD *clusterv1.MachineDeploy } } + if newMD.Spec.MachineNamingStrategy != nil { + allErrs = append(allErrs, validateMDMachineNamingStrategy(newMD.Spec.MachineNamingStrategy, specPath.Child("machineNamingStrategy"))...) + } + // Validate the metadata of the template. allErrs = append(allErrs, newMD.Spec.Template.ObjectMeta.Validate(specPath.Child("template", "metadata"))...) @@ -310,6 +315,41 @@ func (webhook *MachineDeployment) validate(oldMD, newMD *clusterv1.MachineDeploy return apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("MachineDeployment").GroupKind(), newMD.Name, allErrs) } +func validateMDMachineNamingStrategy(machineNamingStrategy *clusterv1.MachineNamingStrategy, pathPrefix *field.Path) field.ErrorList { + var allErrs field.ErrorList + + if machineNamingStrategy.Template != "" { + if !strings.Contains(machineNamingStrategy.Template, "{{ .random }}") { + allErrs = append(allErrs, + field.Invalid( + pathPrefix.Child("template"), + machineNamingStrategy.Template, + "invalid template, {{ .random }} is missing", + )) + } + name, err := topologynames.MachineSetMachineNameGenerator(machineNamingStrategy.Template, "cluster", "machineset").GenerateName() + if err != nil { + allErrs = append(allErrs, + field.Invalid( + pathPrefix.Child("template"), + machineNamingStrategy.Template, + fmt.Sprintf("invalid template: %v", err), + )) + } else { + for _, err := range validation.IsDNS1123Subdomain(name) { + allErrs = append(allErrs, + field.Invalid( + pathPrefix.Child("template"), + machineNamingStrategy.Template, + fmt.Sprintf("invalid template, generated names would not be valid Kubernetes object names: %v", err), + )) + } + } + } + + return allErrs +} + // calculateMachineDeploymentReplicas calculates the default value of the replicas field. // The value will be calculated based on the following logic: // * if replicas is already set on newMD, keep the current value diff --git a/internal/webhooks/machinedeployment_test.go b/internal/webhooks/machinedeployment_test.go index ebdcd3ad0ebe..665db8180522 100644 --- a/internal/webhooks/machinedeployment_test.go +++ b/internal/webhooks/machinedeployment_test.go @@ -301,13 +301,14 @@ func TestMachineDeploymentValidation(t *testing.T) { goodMaxUnavailableInt := intstr.FromInt(0) goodMaxInFlightInt := intstr.FromInt(5) tests := []struct { - name string - md *clusterv1.MachineDeployment - mdName string - selectors map[string]string - labels map[string]string - strategy clusterv1.MachineDeploymentStrategy - expectErr bool + name string + md *clusterv1.MachineDeployment + mdName string + selectors map[string]string + labels map[string]string + strategy clusterv1.MachineDeploymentStrategy + expectErr bool + machineNamingStrategy clusterv1.MachineNamingStrategy }{ { name: "pass with name of under 63 characters", @@ -454,6 +455,27 @@ func TestMachineDeploymentValidation(t *testing.T) { }, expectErr: false, }, + { + name: "should not return error when MachineNamingStrategy have {{ .random }}", + machineNamingStrategy: clusterv1.MachineNamingStrategy{ + Template: "{{ .machineSet.name }}-{{ .random }}", + }, + expectErr: false, + }, + { + name: "should return error when MachineNamingStrategy does not have {{ .random }}", + machineNamingStrategy: clusterv1.MachineNamingStrategy{ + Template: "{{ .machineSet.name }}", + }, + expectErr: true, + }, + { + name: "should return error when MachineNamingStrategy does not follow DNS1123Subdomain rules", + machineNamingStrategy: clusterv1.MachineNamingStrategy{ + Template: "{{ .machineSet.name }}-{{ .random }}-", + }, + expectErr: true, + }, } for i := range tests { @@ -474,6 +496,7 @@ func TestMachineDeploymentValidation(t *testing.T) { Labels: tt.labels, }, }, + MachineNamingStrategy: &tt.machineNamingStrategy, }, } diff --git a/internal/webhooks/machineset.go b/internal/webhooks/machineset.go index 2624abdbe83b..9f074f5bafff 100644 --- a/internal/webhooks/machineset.go +++ b/internal/webhooks/machineset.go @@ -29,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" @@ -38,6 +39,7 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/feature" + topologynames "sigs.k8s.io/cluster-api/internal/topology/names" "sigs.k8s.io/cluster-api/util/labels/format" "sigs.k8s.io/cluster-api/util/version" ) @@ -219,6 +221,9 @@ func (webhook *MachineSet) validate(oldMS, newMS *clusterv1.MachineSet) error { } } + if newMS.Spec.MachineNamingStrategy != nil { + allErrs = append(allErrs, validateMSMachineNamingStrategy(newMS.Spec.MachineNamingStrategy, specPath.Child("machineNamingStrategy"))...) + } // Validate the metadata of the template. allErrs = append(allErrs, newMS.Spec.Template.ObjectMeta.Validate(specPath.Child("template", "metadata"))...) @@ -229,6 +234,41 @@ func (webhook *MachineSet) validate(oldMS, newMS *clusterv1.MachineSet) error { return apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("MachineSet").GroupKind(), newMS.Name, allErrs) } +func validateMSMachineNamingStrategy(machineNamingStrategy *clusterv1.MachineNamingStrategy, pathPrefix *field.Path) field.ErrorList { + var allErrs field.ErrorList + + if machineNamingStrategy.Template != "" { + if !strings.Contains(machineNamingStrategy.Template, "{{ .random }}") { + allErrs = append(allErrs, + field.Invalid( + pathPrefix.Child("template"), + machineNamingStrategy.Template, + "invalid template, {{ .random }} is missing", + )) + } + name, err := topologynames.MachineSetMachineNameGenerator(machineNamingStrategy.Template, "cluster", "machineset").GenerateName() + if err != nil { + allErrs = append(allErrs, + field.Invalid( + pathPrefix.Child("template"), + machineNamingStrategy.Template, + fmt.Sprintf("invalid template: %v", err), + )) + } else { + for _, err := range validation.IsDNS1123Subdomain(name) { + allErrs = append(allErrs, + field.Invalid( + pathPrefix.Child("template"), + machineNamingStrategy.Template, + fmt.Sprintf("invalid template, generated names would not be valid Kubernetes object names: %v", err), + )) + } + } + } + + return allErrs +} + func validateSkippedMachineSetPreflightChecks(o client.Object) *field.Error { if o == nil { return nil diff --git a/internal/webhooks/machineset_test.go b/internal/webhooks/machineset_test.go index a972b60dda11..daa2a1546178 100644 --- a/internal/webhooks/machineset_test.go +++ b/internal/webhooks/machineset_test.go @@ -541,3 +541,62 @@ func TestMachineSetTemplateMetadataValidation(t *testing.T) { }) } } + +func TestMachineSetMachineNamingStrategyValidation(t *testing.T) { + tests := []struct { + name string + machineNamingStrategy clusterv1.MachineNamingStrategy + expectErr bool + }{ + { + name: "should not return error when MachineNamingStrategy have {{ .random }}", + machineNamingStrategy: clusterv1.MachineNamingStrategy{ + Template: "{{ .machineSet.name }}-{{ .random }}", + }, + expectErr: false, + }, + { + name: "should return error when MachineNamingStrategy does not have {{ .random }}", + machineNamingStrategy: clusterv1.MachineNamingStrategy{ + Template: "{{ .machineSet.name }}", + }, + expectErr: true, + }, + { + name: "should return error when MachineNamingStrategy does not follow DNS1123Subdomain rules", + machineNamingStrategy: clusterv1.MachineNamingStrategy{ + Template: "{{ .machineSet.name }}-{{ .random }}-", + }, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + ms := &clusterv1.MachineSet{ + Spec: clusterv1.MachineSetSpec{ + MachineNamingStrategy: &tt.machineNamingStrategy, + }, + } + + webhook := &MachineSet{} + + if tt.expectErr { + warnings, err := webhook.ValidateCreate(ctx, ms) + g.Expect(err).To(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + warnings, err = webhook.ValidateUpdate(ctx, ms, ms) + g.Expect(err).To(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + } else { + warnings, err := webhook.ValidateCreate(ctx, ms) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + warnings, err = webhook.ValidateUpdate(ctx, ms, ms) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(warnings).To(BeEmpty()) + } + }) + } +} diff --git a/main.go b/main.go index 59b7e3f9ce56..846b24c6b2c4 100644 --- a/main.go +++ b/main.go @@ -252,7 +252,7 @@ func InitFlags(fs *pflag.FlagSet) { fs.BoolVar(&useDeprecatedInfraMachineNaming, "use-deprecated-infra-machine-naming", false, "Use deprecated infrastructure machine naming") - _ = fs.MarkDeprecated("use-deprecated-infra-machine-naming", "This flag will be removed in v1.9.") + _ = fs.MarkDeprecated("use-deprecated-infra-machine-naming", "This flag will be removed in v1.10.") flags.AddManagerOptions(fs, &managerOptions)