Skip to content

Commit

Permalink
Use the same names for Machine objects generated by KCP & MachineSets
Browse files Browse the repository at this point in the history 
Signed-off-by: Stefan Büringer buringerst@vmware.com
  • Loading branch information
@sbueringer
sbueringer committed Dec 8, 2023
1 parent 351a463 commit 76bb6da
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 33 deletions.
12 changes: 12 additions & 0 deletions controllers/external/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion controllers/external/util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
47 changes: 25 additions & 22 deletions controlplane/kubeadm/internal/controllers/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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),
Expand All @@ -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())
Expand All @@ -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"))
Expand Down Expand Up @@ -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(),
Expand All @@ -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,
Expand Down Expand Up @@ -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")
}
Expand All @@ -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")
}
Expand All @@ -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
Expand Down Expand Up @@ -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,
},
}

Expand All @@ -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
}
18 changes: 8 additions & 10 deletions controlplane/kubeadm/internal/controllers/helpers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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))
Expand Down
2 changes: 2 additions & 0 deletions internal/controllers/machineset/machineset_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down

0 comments on commit 76bb6da

Please sign in to comment.