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 committed Dec 13, 2023
1 parent fcae182 commit 8d62deb
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 40 deletions.
14 changes: 13 additions & 1 deletion controllers/external/util.go
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 @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion controllers/external/util_test.go
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
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
}
22 changes: 10 additions & 12 deletions controlplane/kubeadm/internal/controllers/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
Expand Down Expand Up @@ -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,
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
4 changes: 2 additions & 2 deletions controlplane/kubeadm/internal/controllers/scale_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
Expand Down
3 changes: 1 addition & 2 deletions exp/internal/controllers/machinepool_controller_phases.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions internal/controllers/machineset/machineset_controller.go
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 8d62deb

Please sign in to comment.