Skip to content

Commit

Permalink
refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
Yuvaraj Kakaraparthi committed Feb 16, 2023
1 parent 3f04023 commit 8f1b10a
Show file tree
Hide file tree
Showing 3 changed files with 197 additions and 99 deletions.
4 changes: 2 additions & 2 deletions controlplane/kubeadm/internal/controllers/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ func (r *KubeadmControlPlaneReconciler) syncKubeadmConfigs(ctx context.Context,
if err := ssa.CleanUpManagedFieldsForSSAAdoption(ctx, kubeadmConfig, kcpManagerName, r.Client); err != nil {
return errors.Wrapf(err, "failed to clean up managedFields of KubeadmConfig %s", klog.KObj(kubeadmConfig))
}
_, err := r.applyKubeadmConfig(ctx, controlPlane.KCP, controlPlane.Cluster, &kubeadmConfig.Spec, kubeadmConfig.Name, kubeadmConfig.UID)
_, err := r.updateKubeadmConfig(ctx, controlPlane.KCP, controlPlane.Cluster, kubeadmConfig)
if err != nil {
return errors.Wrapf(err, "failed to update KubeadmConfig %s", klog.KObj(kubeadmConfig))
}
Expand Down Expand Up @@ -764,7 +764,7 @@ func (r *KubeadmControlPlaneReconciler) adoptMachines(ctx context.Context, kcp *
ref := m.Spec.Bootstrap.ConfigRef

// TODO instead of returning error here, we should instead Event and add a watch on potentially adoptable Machines
if ref == nil || ref.Kind != "KubeadmConfig" {
if ref == nil || ref.Kind != kubeadmConfigKind {
return errors.Errorf("unable to adopt Machine %v/%v: expected a ConfigRef of kind KubeadmConfig but instead found %v", m.Namespace, m.Name, ref)
}

Expand Down
71 changes: 49 additions & 22 deletions controlplane/kubeadm/internal/controllers/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ import (
"sigs.k8s.io/cluster-api/util/secret"
)

const kubeadmConfigKind = "KubeadmConfig"

func (r *KubeadmControlPlaneReconciler) reconcileKubeconfig(ctx context.Context, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane) (ctrl.Result, error) {
log := ctrl.LoggerFrom(ctx)

Expand Down Expand Up @@ -189,7 +191,7 @@ func (r *KubeadmControlPlaneReconciler) cloneConfigsAndGenerateMachine(ctx conte
}

// Clone the bootstrap configuration
bootstrapRef, err := r.applyKubeadmConfig(ctx, kcp, cluster, bootstrapSpec, "", "")
bootstrapRef, err := r.createKubeadmConfig(ctx, kcp, cluster, bootstrapSpec)
if err != nil {
conditions.MarkFalse(kcp, controlplanev1.MachinesCreatedCondition, controlplanev1.BootstrapTemplateCloningFailedReason,
clusterv1.ConditionSeverityError, err.Error())
Expand Down Expand Up @@ -238,7 +240,49 @@ func (r *KubeadmControlPlaneReconciler) cleanupFromGeneration(ctx context.Contex
return kerrors.NewAggregate(errs)
}

func (r *KubeadmControlPlaneReconciler) applyKubeadmConfig(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, cluster *clusterv1.Cluster, spec *bootstrapv1.KubeadmConfigSpec, name string, uid types.UID) (*corev1.ObjectReference, error) {
func (r *KubeadmControlPlaneReconciler) createKubeadmConfig(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, cluster *clusterv1.Cluster, spec *bootstrapv1.KubeadmConfigSpec) (*corev1.ObjectReference, error) {
kubeadmConfig := r.computeDesiredKubeadmConfig(kcp, cluster, spec, "", "")
patchOptions := []client.PatchOption{
client.ForceOwnership,
client.FieldOwner(kcpManagerName),
}
if err := r.Client.Patch(ctx, kubeadmConfig, client.Apply, patchOptions...); err != nil {
return nil, errors.Wrap(err, "failed to create KubeadmConfig: Apply failed")
}
return getReference(kubeadmConfig), nil
}

func (r *KubeadmControlPlaneReconciler) updateKubeadmConfig(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, cluster *clusterv1.Cluster, kubeadmConfig *bootstrapv1.KubeadmConfig) (*corev1.ObjectReference, error) {
updatedKubeadmConfig := r.computeDesiredKubeadmConfig(kcp, cluster, &kubeadmConfig.Spec, kubeadmConfig.Name, kubeadmConfig.UID)
patchOptions := []client.PatchOption{
client.ForceOwnership,
client.FieldOwner(kcpManagerName),
}
if err := r.Client.Patch(ctx, updatedKubeadmConfig, client.Apply, patchOptions...); err != nil {
return nil, errors.Wrap(err, "failed to create KubeadmConfig: Apply failed")
}
return getReference(updatedKubeadmConfig), nil
}

func getReference(obj client.Object) *corev1.ObjectReference {
return &corev1.ObjectReference{
APIVersion: obj.GetObjectKind().GroupVersionKind().GroupVersion().String(),
Kind: obj.GetObjectKind().GroupVersionKind().Kind,
Name: obj.GetName(),
Namespace: obj.GetNamespace(),
UID: obj.GetUID(),
}
}

// computeDesiredKubeadmConfig computes the desired KubeadmConfig.
// This KubeadmConfig will be used during reconciliation to:
// * create a KubeadmConfig
// * update an existing KubeadmConfig
// Because we are using Server-Side-Apply we always have to calculate the full object.
// There are small differences in how we calculate the KubeadmConfig depending on if it
// is a create or update. Example: for a new KubeadmConfig we have to calculate a new name,
// while for an existing Machine we have to use the name of the existing KubeadmConfig.
func (r *KubeadmControlPlaneReconciler) computeDesiredKubeadmConfig(kcp *controlplanev1.KubeadmControlPlane, cluster *clusterv1.Cluster, spec *bootstrapv1.KubeadmConfigSpec, name string, uid types.UID) *bootstrapv1.KubeadmConfig {
// 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 @@ -249,7 +293,7 @@ func (r *KubeadmControlPlaneReconciler) applyKubeadmConfig(ctx context.Context,

bootstrapConfig := &bootstrapv1.KubeadmConfig{
TypeMeta: metav1.TypeMeta{
Kind: "KubeadmConfig",
Kind: kubeadmConfigKind,
APIVersion: bootstrapv1.GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -259,32 +303,15 @@ func (r *KubeadmControlPlaneReconciler) applyKubeadmConfig(ctx context.Context,
Annotations: kcp.Spec.MachineTemplate.ObjectMeta.Annotations,
OwnerReferences: []metav1.OwnerReference{owner},
},
Spec: *spec,
Spec: *spec.DeepCopy(),
}
if name != "" {
bootstrapConfig.Name = name
}
if uid != "" {
bootstrapConfig.UID = uid
}

patchOptions := []client.PatchOption{
client.ForceOwnership,
client.FieldOwner(kcpManagerName),
}
if err := r.Client.Patch(ctx, bootstrapConfig, client.Apply, patchOptions...); err != nil {
return nil, errors.Wrap(err, "failed to create KubeadmConfig: Apply failed")
}

bootstrapRef := &corev1.ObjectReference{
APIVersion: bootstrapv1.GroupVersion.String(),
Kind: "KubeadmConfig",
Name: bootstrapConfig.GetName(),
Namespace: bootstrapConfig.GetNamespace(),
UID: bootstrapConfig.GetUID(),
}

return bootstrapRef, nil
return bootstrapConfig
}

func (r *KubeadmControlPlaneReconciler) createInfraMachine(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, cluster *clusterv1.Cluster) (*corev1.ObjectReference, error) {
Expand Down
221 changes: 146 additions & 75 deletions controlplane/kubeadm/internal/controllers/helpers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ func TestKubeadmControlPlaneReconciler_computeDesiredMachine(t *testing.T) {
})
}

func TestKubeadmControlPlaneReconciler_applyKubeadmConfig(t *testing.T) {
func TestKubeadmControlPlaneReconciler_createKubeadmConfig(t *testing.T) {
setup := func(t *testing.T, g *WithT) *corev1.Namespace {
t.Helper()

Expand Down Expand Up @@ -757,87 +757,158 @@ func TestKubeadmControlPlaneReconciler_applyKubeadmConfig(t *testing.T) {
UID: kcp.UID,
}
r := &KubeadmControlPlaneReconciler{Client: env}
t.Run("should create a new KubeadmConfig", func(t *testing.T) {
// Create the KubeadmConfig
got, err := r.applyKubeadmConfig(ctx, kcp, cluster, kubeadmConfigSpec, "", "")
// Create the KubeadmConfig
got, err := r.createKubeadmConfig(ctx, kcp, cluster, kubeadmConfigSpec)
g.Expect(err).To(BeNil())
g.Expect(got).NotTo(BeNil())
g.Expect(got.Name).To(HavePrefix(kcp.Name))
g.Expect(got.Namespace).To(Equal(kcp.Namespace))
g.Expect(got.Kind).To(Equal(expectedReferenceKind))
g.Expect(got.APIVersion).To(Equal(expectedReferenceAPIVersion))

bootstrapConfig := &bootstrapv1.KubeadmConfig{}
key := client.ObjectKey{Name: got.Name, Namespace: got.Namespace}
g.Expect(env.Get(ctx, key, bootstrapConfig)).To(Succeed())
g.Expect(bootstrapConfig.OwnerReferences).To(HaveLen(1))
g.Expect(bootstrapConfig.OwnerReferences).To(ContainElement(expectedOwner))
g.Expect(bootstrapConfig.Spec).To(BeComparableTo(*kubeadmConfigSpec))

// Verify the labels are propagated from the MachineTemplate
// Verify that the machineTemplate.ObjectMeta has been propagated to the Machine.
for k, v := range kcp.Spec.MachineTemplate.ObjectMeta.Labels {
g.Expect(bootstrapConfig.Labels[k]).To(Equal(v))
}
g.Expect(bootstrapConfig.Labels[clusterv1.ClusterNameLabel]).To(Equal(cluster.Name))
g.Expect(bootstrapConfig.Labels[clusterv1.MachineControlPlaneLabel]).To(Equal(""))
g.Expect(bootstrapConfig.Labels[clusterv1.MachineControlPlaneNameLabel]).To(Equal(kcp.Name))

// Verify the annotations are propagated from the MachineTemplate
g.Expect(bootstrapConfig.Annotations).To(Equal(kcp.Spec.MachineTemplate.ObjectMeta.Annotations))
}

func TestKubeadmControlPlaneReconciler_updateKubeadmConfig(t *testing.T) {
setup := func(t *testing.T, g *WithT) *corev1.Namespace {
t.Helper()

t.Log("Creating the namespace")
ns, err := env.CreateNamespace(ctx, "test-applykubeadmconfig")
g.Expect(err).To(BeNil())
g.Expect(got).NotTo(BeNil())
g.Expect(got.Name).To(HavePrefix(kcp.Name))
g.Expect(got.Namespace).To(Equal(kcp.Namespace))
g.Expect(got.Kind).To(Equal(expectedReferenceKind))
g.Expect(got.APIVersion).To(Equal(expectedReferenceAPIVersion))

bootstrapConfig := &bootstrapv1.KubeadmConfig{}
key := client.ObjectKey{Name: got.Name, Namespace: got.Namespace}
g.Expect(env.Get(ctx, key, bootstrapConfig)).To(Succeed())
g.Expect(bootstrapConfig.OwnerReferences).To(HaveLen(1))
g.Expect(bootstrapConfig.OwnerReferences).To(ContainElement(expectedOwner))
g.Expect(bootstrapConfig.Spec).To(BeComparableTo(*kubeadmConfigSpec))

// Verify the labels are propagated from the MachineTemplate
// Verify that the machineTemplate.ObjectMeta has been propagated to the Machine.
for k, v := range kcp.Spec.MachineTemplate.ObjectMeta.Labels {
g.Expect(bootstrapConfig.Labels[k]).To(Equal(v))
}
g.Expect(bootstrapConfig.Labels[clusterv1.ClusterNameLabel]).To(Equal(cluster.Name))
g.Expect(bootstrapConfig.Labels[clusterv1.MachineControlPlaneLabel]).To(Equal(""))
g.Expect(bootstrapConfig.Labels[clusterv1.MachineControlPlaneNameLabel]).To(Equal(kcp.Name))

// Verify the annotations are propagated from the MachineTemplate
g.Expect(bootstrapConfig.Annotations).To(Equal(kcp.Spec.MachineTemplate.ObjectMeta.Annotations))
})
return ns
}

t.Run("should update an existing KubeadmConfig", func(t *testing.T) {
oldKCPMachineTemplateLabels := kcp.Spec.MachineTemplate.ObjectMeta.Labels
// Set new labels on KCP MachineTemplate
kcp.Spec.MachineTemplate.ObjectMeta.Labels = map[string]string{"label-2": "value-2"}
teardown := func(t *testing.T, g *WithT, ns *corev1.Namespace) {
t.Helper()

// Create a KubeadmConfig that will be updated
bootstrapConfig := &bootstrapv1.KubeadmConfig{
TypeMeta: metav1.TypeMeta{
Kind: "KubeadmConfig",
APIVersion: bootstrapv1.GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "test-kubeadmconfig",
Namespace: namespace.Name,
Labels: oldKCPMachineTemplateLabels,
t.Log("Deleting the namespace")
g.Expect(env.Delete(ctx, ns)).To(Succeed())
}

g := NewWithT(t)
namespace := setup(t, g)
defer teardown(t, g, namespace)

cluster := &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "testCluster",
Namespace: namespace.Name,
},
}

kubeadmConfigSpec := &bootstrapv1.KubeadmConfigSpec{
Format: "cloud-config",
Users: []bootstrapv1.User{
{Name: "test-user"},
},
}
kcp := &controlplanev1.KubeadmControlPlane{
ObjectMeta: metav1.ObjectMeta{
Name: "test-control-plane",
Namespace: namespace.Name,
},
Spec: controlplanev1.KubeadmControlPlaneSpec{
Version: "v1.25.3",
KubeadmConfigSpec: *kubeadmConfigSpec,
MachineTemplate: controlplanev1.KubeadmControlPlaneMachineTemplate{
ObjectMeta: clusterv1.ObjectMeta{
Labels: map[string]string{
"label-1": "value-1",
},
Annotations: map[string]string{
"annotation-1": "value-1",
},
},
InfrastructureRef: corev1.ObjectReference{
Kind: "GenericInfrastructureMachineTemplate",
Namespace: namespace.Name,
Name: "control-plane-infra-machine-template",
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
},
},
Spec: *kubeadmConfigSpec.DeepCopy(),
}
g.Expect(env.Patch(ctx, bootstrapConfig, client.Apply, client.ForceOwnership, client.FieldOwner(kcpManagerName))).To(Succeed())
},
}

// Update the KubeadmConfig
got, err := r.applyKubeadmConfig(ctx, kcp, cluster, &bootstrapConfig.Spec, bootstrapConfig.Name, bootstrapConfig.UID)
g.Expect(err).To(BeNil())
g.Expect(got).NotTo(BeNil())
g.Expect(got.UID).To(Equal(bootstrapConfig.UID))
g.Expect(got.Name).To(Equal(bootstrapConfig.Name))
g.Expect(got.Namespace).To(Equal(kcp.Namespace))
g.Expect(got.Kind).To(Equal(expectedReferenceKind))
g.Expect(got.APIVersion).To(Equal(expectedReferenceAPIVersion))

updatedBootstrapConfig := &bootstrapv1.KubeadmConfig{}
key := client.ObjectKey{Name: got.Name, Namespace: got.Namespace}
g.Expect(env.Get(ctx, key, updatedBootstrapConfig)).To(Succeed())
g.Expect(updatedBootstrapConfig.OwnerReferences).To(HaveLen(1))
g.Expect(updatedBootstrapConfig.OwnerReferences).To(ContainElement(expectedOwner))
g.Expect(updatedBootstrapConfig.Spec).To(BeComparableTo(*kubeadmConfigSpec))

// Verify the labels are propagated from the MachineTemplate
// Verify that the machineTemplate.ObjectMeta has been propagated to the Machine.
for k, v := range kcp.Spec.MachineTemplate.ObjectMeta.Labels {
g.Expect(updatedBootstrapConfig.Labels[k]).To(Equal(v))
}
g.Expect(updatedBootstrapConfig.Labels[clusterv1.ClusterNameLabel]).To(Equal(cluster.Name))
g.Expect(updatedBootstrapConfig.Labels[clusterv1.MachineControlPlaneLabel]).To(Equal(""))
g.Expect(updatedBootstrapConfig.Labels[clusterv1.MachineControlPlaneNameLabel]).To(Equal(kcp.Name))
// Creat the KCP object
g.Expect(env.Create(ctx, kcp)).To(Succeed())

// Verify the old labels are not present on the Bootstrap config
for k := range oldKCPMachineTemplateLabels {
g.Expect(updatedBootstrapConfig.Labels).NotTo(HaveKey(k))
}
})
expectedReferenceKind := "KubeadmConfig"
expectedReferenceAPIVersion := bootstrapv1.GroupVersion.String()
expectedOwner := metav1.OwnerReference{
Kind: "KubeadmControlPlane",
APIVersion: controlplanev1.GroupVersion.String(),
Name: kcp.Name,
UID: kcp.UID,
}
r := &KubeadmControlPlaneReconciler{Client: env}
oldKCPMachineTemplateLabels := kcp.Spec.MachineTemplate.ObjectMeta.Labels
// Set new labels on KCP MachineTemplate
kcp.Spec.MachineTemplate.ObjectMeta.Labels = map[string]string{"label-2": "value-2"}

// Create a KubeadmConfig that will be updated
bootstrapConfig := &bootstrapv1.KubeadmConfig{
TypeMeta: metav1.TypeMeta{
Kind: "KubeadmConfig",
APIVersion: bootstrapv1.GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "test-kubeadmconfig",
Namespace: namespace.Name,
Labels: oldKCPMachineTemplateLabels,
},
Spec: *kubeadmConfigSpec.DeepCopy(),
}
g.Expect(env.Patch(ctx, bootstrapConfig, client.Apply, client.ForceOwnership, client.FieldOwner(kcpManagerName))).To(Succeed())

// Update the KubeadmConfig
got, err := r.updateKubeadmConfig(ctx, kcp, cluster, bootstrapConfig)
g.Expect(err).To(BeNil())
g.Expect(got).NotTo(BeNil())
g.Expect(got.UID).To(Equal(bootstrapConfig.UID)) // Verify that UID is the same
g.Expect(got.Name).To(Equal(bootstrapConfig.Name)) // Verify that Name is the same
g.Expect(got.Namespace).To(Equal(kcp.Namespace))
g.Expect(got.Kind).To(Equal(expectedReferenceKind))
g.Expect(got.APIVersion).To(Equal(expectedReferenceAPIVersion))

updatedBootstrapConfig := &bootstrapv1.KubeadmConfig{}
key := client.ObjectKey{Name: got.Name, Namespace: got.Namespace}
g.Expect(env.Get(ctx, key, updatedBootstrapConfig)).To(Succeed())
g.Expect(updatedBootstrapConfig.OwnerReferences).To(HaveLen(1))
g.Expect(updatedBootstrapConfig.OwnerReferences).To(ContainElement(expectedOwner))
g.Expect(updatedBootstrapConfig.Spec).To(BeComparableTo(*kubeadmConfigSpec))

// Verify the labels are propagated from the MachineTemplate
// Verify that the machineTemplate.ObjectMeta has been propagated to the Machine.
for k, v := range kcp.Spec.MachineTemplate.ObjectMeta.Labels {
g.Expect(updatedBootstrapConfig.Labels[k]).To(Equal(v))
}
g.Expect(updatedBootstrapConfig.Labels[clusterv1.ClusterNameLabel]).To(Equal(cluster.Name))
g.Expect(updatedBootstrapConfig.Labels[clusterv1.MachineControlPlaneLabel]).To(Equal(""))
g.Expect(updatedBootstrapConfig.Labels[clusterv1.MachineControlPlaneNameLabel]).To(Equal(kcp.Name))

// Verify the old labels are not present on the Bootstrap config
for k := range oldKCPMachineTemplateLabels {
g.Expect(updatedBootstrapConfig.Labels).NotTo(HaveKey(k))
}
}

func TestKubeadmControlPlaneReconciler_adoptKubeconfigSecret(t *testing.T) {
Expand Down

0 comments on commit 8f1b10a

Please sign in to comment.