Merge pull request #9833 from sbueringer/pr-same-names

⚠️ Use objects generated by KCP, MachineSets and MachinePools will now consistently use machine name

Author: k8s-ci-robot

controllers/external/util.go

@@ -70,6 +70,10 @@ type CreateFromTemplateInput struct {
 	// Namespace is the Kubernetes namespace the cloned object should be created into.
 	Namespace string
 
+	// Name is used as the name of the generated object, if set.
+	// If it isn't set the template name will be used as prefix to generate a name instead.
+	Name string
+
 	// ClusterName is the cluster this object is linked to.
 	ClusterName string
 
@@ -96,6 +100,7 @@ func CreateFromTemplate(ctx context.Context, in *CreateFromTemplateInput) (*core
 		Template:    from,
 		TemplateRef: in.TemplateRef,
 		Namespace:   in.Namespace,
+		Name:        in.Name,
 		ClusterName: in.ClusterName,
 		OwnerRef:    in.OwnerRef,
 		Labels:      in.Labels,
@@ -125,6 +130,10 @@ type GenerateTemplateInput struct {
 	// Namespace is the Kubernetes namespace the cloned object should be created into.
 	Namespace string
 
+	// Name is used as the name of the generated object, if set.
+	// If it isn't set the template name will be used as prefix to generate a name instead.
+	Name string
+
 	// ClusterName is the cluster this object is linked to.
 	ClusterName string
 
@@ -156,7 +165,10 @@ func GenerateTemplate(in *GenerateTemplateInput) (*unstructured.Unstructured, er
 	to.SetFinalizers(nil)
 	to.SetUID("")
 	to.SetSelfLink("")
-	to.SetName(names.SimpleNameGenerator.GenerateName(in.Template.GetName() + "-"))
+	to.SetName(in.Name)
+	if to.GetName() == "" {
+		to.SetName(names.SimpleNameGenerator.GenerateName(in.Template.GetName() + "-"))
+	}
 	to.SetNamespace(in.Namespace)
 
 	// Set annotations.

controllers/external/util_test.go

@@ -265,14 +265,15 @@ func TestCloneTemplateResourceFoundNoOwner(t *testing.T) {
 		Client:      fakeClient,
 		TemplateRef: templateRef,
 		Namespace:   metav1.NamespaceDefault,
+		Name:        "object-name",
 		ClusterName: testClusterName,
 	})
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(ref).NotTo(BeNil())
 	g.Expect(ref.Kind).To(Equal(expectedKind))
 	g.Expect(ref.APIVersion).To(Equal(expectedAPIVersion))
 	g.Expect(ref.Namespace).To(Equal(metav1.NamespaceDefault))
-	g.Expect(ref.Name).To(HavePrefix(templateRef.Name))
+	g.Expect(ref.Name).To(Equal("object-name"))
 
 	clone := &unstructured.Unstructured{}
 	clone.SetKind(expectedKind)

controlplane/kubeadm/internal/controllers/helpers.go

@@ -165,6 +165,12 @@ func (r *KubeadmControlPlaneReconciler) reconcileExternalReference(ctx context.C
 func (r *KubeadmControlPlaneReconciler) cloneConfigsAndGenerateMachine(ctx context.Context, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, bootstrapSpec *bootstrapv1.KubeadmConfigSpec, failureDomain *string) error {
 	var errs []error
 
+	// Compute desired Machine
+	machine, err := r.computeDesiredMachine(kcp, cluster, failureDomain, nil)
+	if err != nil {
+		return errors.Wrap(err, "failed to create Machine: failed to compute desired Machine")
+	}
+
 	// Since the cloned resource should eventually have a controller ref for the Machine, we create an
 	// OwnerReference here without the Controller field set
 	infraCloneOwner := &metav1.OwnerReference{
@@ -179,6 +185,7 @@ func (r *KubeadmControlPlaneReconciler) cloneConfigsAndGenerateMachine(ctx conte
 		Client:      r.Client,
 		TemplateRef: &kcp.Spec.MachineTemplate.InfrastructureRef,
 		Namespace:   kcp.Namespace,
+		Name:        machine.Name,
 		OwnerRef:    infraCloneOwner,
 		ClusterName: cluster.Name,
 		Labels:      internal.ControlPlaneMachineLabelsForCluster(kcp, cluster.Name),
@@ -190,9 +197,10 @@ func (r *KubeadmControlPlaneReconciler) cloneConfigsAndGenerateMachine(ctx conte
 			clusterv1.ConditionSeverityError, err.Error())
 		return errors.Wrap(err, "failed to clone infrastructure template")
 	}
+	machine.Spec.InfrastructureRef = *infraRef
 
 	// Clone the bootstrap configuration
-	bootstrapRef, err := r.generateKubeadmConfig(ctx, kcp, cluster, bootstrapSpec)
+	bootstrapRef, err := r.generateKubeadmConfig(ctx, kcp, cluster, bootstrapSpec, machine.Name)
 	if err != nil {
 		conditions.MarkFalse(kcp, controlplanev1.MachinesCreatedCondition, controlplanev1.BootstrapTemplateCloningFailedReason,
 			clusterv1.ConditionSeverityError, err.Error())
@@ -201,7 +209,9 @@ func (r *KubeadmControlPlaneReconciler) cloneConfigsAndGenerateMachine(ctx conte
 
 	// Only proceed to generating the Machine if we haven't encountered an error
 	if len(errs) == 0 {
-		if err := r.createMachine(ctx, kcp, cluster, infraRef, bootstrapRef, failureDomain); err != nil {
+		machine.Spec.Bootstrap.ConfigRef = bootstrapRef
+
+		if err := r.createMachine(ctx, kcp, machine); err != nil {
 			conditions.MarkFalse(kcp, controlplanev1.MachinesCreatedCondition, controlplanev1.MachineGenerationFailedReason,
 				clusterv1.ConditionSeverityError, err.Error())
 			errs = append(errs, errors.Wrap(err, "failed to create Machine"))
@@ -241,7 +251,7 @@ func (r *KubeadmControlPlaneReconciler) cleanupFromGeneration(ctx context.Contex
 	return kerrors.NewAggregate(errs)
 }
 
-func (r *KubeadmControlPlaneReconciler) generateKubeadmConfig(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, cluster *clusterv1.Cluster, spec *bootstrapv1.KubeadmConfigSpec) (*corev1.ObjectReference, error) {
+func (r *KubeadmControlPlaneReconciler) generateKubeadmConfig(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, cluster *clusterv1.Cluster, spec *bootstrapv1.KubeadmConfigSpec, name string) (*corev1.ObjectReference, error) {
 	// Create an owner reference without a controller reference because the owning controller is the machine controller
 	owner := metav1.OwnerReference{
 		APIVersion: controlplanev1.GroupVersion.String(),
@@ -252,7 +262,7 @@ func (r *KubeadmControlPlaneReconciler) generateKubeadmConfig(ctx context.Contex
 
 	bootstrapConfig := &bootstrapv1.KubeadmConfig{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:            names.SimpleNameGenerator.GenerateName(kcp.Name + "-"),
+			Name:            name,
 			Namespace:       kcp.Namespace,
 			Labels:          internal.ControlPlaneMachineLabelsForCluster(kcp, cluster.Name),
 			Annotations:     kcp.Spec.MachineTemplate.ObjectMeta.Annotations,
@@ -297,11 +307,7 @@ func (r *KubeadmControlPlaneReconciler) updateExternalObject(ctx context.Context
 	return nil
 }
 
-func (r *KubeadmControlPlaneReconciler) createMachine(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, cluster *clusterv1.Cluster, infraRef, bootstrapRef *corev1.ObjectReference, failureDomain *string) error {
-	machine, err := r.computeDesiredMachine(kcp, cluster, infraRef, bootstrapRef, failureDomain, nil)
-	if err != nil {
-		return errors.Wrap(err, "failed to create Machine: failed to compute desired Machine")
-	}
+func (r *KubeadmControlPlaneReconciler) createMachine(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, machine *clusterv1.Machine) error {
 	if err := ssa.Patch(ctx, r.Client, kcpManagerName, machine); err != nil {
 		return errors.Wrap(err, "failed to create Machine")
 	}
@@ -312,11 +318,7 @@ func (r *KubeadmControlPlaneReconciler) createMachine(ctx context.Context, kcp *
 }
 
 func (r *KubeadmControlPlaneReconciler) updateMachine(ctx context.Context, machine *clusterv1.Machine, kcp *controlplanev1.KubeadmControlPlane, cluster *clusterv1.Cluster) (*clusterv1.Machine, error) {
-	updatedMachine, err := r.computeDesiredMachine(
-		kcp, cluster,
-		&machine.Spec.InfrastructureRef, machine.Spec.Bootstrap.ConfigRef,
-		machine.Spec.FailureDomain, machine,
-	)
+	updatedMachine, err := r.computeDesiredMachine(kcp, cluster, machine.Spec.FailureDomain, machine)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to update Machine: failed to compute desired Machine")
 	}
@@ -336,7 +338,7 @@ func (r *KubeadmControlPlaneReconciler) updateMachine(ctx context.Context, machi
 // 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 *KubeadmControlPlaneReconciler) computeDesiredMachine(kcp *controlplanev1.KubeadmControlPlane, cluster *clusterv1.Cluster, infraRef, bootstrapRef *corev1.ObjectReference, failureDomain *string, existingMachine *clusterv1.Machine) (*clusterv1.Machine, error) {
+func (r *KubeadmControlPlaneReconciler) computeDesiredMachine(kcp *controlplanev1.KubeadmControlPlane, cluster *clusterv1.Cluster, failureDomain *string, existingMachine *clusterv1.Machine) (*clusterv1.Machine, error) {
 	var machineName string
 	var machineUID types.UID
 	var version *string
@@ -399,13 +401,9 @@ func (r *KubeadmControlPlaneReconciler) computeDesiredMachine(kcp *controlplanev
 			Annotations: map[string]string{},
 		},
 		Spec: clusterv1.MachineSpec{
-			ClusterName:       cluster.Name,
-			Version:           version,
-			FailureDomain:     failureDomain,
-			InfrastructureRef: *infraRef,
-			Bootstrap: clusterv1.Bootstrap{
-				ConfigRef: bootstrapRef,
-			},
+			ClusterName:   cluster.Name,
+			Version:       version,
+			FailureDomain: failureDomain,
 		},
 	}
 
@@ -431,5 +429,10 @@ func (r *KubeadmControlPlaneReconciler) computeDesiredMachine(kcp *controlplanev
 	desiredMachine.Spec.NodeDeletionTimeout = kcp.Spec.MachineTemplate.NodeDeletionTimeout
 	desiredMachine.Spec.NodeVolumeDetachTimeout = kcp.Spec.MachineTemplate.NodeVolumeDetachTimeout
 
+	if existingMachine != nil {
+		desiredMachine.Spec.InfrastructureRef = existingMachine.Spec.InfrastructureRef
+		desiredMachine.Spec.Bootstrap.ConfigRef = existingMachine.Spec.Bootstrap.ConfigRef
+	}
+
 	return desiredMachine, nil
 }

controlplane/kubeadm/internal/controllers/helpers_test.go

@@ -386,12 +386,12 @@ func TestCloneConfigsAndGenerateMachine(t *testing.T) {
 		g.Expect(infraObj.GetAnnotations()).To(HaveKeyWithValue(clusterv1.TemplateClonedFromGroupKindAnnotation, genericInfrastructureMachineTemplate.GroupVersionKind().GroupKind().String()))
 
 		g.Expect(m.Spec.InfrastructureRef.Namespace).To(Equal(cluster.Namespace))
-		g.Expect(m.Spec.InfrastructureRef.Name).To(HavePrefix(genericInfrastructureMachineTemplate.GetName()))
+		g.Expect(m.Spec.InfrastructureRef.Name).To(Equal(m.Name))
 		g.Expect(m.Spec.InfrastructureRef.APIVersion).To(Equal(genericInfrastructureMachineTemplate.GetAPIVersion()))
 		g.Expect(m.Spec.InfrastructureRef.Kind).To(Equal("GenericInfrastructureMachine"))
 
 		g.Expect(m.Spec.Bootstrap.ConfigRef.Namespace).To(Equal(cluster.Namespace))
-		g.Expect(m.Spec.Bootstrap.ConfigRef.Name).To(HavePrefix(kcp.Name))
+		g.Expect(m.Spec.Bootstrap.ConfigRef.Name).To(Equal(m.Name))
 		g.Expect(m.Spec.Bootstrap.ConfigRef.APIVersion).To(Equal(bootstrapv1.GroupVersion.String()))
 		g.Expect(m.Spec.Bootstrap.ConfigRef.Kind).To(Equal("KubeadmConfig"))
 	}
@@ -528,18 +528,13 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
 		failureDomain := ptr.To("fd1")
 		createdMachine, err := (&KubeadmControlPlaneReconciler{}).computeDesiredMachine(
 			kcp, cluster,
-			infraRef, bootstrapRef,
 			failureDomain, nil,
 		)
 		g.Expect(err).ToNot(HaveOccurred())
 
 		expectedMachineSpec := clusterv1.MachineSpec{
-			ClusterName: cluster.Name,
-			Version:     ptr.To(kcp.Spec.Version),
-			Bootstrap: clusterv1.Bootstrap{
-				ConfigRef: bootstrapRef,
-			},
-			InfrastructureRef:       *infraRef,
+			ClusterName:             cluster.Name,
+			Version:                 ptr.To(kcp.Spec.Version),
 			FailureDomain:           failureDomain,
 			NodeDrainTimeout:        kcp.Spec.MachineTemplate.NodeDrainTimeout,
 			NodeDeletionTimeout:     kcp.Spec.MachineTemplate.NodeDeletionTimeout,
@@ -600,12 +595,15 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
 				NodeDrainTimeout:        duration10s,
 				NodeDeletionTimeout:     duration10s,
 				NodeVolumeDetachTimeout: duration10s,
+				Bootstrap: clusterv1.Bootstrap{
+					ConfigRef: bootstrapRef,
+				},
+				InfrastructureRef: *infraRef,
 			},
 		}
 
 		updatedMachine, err := (&KubeadmControlPlaneReconciler{}).computeDesiredMachine(
 			kcp, cluster,
-			infraRef, bootstrapRef,
 			existingMachine.Spec.FailureDomain, existingMachine,
 		)
 		g.Expect(err).ToNot(HaveOccurred())
@@ -689,10 +687,10 @@ func TestKubeadmControlPlaneReconciler_generateKubeadmConfig(t *testing.T) {
 		recorder:            record.NewFakeRecorder(32),
 	}
 
-	got, err := r.generateKubeadmConfig(ctx, kcp, cluster, spec.DeepCopy())
+	got, err := r.generateKubeadmConfig(ctx, kcp, cluster, spec.DeepCopy(), "kubeadmconfig-name")
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(got).NotTo(BeNil())
-	g.Expect(got.Name).To(HavePrefix(kcp.Name))
+	g.Expect(got.Name).To(Equal("kubeadmconfig-name"))
 	g.Expect(got.Namespace).To(Equal(kcp.Namespace))
 	g.Expect(got.Kind).To(Equal(expectedReferenceKind))
 	g.Expect(got.APIVersion).To(Equal(expectedReferenceAPIVersion))

controlplane/kubeadm/internal/controllers/scale_test.go

@@ -94,12 +94,12 @@ func TestKubeadmControlPlaneReconciler_initializeControlPlane(t *testing.T) {
 	g.Expect(machineList.Items[0].Name).To(HavePrefix(kcp.Name))
 
 	g.Expect(machineList.Items[0].Spec.InfrastructureRef.Namespace).To(Equal(cluster.Namespace))
-	g.Expect(machineList.Items[0].Spec.InfrastructureRef.Name).To(HavePrefix(genericInfrastructureMachineTemplate.GetName()))
+	g.Expect(machineList.Items[0].Spec.InfrastructureRef.Name).To(Equal(machineList.Items[0].Name))
 	g.Expect(machineList.Items[0].Spec.InfrastructureRef.APIVersion).To(Equal(genericInfrastructureMachineTemplate.GetAPIVersion()))
 	g.Expect(machineList.Items[0].Spec.InfrastructureRef.Kind).To(Equal("GenericInfrastructureMachine"))
 
 	g.Expect(machineList.Items[0].Spec.Bootstrap.ConfigRef.Namespace).To(Equal(cluster.Namespace))
-	g.Expect(machineList.Items[0].Spec.Bootstrap.ConfigRef.Name).To(HavePrefix(kcp.Name))
+	g.Expect(machineList.Items[0].Spec.Bootstrap.ConfigRef.Name).To(Equal(machineList.Items[0].Name))
 	g.Expect(machineList.Items[0].Spec.Bootstrap.ConfigRef.APIVersion).To(Equal(bootstrapv1.GroupVersion.String()))
 	g.Expect(machineList.Items[0].Spec.Bootstrap.ConfigRef.Kind).To(Equal("KubeadmConfig"))
 }

exp/internal/controllers/machinepool_controller_phases.go

@@ -29,7 +29,6 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/apiserver/pkg/storage/names"
 	"k8s.io/klog/v2"
 	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -443,7 +442,7 @@ func computeDesiredMachine(mp *expv1.MachinePool, infraMachine *unstructured.Uns
 
 	machine := &clusterv1.Machine{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: names.SimpleNameGenerator.GenerateName(fmt.Sprintf("%s-", mp.Name)),
+			Name: infraMachine.GetName(),
 			// 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(mp, machinePoolKind)},
 			Namespace:       mp.Namespace,

internal/controllers/machineset/machineset_controller.go

@@ -475,6 +475,7 @@ func (r *Reconciler) syncReplicas(ctx context.Context, cluster *clusterv1.Cluste
 					Client:      r.UnstructuredCachingClient,
 					TemplateRef: ms.Spec.Template.Spec.Bootstrap.ConfigRef,
 					Namespace:   machine.Namespace,
+					Name:        machine.Name,
 					ClusterName: machine.Spec.ClusterName,
 					Labels:      machine.Labels,
 					Annotations: machine.Annotations,
@@ -500,6 +501,7 @@ func (r *Reconciler) syncReplicas(ctx context.Context, cluster *clusterv1.Cluste
 				Client:      r.UnstructuredCachingClient,
 				TemplateRef: &ms.Spec.Template.Spec.InfrastructureRef,
 				Namespace:   machine.Namespace,
+				Name:        machine.Name,
 				ClusterName: machine.Spec.ClusterName,
 				Labels:      machine.Labels,
 				Annotations: machine.Annotations,