Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚠️ Objects generated by KCP, MachineSets and MachinePools will now consistently use machine name #9833

Merged
merged 1 commit into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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