diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go index 88f5395a34c1..c0447c79a399 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go @@ -123,6 +123,11 @@ type KubeadmControlPlaneSpec struct { // The RemediationStrategy that controls how control plane machine remediation happens. // +optional RemediationStrategy *RemediationStrategy `json:"remediationStrategy,omitempty"` + + // MachineNamingStrategy allows changing the naming pattern used when creating Machines. + // InfraMachines & KubeadmConfigs will use the same name as the corresponding Machines. + // +optional + MachineNamingStrategy *MachineNamingStrategy `json:"machineNamingStrategy,omitempty"` } // KubeadmControlPlaneMachineTemplate defines the template for Machines @@ -234,6 +239,23 @@ type RemediationStrategy struct { MinHealthyPeriod *metav1.Duration `json:"minHealthyPeriod,omitempty"` } +// MachineNamingStrategy allows changing the naming pattern used when creating Machines. +// InfraMachines & KubeadmConfigs 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 `{{ .cluster.name }}-{{ .kubeadmcontrolplane.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 93 characters. + // The template allows the following variables `.cluster.name`, `.kubeadmcontrolplane.name` and `.random`. + // The variable `.cluster.name` retrieves the name of the cluster object that owns the Machines being created. + // The variable `.kubeadmcontrolplane.name` retrieves the name of the KubeadmControlPlane object that owns the Machines being created. + // The variable `.random` is substituted with random alphanumeric string, without vowels, of length 5. + // +optional + // +kubebuilder:validation:MaxLength=93 + Template string `json:"template,omitempty"` +} + // KubeadmControlPlaneStatus defines the observed state of KubeadmControlPlane. type KubeadmControlPlaneStatus struct { // Selector is the label selector in string format to avoid introspection diff --git a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go b/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go index 478e30c7e786..353359661089 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go @@ -101,6 +101,11 @@ type KubeadmControlPlaneTemplateResourceSpec struct { // The RemediationStrategy that controls how control plane machine remediation happens. // +optional RemediationStrategy *RemediationStrategy `json:"remediationStrategy,omitempty"` + + // MachineNamingStrategy allows changing the naming pattern used when creating Machines. + // InfraMachines & KubeadmConfigs will use the same name as the corresponding Machines. + // +optional + MachineNamingStrategy *MachineNamingStrategy `json:"machineNamingStrategy,omitempty"` } // KubeadmControlPlaneTemplateMachineTemplate defines the template for Machines diff --git a/controlplane/kubeadm/api/v1beta1/zz_generated.deepcopy.go b/controlplane/kubeadm/api/v1beta1/zz_generated.deepcopy.go index fdef3a38dfbd..752ae6a689b7 100644 --- a/controlplane/kubeadm/api/v1beta1/zz_generated.deepcopy.go +++ b/controlplane/kubeadm/api/v1beta1/zz_generated.deepcopy.go @@ -147,6 +147,11 @@ func (in *KubeadmControlPlaneSpec) DeepCopyInto(out *KubeadmControlPlaneSpec) { *out = new(RemediationStrategy) (*in).DeepCopyInto(*out) } + 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 KubeadmControlPlaneSpec. @@ -330,6 +335,11 @@ func (in *KubeadmControlPlaneTemplateResourceSpec) DeepCopyInto(out *KubeadmCont *out = new(RemediationStrategy) (*in).DeepCopyInto(*out) } + 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 KubeadmControlPlaneTemplateResourceSpec. @@ -374,6 +384,21 @@ func (in *LastRemediationStatus) DeepCopy() *LastRemediationStatus { return out } +// 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 *RemediationStrategy) DeepCopyInto(out *RemediationStrategy) { *out = *in diff --git a/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml b/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml index cb4668d56d03..50e2de52b15f 100644 --- a/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml +++ b/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml @@ -4185,6 +4185,25 @@ spec: format: int32 type: integer type: object + machineNamingStrategy: + description: |- + MachineNamingStrategy allows changing the naming pattern used when creating Machines. + InfraMachines & KubeadmConfigs 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 `{{ .cluster.name }}-{{ .kubeadmcontrolplane.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 93 characters. + The template allows the following variables `.cluster.name`, `.kubeadmcontrolplane.name` and `.random`. + The variable `.cluster.name` retrieves the name of the cluster object that owns the Machines being created. + The variable `.kubeadmcontrolplane.name` retrieves the name of the KubeadmControlPlane object that owns the Machines being created. + The variable `.random` is substituted with random alphanumeric string, without vowels, of length 5. + maxLength: 93 + type: string + type: object machineTemplate: description: |- MachineTemplate contains information about how machines diff --git a/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanetemplates.yaml b/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanetemplates.yaml index 8ce2a88e96f5..85cfd7d61d7f 100644 --- a/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanetemplates.yaml +++ b/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanetemplates.yaml @@ -2926,6 +2926,25 @@ spec: format: int32 type: integer type: object + machineNamingStrategy: + description: |- + MachineNamingStrategy allows changing the naming pattern used when creating Machines. + InfraMachines & KubeadmConfigs 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 `{{ .cluster.name }}-{{ .kubeadmcontrolplane.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 93 characters. + The template allows the following variables `.cluster.name`, `.kubeadmcontrolplane.name` and `.random`. + The variable `.cluster.name` retrieves the name of the cluster object that owns the Machines being created. + The variable `.kubeadmcontrolplane.name` retrieves the name of the KubeadmControlPlane object that owns the Machines being created. + The variable `.random` is substituted with random alphanumeric string, without vowels, of length 5. + maxLength: 93 + type: string + type: object machineTemplate: description: |- MachineTemplate contains information about how machines diff --git a/controlplane/kubeadm/internal/controllers/helpers.go b/controlplane/kubeadm/internal/controllers/helpers.go index 98dcfd0d6ba8..0ceb9cd5ac97 100644 --- a/controlplane/kubeadm/internal/controllers/helpers.go +++ b/controlplane/kubeadm/internal/controllers/helpers.go @@ -37,6 +37,7 @@ import ( "sigs.k8s.io/cluster-api/controllers/external" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" + 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/certs" @@ -184,6 +185,7 @@ func (r *KubeadmControlPlaneReconciler) cloneConfigsAndGenerateMachine(ctx conte if r.DeprecatedInfraMachineNaming { infraMachineName = names.SimpleNameGenerator.GenerateName(kcp.Spec.MachineTemplate.InfrastructureRef.Name + "-") } + // Clone the infrastructure template infraRef, err := external.CreateFromTemplate(ctx, &external.CreateFromTemplateInput{ Client: r.Client, @@ -349,7 +351,15 @@ func (r *KubeadmControlPlaneReconciler) computeDesiredMachine(kcp *controlplanev annotations := map[string]string{} if existingMachine == nil { // Creating a new machine - machineName = names.SimpleNameGenerator.GenerateName(kcp.Name + "-") + nameTemplate := "{{ .cluster.name }}-{{ .kubeadmcontrolplane.name }}-{{ .random }}" + if kcp.Spec.MachineNamingStrategy != nil && kcp.Spec.MachineNamingStrategy.Template != "" { + nameTemplate = kcp.Spec.MachineNamingStrategy.Template + } + generatedMachineName, err := topologynames.KCPMachineNameGenerator(nameTemplate, cluster.Name, kcp.Name).GenerateName() + if err != nil { + return nil, errors.Wrap(err, "failed to generate name for KCP machine") + } + machineName = generatedMachineName version = &kcp.Spec.Version // Machine's bootstrap config may be missing ClusterConfiguration if it is not the first machine in the control plane. diff --git a/controlplane/kubeadm/internal/controllers/helpers_test.go b/controlplane/kubeadm/internal/controllers/helpers_test.go index 28108cf9b81b..59551845e581 100644 --- a/controlplane/kubeadm/internal/controllers/helpers_test.go +++ b/controlplane/kubeadm/internal/controllers/helpers_test.go @@ -17,10 +17,12 @@ limitations under the License. package controllers import ( + "fmt" "testing" "time" . "github.com/onsi/gomega" + gomegatypes "github.com/onsi/gomega/types" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -340,6 +342,7 @@ func TestCloneConfigsAndGenerateMachine(t *testing.T) { } g.Expect(env.CreateAndWait(ctx, genericInfrastructureMachineTemplate)).To(Succeed()) + namingTemplateKey := "-testkcp" kcp := &controlplanev1.KubeadmControlPlane{ ObjectMeta: metav1.ObjectMeta{ Name: "kcp-foo", @@ -356,6 +359,9 @@ func TestCloneConfigsAndGenerateMachine(t *testing.T) { }, }, Version: "v1.16.6", + MachineNamingStrategy: &controlplanev1.MachineNamingStrategy{ + Template: "{{ .kubeadmcontrolplane.name }}" + namingTemplateKey + "-{{ .random }}", + }, }, } @@ -378,7 +384,7 @@ func TestCloneConfigsAndGenerateMachine(t *testing.T) { m := machineList.Items[i] g.Expect(m.Namespace).To(Equal(cluster.Namespace)) g.Expect(m.Name).NotTo(BeEmpty()) - g.Expect(m.Name).To(HavePrefix(kcp.Name)) + g.Expect(m.Name).To(HavePrefix(kcp.Name + namingTemplateKey)) infraObj, err := external.Get(ctx, r.Client, &m.Spec.InfrastructureRef, m.Spec.InfrastructureRef.Namespace) g.Expect(err).ToNot(HaveOccurred()) @@ -474,10 +480,8 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) { Namespace: metav1.NamespaceDefault, }, } - duration5s := &metav1.Duration{Duration: 5 * time.Second} duration10s := &metav1.Duration{Duration: 10 * time.Second} - kcpMachineTemplateObjectMeta := clusterv1.ObjectMeta{ Labels: map[string]string{ "machineTemplateLabel": "machineTemplateLabelValue", @@ -487,26 +491,10 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) { }, } kcpMachineTemplateObjectMetaCopy := kcpMachineTemplateObjectMeta.DeepCopy() - kcp := &controlplanev1.KubeadmControlPlane{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testControlPlane", - Namespace: cluster.Namespace, - }, - Spec: controlplanev1.KubeadmControlPlaneSpec{ - Version: "v1.16.6", - MachineTemplate: controlplanev1.KubeadmControlPlaneMachineTemplate{ - ObjectMeta: kcpMachineTemplateObjectMeta, - NodeDrainTimeout: duration5s, - NodeDeletionTimeout: duration5s, - NodeVolumeDetachTimeout: duration5s, - }, - KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ - ClusterConfiguration: &bootstrapv1.ClusterConfiguration{ - ClusterName: "testCluster", - }, - }, - }, - } + namingTemplateKey := "-kcp" + kcpName := "testControlPlane" + clusterName := "testCluster" + clusterConfigurationString := "{\"etcd\":{},\"networking\":{},\"apiServer\":{},\"controllerManager\":{},\"scheduler\":{},\"dns\":{},\"clusterName\":\"testCluster\"}" infraRef := &corev1.ObjectReference{ @@ -522,140 +510,292 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) { Namespace: cluster.Namespace, } - t.Run("should return the correct Machine object when creating a new Machine", func(t *testing.T) { - g := NewWithT(t) - - failureDomain := ptr.To("fd1") - createdMachine, err := (&KubeadmControlPlaneReconciler{}).computeDesiredMachine( - kcp, cluster, - failureDomain, nil, - ) - g.Expect(err).ToNot(HaveOccurred()) - - expectedMachineSpec := clusterv1.MachineSpec{ - ClusterName: cluster.Name, - Version: ptr.To(kcp.Spec.Version), - FailureDomain: failureDomain, - NodeDrainTimeout: kcp.Spec.MachineTemplate.NodeDrainTimeout, - NodeDeletionTimeout: kcp.Spec.MachineTemplate.NodeDeletionTimeout, - NodeVolumeDetachTimeout: kcp.Spec.MachineTemplate.NodeVolumeDetachTimeout, - } - g.Expect(createdMachine.Name).To(HavePrefix(kcp.Name)) - g.Expect(createdMachine.Namespace).To(Equal(kcp.Namespace)) - g.Expect(createdMachine.OwnerReferences).To(HaveLen(1)) - g.Expect(createdMachine.OwnerReferences).To(ContainElement(*metav1.NewControllerRef(kcp, controlplanev1.GroupVersion.WithKind("KubeadmControlPlane")))) - g.Expect(createdMachine.Spec).To(BeComparableTo(expectedMachineSpec)) - - // Verify that the machineTemplate.ObjectMeta has been propagated to the Machine. - // Verify labels. - expectedLabels := map[string]string{} - for k, v := range kcpMachineTemplateObjectMeta.Labels { - expectedLabels[k] = v - } - expectedLabels[clusterv1.ClusterNameLabel] = cluster.Name - expectedLabels[clusterv1.MachineControlPlaneLabel] = "" - expectedLabels[clusterv1.MachineControlPlaneNameLabel] = kcp.Name - g.Expect(createdMachine.Labels).To(Equal(expectedLabels)) - // Verify annotations. - expectedAnnotations := map[string]string{} - for k, v := range kcpMachineTemplateObjectMeta.Annotations { - expectedAnnotations[k] = v - } - expectedAnnotations[controlplanev1.KubeadmClusterConfigurationAnnotation] = clusterConfigurationString - // The pre-terminate annotation should always be added - expectedAnnotations[controlplanev1.PreTerminateHookCleanupAnnotation] = "" - g.Expect(createdMachine.Annotations).To(Equal(expectedAnnotations)) - - // Verify that machineTemplate.ObjectMeta in KCP has not been modified. - g.Expect(kcp.Spec.MachineTemplate.ObjectMeta.Labels).To(Equal(kcpMachineTemplateObjectMetaCopy.Labels)) - g.Expect(kcp.Spec.MachineTemplate.ObjectMeta.Annotations).To(Equal(kcpMachineTemplateObjectMetaCopy.Annotations)) - }) - - t.Run("should return the correct Machine object when updating an existing Machine", func(t *testing.T) { - g := NewWithT(t) - - machineName := "existing-machine" - machineUID := types.UID("abc-123-existing-machine") - // Use different ClusterConfiguration string than the information present in KCP - // to verify that for an existing machine we do not override this information. - existingClusterConfigurationString := "existing-cluster-configuration-information" - remediationData := "remediation-data" - failureDomain := ptr.To("fd-1") - machineVersion := ptr.To("v1.25.3") - existingMachine := &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: machineName, - UID: machineUID, - Annotations: map[string]string{ - controlplanev1.KubeadmClusterConfigurationAnnotation: existingClusterConfigurationString, - controlplanev1.RemediationForAnnotation: remediationData, + tests := []struct { + name string + kcp *controlplanev1.KubeadmControlPlane + isUpdatingExistingMachine bool + want []gomegatypes.GomegaMatcher + wantErr bool + }{ + { + name: "should return the correct Machine object when creating a new Machine", + kcp: &controlplanev1.KubeadmControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: kcpName, + Namespace: cluster.Namespace, + }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + MachineTemplate: controlplanev1.KubeadmControlPlaneMachineTemplate{ + ObjectMeta: kcpMachineTemplateObjectMeta, + NodeDrainTimeout: duration5s, + NodeDeletionTimeout: duration5s, + NodeVolumeDetachTimeout: duration5s, + }, + KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ + ClusterConfiguration: &bootstrapv1.ClusterConfiguration{ + ClusterName: clusterName, + }, + }, + MachineNamingStrategy: &controlplanev1.MachineNamingStrategy{ + Template: "{{ .kubeadmcontrolplane.name }}" + namingTemplateKey + "-{{ .random }}", + }, + }, + }, + isUpdatingExistingMachine: false, + want: []gomegatypes.GomegaMatcher{ + HavePrefix(kcpName + namingTemplateKey), + Not(HaveSuffix("00000")), + }, + wantErr: false, + }, + { + name: "should not return error when creating a new Machine when the generated name exceeds 63", + kcp: &controlplanev1.KubeadmControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: kcpName, + Namespace: cluster.Namespace, + }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + MachineTemplate: controlplanev1.KubeadmControlPlaneMachineTemplate{ + ObjectMeta: kcpMachineTemplateObjectMeta, + NodeDrainTimeout: duration5s, + NodeDeletionTimeout: duration5s, + NodeVolumeDetachTimeout: duration5s, + }, + KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ + ClusterConfiguration: &bootstrapv1.ClusterConfiguration{ + ClusterName: clusterName, + }, + }, + MachineNamingStrategy: &controlplanev1.MachineNamingStrategy{ + Template: fmt.Sprintf("%064d", 0), + }, }, }, - Spec: clusterv1.MachineSpec{ - Version: machineVersion, - FailureDomain: failureDomain, - NodeDrainTimeout: duration10s, - NodeDeletionTimeout: duration10s, - NodeVolumeDetachTimeout: duration10s, - Bootstrap: clusterv1.Bootstrap{ - ConfigRef: bootstrapRef, + isUpdatingExistingMachine: false, + want: []gomegatypes.GomegaMatcher{ + HavePrefix(fmt.Sprintf("%058d", 0)), + Not(HaveSuffix("00000")), + }, + wantErr: false, + }, + { + name: "should return error when creating a new Machine", + kcp: &controlplanev1.KubeadmControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: kcpName, + Namespace: cluster.Namespace, + }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + MachineTemplate: controlplanev1.KubeadmControlPlaneMachineTemplate{ + ObjectMeta: kcpMachineTemplateObjectMeta, + NodeDrainTimeout: duration5s, + NodeDeletionTimeout: duration5s, + NodeVolumeDetachTimeout: duration5s, + }, + KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ + ClusterConfiguration: &bootstrapv1.ClusterConfiguration{ + ClusterName: clusterName, + }, + }, + MachineNamingStrategy: &controlplanev1.MachineNamingStrategy{ + Template: "some-hardcoded-name-{{ .doesnotexistindata }}", // invalid template + }, + }, + }, + isUpdatingExistingMachine: false, + wantErr: true, + }, + { + name: "should return the correct Machine object when creating a new Machine with default templated name", + kcp: &controlplanev1.KubeadmControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: kcpName, + Namespace: cluster.Namespace, + }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + MachineTemplate: controlplanev1.KubeadmControlPlaneMachineTemplate{ + ObjectMeta: kcpMachineTemplateObjectMeta, + NodeDrainTimeout: duration5s, + NodeDeletionTimeout: duration5s, + NodeVolumeDetachTimeout: duration5s, + }, + KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ + ClusterConfiguration: &bootstrapv1.ClusterConfiguration{ + ClusterName: clusterName, + }, + }, + }, + }, + isUpdatingExistingMachine: false, + wantErr: false, + want: []gomegatypes.GomegaMatcher{ + HavePrefix(fmt.Sprintf("%s-%s", clusterName, kcpName)), + Not(HaveSuffix("00000")), + }, + }, + { + name: "should return the correct Machine object when updating an existing Machine", + kcp: &controlplanev1.KubeadmControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: kcpName, + Namespace: cluster.Namespace, + }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + MachineTemplate: controlplanev1.KubeadmControlPlaneMachineTemplate{ + ObjectMeta: kcpMachineTemplateObjectMeta, + NodeDrainTimeout: duration5s, + NodeDeletionTimeout: duration5s, + NodeVolumeDetachTimeout: duration5s, + }, + KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ + ClusterConfiguration: &bootstrapv1.ClusterConfiguration{ + ClusterName: "testCluster", + }, + }, + MachineNamingStrategy: &controlplanev1.MachineNamingStrategy{ + Template: "{{ .kubeadmcontrolplane.name }}" + namingTemplateKey + "-{{ .random }}", + }, }, - InfrastructureRef: *infraRef, }, - } + isUpdatingExistingMachine: true, + wantErr: false, + }, + } - updatedMachine, err := (&KubeadmControlPlaneReconciler{}).computeDesiredMachine( - kcp, cluster, - existingMachine.Spec.FailureDomain, existingMachine, - ) - g.Expect(err).ToNot(HaveOccurred()) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + var desiredMachine *clusterv1.Machine + failureDomain := ptr.To("fd-1") + var expectedMachineSpec clusterv1.MachineSpec + var err error + + if tt.isUpdatingExistingMachine { + machineName := "existing-machine" + machineUID := types.UID("abc-123-existing-machine") + // Use different ClusterConfiguration string than the information present in KCP + // to verify that for an existing machine we do not override this information. + existingClusterConfigurationString := "existing-cluster-configuration-information" + remediationData := "remediation-data" + machineVersion := ptr.To("v1.25.3") + existingMachine := &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: machineName, + UID: machineUID, + Annotations: map[string]string{ + controlplanev1.KubeadmClusterConfigurationAnnotation: existingClusterConfigurationString, + controlplanev1.RemediationForAnnotation: remediationData, + }, + }, + Spec: clusterv1.MachineSpec{ + Version: machineVersion, + FailureDomain: failureDomain, + NodeDrainTimeout: duration10s, + NodeDeletionTimeout: duration10s, + NodeVolumeDetachTimeout: duration10s, + Bootstrap: clusterv1.Bootstrap{ + ConfigRef: bootstrapRef, + }, + InfrastructureRef: *infraRef, + }, + } + desiredMachine, err = (&KubeadmControlPlaneReconciler{}).computeDesiredMachine( + tt.kcp, cluster, + existingMachine.Spec.FailureDomain, existingMachine, + ) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + expectedMachineSpec = clusterv1.MachineSpec{ + ClusterName: cluster.Name, + Version: machineVersion, // Should use the Machine version and not the version from KCP. + Bootstrap: clusterv1.Bootstrap{ + ConfigRef: bootstrapRef, + }, + InfrastructureRef: *infraRef, + FailureDomain: failureDomain, + NodeDrainTimeout: tt.kcp.Spec.MachineTemplate.NodeDrainTimeout, + NodeDeletionTimeout: tt.kcp.Spec.MachineTemplate.NodeDeletionTimeout, + NodeVolumeDetachTimeout: tt.kcp.Spec.MachineTemplate.NodeVolumeDetachTimeout, + } + + // Verify the Name and UID of the Machine remain unchanged + g.Expect(desiredMachine.Name).To(Equal(machineName)) + g.Expect(desiredMachine.UID).To(Equal(machineUID)) + // Verify annotations. + expectedAnnotations := map[string]string{} + for k, v := range kcpMachineTemplateObjectMeta.Annotations { + expectedAnnotations[k] = v + } + expectedAnnotations[controlplanev1.KubeadmClusterConfigurationAnnotation] = existingClusterConfigurationString + expectedAnnotations[controlplanev1.RemediationForAnnotation] = remediationData + // The pre-terminate annotation should always be added + expectedAnnotations[controlplanev1.PreTerminateHookCleanupAnnotation] = "" + g.Expect(desiredMachine.Annotations).To(Equal(expectedAnnotations)) + } else { + desiredMachine, err = (&KubeadmControlPlaneReconciler{}).computeDesiredMachine( + tt.kcp, cluster, + failureDomain, nil, + ) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + + expectedMachineSpec = clusterv1.MachineSpec{ + ClusterName: cluster.Name, + Version: ptr.To(tt.kcp.Spec.Version), + FailureDomain: failureDomain, + NodeDrainTimeout: tt.kcp.Spec.MachineTemplate.NodeDrainTimeout, + NodeDeletionTimeout: tt.kcp.Spec.MachineTemplate.NodeDeletionTimeout, + NodeVolumeDetachTimeout: tt.kcp.Spec.MachineTemplate.NodeVolumeDetachTimeout, + } + // Verify Name. + for _, matcher := range tt.want { + g.Expect(desiredMachine.Name).To(matcher) + } + // Verify annotations. + expectedAnnotations := map[string]string{} + for k, v := range kcpMachineTemplateObjectMeta.Annotations { + expectedAnnotations[k] = v + } + expectedAnnotations[controlplanev1.KubeadmClusterConfigurationAnnotation] = clusterConfigurationString + // The pre-terminate annotation should always be added + expectedAnnotations[controlplanev1.PreTerminateHookCleanupAnnotation] = "" + g.Expect(desiredMachine.Annotations).To(Equal(expectedAnnotations)) + } - expectedMachineSpec := clusterv1.MachineSpec{ - ClusterName: cluster.Name, - Version: machineVersion, // Should use the Machine version and not the version from KCP. - Bootstrap: clusterv1.Bootstrap{ - ConfigRef: bootstrapRef, - }, - InfrastructureRef: *infraRef, - FailureDomain: failureDomain, - NodeDrainTimeout: kcp.Spec.MachineTemplate.NodeDrainTimeout, - NodeDeletionTimeout: kcp.Spec.MachineTemplate.NodeDeletionTimeout, - NodeVolumeDetachTimeout: kcp.Spec.MachineTemplate.NodeVolumeDetachTimeout, - } - g.Expect(updatedMachine.Namespace).To(Equal(kcp.Namespace)) - g.Expect(updatedMachine.OwnerReferences).To(HaveLen(1)) - g.Expect(updatedMachine.OwnerReferences).To(ContainElement(*metav1.NewControllerRef(kcp, controlplanev1.GroupVersion.WithKind("KubeadmControlPlane")))) - g.Expect(updatedMachine.Spec).To(BeComparableTo(expectedMachineSpec)) - - // Verify the Name and UID of the Machine remain unchanged - g.Expect(updatedMachine.Name).To(Equal(machineName)) - g.Expect(updatedMachine.UID).To(Equal(machineUID)) - - // Verify that the machineTemplate.ObjectMeta has been propagated to the Machine. - // Verify labels. - expectedLabels := map[string]string{} - for k, v := range kcpMachineTemplateObjectMeta.Labels { - expectedLabels[k] = v - } - expectedLabels[clusterv1.ClusterNameLabel] = cluster.Name - expectedLabels[clusterv1.MachineControlPlaneLabel] = "" - expectedLabels[clusterv1.MachineControlPlaneNameLabel] = kcp.Name - g.Expect(updatedMachine.Labels).To(Equal(expectedLabels)) - // Verify annotations. - expectedAnnotations := map[string]string{} - for k, v := range kcpMachineTemplateObjectMeta.Annotations { - expectedAnnotations[k] = v - } - expectedAnnotations[controlplanev1.KubeadmClusterConfigurationAnnotation] = existingClusterConfigurationString - expectedAnnotations[controlplanev1.RemediationForAnnotation] = remediationData - // The pre-terminate annotation should always be added - expectedAnnotations[controlplanev1.PreTerminateHookCleanupAnnotation] = "" - g.Expect(updatedMachine.Annotations).To(Equal(expectedAnnotations)) - - // Verify that machineTemplate.ObjectMeta in KCP has not been modified. - g.Expect(kcp.Spec.MachineTemplate.ObjectMeta.Labels).To(Equal(kcpMachineTemplateObjectMetaCopy.Labels)) - g.Expect(kcp.Spec.MachineTemplate.ObjectMeta.Annotations).To(Equal(kcpMachineTemplateObjectMetaCopy.Annotations)) - }) + g.Expect(desiredMachine.Namespace).To(Equal(tt.kcp.Namespace)) + g.Expect(desiredMachine.OwnerReferences).To(HaveLen(1)) + g.Expect(desiredMachine.OwnerReferences).To(ContainElement(*metav1.NewControllerRef(tt.kcp, controlplanev1.GroupVersion.WithKind("KubeadmControlPlane")))) + g.Expect(desiredMachine.Spec).To(BeComparableTo(expectedMachineSpec)) + + // Verify that the machineTemplate.ObjectMeta has been propagated to the Machine. + // Verify labels. + expectedLabels := map[string]string{} + for k, v := range kcpMachineTemplateObjectMeta.Labels { + expectedLabels[k] = v + } + expectedLabels[clusterv1.ClusterNameLabel] = cluster.Name + expectedLabels[clusterv1.MachineControlPlaneLabel] = "" + expectedLabels[clusterv1.MachineControlPlaneNameLabel] = tt.kcp.Name + g.Expect(desiredMachine.Labels).To(Equal(expectedLabels)) + + // Verify that machineTemplate.ObjectMeta in KCP has not been modified. + g.Expect(tt.kcp.Spec.MachineTemplate.ObjectMeta.Labels).To(Equal(kcpMachineTemplateObjectMetaCopy.Labels)) + g.Expect(tt.kcp.Spec.MachineTemplate.ObjectMeta.Annotations).To(Equal(kcpMachineTemplateObjectMetaCopy.Annotations)) + }) + } } func TestKubeadmControlPlaneReconciler_generateKubeadmConfig(t *testing.T) { diff --git a/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go b/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go index d848d4616bba..a526a488829b 100644 --- a/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go +++ b/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go @@ -29,6 +29,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -37,6 +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" "sigs.k8s.io/cluster-api/internal/util/kubeadm" "sigs.k8s.io/cluster-api/util/container" "sigs.k8s.io/cluster-api/util/version" @@ -234,6 +236,8 @@ func (webhook *KubeadmControlPlane) ValidateUpdate(_ context.Context, oldObj, ne {spec, "version"}, {spec, "remediationStrategy"}, {spec, "remediationStrategy", "*"}, + {spec, "machineNamingStrategy"}, + {spec, "machineNamingStrategy", "*"}, {spec, "rolloutAfter"}, {spec, "rolloutBefore"}, {spec, "rolloutBefore", "*"}, @@ -393,6 +397,9 @@ func validateKubeadmControlPlaneSpec(s controlplanev1.KubeadmControlPlaneSpec, n allErrs = append(allErrs, validateRolloutBefore(s.RolloutBefore, pathPrefix.Child("rolloutBefore"))...) allErrs = append(allErrs, validateRolloutStrategy(s.RolloutStrategy, s.Replicas, pathPrefix.Child("rolloutStrategy"))...) + if s.MachineNamingStrategy != nil { + allErrs = append(allErrs, validateNamingStrategy(s.MachineNamingStrategy, pathPrefix.Child("machineNamingStrategy"))...) + } return allErrs } @@ -455,6 +462,28 @@ func validateRolloutStrategy(rolloutStrategy *controlplanev1.RolloutStrategy, re return allErrs } +func validateNamingStrategy(machineNamingStrategy *controlplanev1.MachineNamingStrategy, pathPrefix *field.Path) field.ErrorList { + var allErrs field.ErrorList + + if machineNamingStrategy.Template != "" { + name, err := names.KCPMachineNameGenerator(machineNamingStrategy.Template, "cluster", "kubeadmcontrolplane").GenerateName() + if err != nil { + allErrs = append(allErrs, + field.Invalid( + pathPrefix.Child("template"), + machineNamingStrategy.Template, + fmt.Sprintf("invalid ControlPlane name template: %v", err), + )) + } else { + for _, err := range validation.IsDNS1123Subdomain(name) { + allErrs = append(allErrs, field.Invalid(pathPrefix.Child("template"), machineNamingStrategy.Template, err)) + } + } + } + + return allErrs +} + func validateClusterConfiguration(oldClusterConfiguration, newClusterConfiguration *bootstrapv1.ClusterConfiguration, pathPrefix *field.Path) field.ErrorList { allErrs := field.ErrorList{} diff --git a/controlplane/kubeadm/internal/webhooks/kubeadmcontrolplanetemplate.go b/controlplane/kubeadm/internal/webhooks/kubeadmcontrolplanetemplate.go index ad58d472be2a..9acda9c33bf7 100644 --- a/controlplane/kubeadm/internal/webhooks/kubeadmcontrolplanetemplate.go +++ b/controlplane/kubeadm/internal/webhooks/kubeadmcontrolplanetemplate.go @@ -141,6 +141,9 @@ func validateKubeadmControlPlaneTemplateResourceSpec(s controlplanev1.KubeadmCon allErrs = append(allErrs, validateRolloutBefore(s.RolloutBefore, pathPrefix.Child("rolloutBefore"))...) allErrs = append(allErrs, validateRolloutStrategy(s.RolloutStrategy, nil, pathPrefix.Child("rolloutStrategy"))...) + if s.MachineNamingStrategy != nil { + allErrs = append(allErrs, validateNamingStrategy(s.MachineNamingStrategy, pathPrefix.Child("machineNamingStrategy"))...) + } if s.MachineTemplate != nil { // Validate the metadata of the MachineTemplate diff --git a/internal/apis/controlplane/kubeadm/v1alpha3/zz_generated.conversion.go b/internal/apis/controlplane/kubeadm/v1alpha3/zz_generated.conversion.go index 9dcb33c824b2..f3621923c185 100644 --- a/internal/apis/controlplane/kubeadm/v1alpha3/zz_generated.conversion.go +++ b/internal/apis/controlplane/kubeadm/v1alpha3/zz_generated.conversion.go @@ -213,6 +213,7 @@ func autoConvert_v1beta1_KubeadmControlPlaneSpec_To_v1alpha3_KubeadmControlPlane // WARNING: in.RolloutAfter requires manual conversion: does not exist in peer-type out.RolloutStrategy = (*RolloutStrategy)(unsafe.Pointer(in.RolloutStrategy)) // WARNING: in.RemediationStrategy requires manual conversion: does not exist in peer-type + // WARNING: in.MachineNamingStrategy requires manual conversion: does not exist in peer-type return nil } diff --git a/internal/apis/controlplane/kubeadm/v1alpha4/zz_generated.conversion.go b/internal/apis/controlplane/kubeadm/v1alpha4/zz_generated.conversion.go index 4efbb1b7b795..325fe2564ad2 100644 --- a/internal/apis/controlplane/kubeadm/v1alpha4/zz_generated.conversion.go +++ b/internal/apis/controlplane/kubeadm/v1alpha4/zz_generated.conversion.go @@ -317,6 +317,7 @@ func autoConvert_v1beta1_KubeadmControlPlaneSpec_To_v1alpha4_KubeadmControlPlane out.RolloutAfter = (*v1.Time)(unsafe.Pointer(in.RolloutAfter)) out.RolloutStrategy = (*RolloutStrategy)(unsafe.Pointer(in.RolloutStrategy)) // WARNING: in.RemediationStrategy requires manual conversion: does not exist in peer-type + // WARNING: in.MachineNamingStrategy requires manual conversion: does not exist in peer-type return nil } diff --git a/internal/topology/names/names.go b/internal/topology/names/names.go index 33e419c3584a..98d3c8a2fe89 100644 --- a/internal/topology/names/names.go +++ b/internal/topology/names/names.go @@ -86,6 +86,12 @@ func MachinePoolNameGenerator(templateString, clusterName, topologyName string) }) } +// KCPMachineNameGenerator returns a generator for creating a kcp machine name. +func KCPMachineNameGenerator(templateString, clusterName, kubeadmControlPlaneName string) NameGenerator { + return newTemplateGeneratorWithKCP(templateString, clusterName, kubeadmControlPlaneName, + map[string]interface{}{}) +} + // templateGenerator parses the template string as text/template and executes it using // the passed data to generate a name. type templateGenerator struct { @@ -105,6 +111,21 @@ func newTemplateGenerator(template, clusterName string, data map[string]interfac } } +func newTemplateGeneratorWithKCP(template, clusterName string, kubeadmControlPlaneName string, data map[string]interface{}) NameGenerator { + data["cluster"] = map[string]interface{}{ + "name": clusterName, + } + data["kubeadmcontrolplane"] = map[string]interface{}{ + "name": kubeadmControlPlaneName, + } + data["random"] = utilrand.String(randomLength) + + return &templateGenerator{ + template: template, + data: data, + } +} + func (g *templateGenerator) GenerateName() (string, error) { tpl, err := template.New("template name generator").Option("missingkey=error").Parse(g.template) if err != nil {