From 0e8bc1653f860dd14a24a03d8825f8c1d6a8433f Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Fri, 8 Dec 2023 13:21:23 +0100 Subject: [PATCH] Use the same names for Machine objects generated by KCP & MachineSets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stefan Büringer buringerst@vmware.com --- controllers/external/util.go | 12 +++++ controllers/external/util_test.go | 3 +- .../kubeadm/internal/controllers/helpers.go | 47 ++++++++++--------- .../internal/controllers/helpers_test.go | 22 ++++----- .../internal/controllers/scale_test.go | 4 +- .../machineset/machineset_controller.go | 2 + 6 files changed, 53 insertions(+), 37 deletions(-) diff --git a/controllers/external/util.go b/controllers/external/util.go index 89647454417d..3e99228ad974 100644 --- a/controllers/external/util.go +++ b/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 @@ -157,6 +166,9 @@ func GenerateTemplate(in *GenerateTemplateInput) (*unstructured.Unstructured, er to.SetUID("") to.SetSelfLink("") to.SetName(names.SimpleNameGenerator.GenerateName(in.Template.GetName() + "-")) + if in.Name != "" { + to.SetName(in.Name) + } to.SetNamespace(in.Namespace) // Set annotations. diff --git a/controllers/external/util_test.go b/controllers/external/util_test.go index 012445478e8a..f10eefab3687 100644 --- a/controllers/external/util_test.go +++ b/controllers/external/util_test.go @@ -265,6 +265,7 @@ func TestCloneTemplateResourceFoundNoOwner(t *testing.T) { Client: fakeClient, TemplateRef: templateRef, Namespace: metav1.NamespaceDefault, + Name: "object-name", ClusterName: testClusterName, }) g.Expect(err).ToNot(HaveOccurred()) @@ -272,7 +273,7 @@ func TestCloneTemplateResourceFoundNoOwner(t *testing.T) { 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) diff --git a/controlplane/kubeadm/internal/controllers/helpers.go b/controlplane/kubeadm/internal/controllers/helpers.go index 49431f90a729..d03784ea2f38 100644 --- a/controlplane/kubeadm/internal/controllers/helpers.go +++ b/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 } diff --git a/controlplane/kubeadm/internal/controllers/helpers_test.go b/controlplane/kubeadm/internal/controllers/helpers_test.go index 1449618efa8b..83993215a16e 100644 --- a/controlplane/kubeadm/internal/controllers/helpers_test.go +++ b/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 := pointer.String("fd1") createdMachine, err := (&KubeadmControlPlaneReconciler{}).computeDesiredMachine( kcp, cluster, - infraRef, bootstrapRef, failureDomain, nil, ) g.Expect(err).ToNot(HaveOccurred()) expectedMachineSpec := clusterv1.MachineSpec{ - ClusterName: cluster.Name, - Version: pointer.String(kcp.Spec.Version), - Bootstrap: clusterv1.Bootstrap{ - ConfigRef: bootstrapRef, - }, - InfrastructureRef: *infraRef, + ClusterName: cluster.Name, + Version: pointer.String(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)) diff --git a/controlplane/kubeadm/internal/controllers/scale_test.go b/controlplane/kubeadm/internal/controllers/scale_test.go index f40396259389..2ef540db42d4 100644 --- a/controlplane/kubeadm/internal/controllers/scale_test.go +++ b/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")) } diff --git a/internal/controllers/machineset/machineset_controller.go b/internal/controllers/machineset/machineset_controller.go index 1b93130f7c81..25b356f8a75c 100644 --- a/internal/controllers/machineset/machineset_controller.go +++ b/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,