From 520299ea1bea7760cc82c30e7dbbb8539cff1a21 Mon Sep 17 00:00:00 2001 From: Vince Prignano Date: Fri, 17 Jan 2020 11:35:47 -0800 Subject: [PATCH] Refactor CloneTemplate / propagate clone labels in MS Signed-off-by: Vince Prignano --- controllers/external/util.go | 61 +++++++++++++---- controllers/external/util_test.go | 68 +++++++++---------- controllers/machineset_controller.go | 16 ++++- .../kubeadm_control_plane_controller.go | 17 ++--- 4 files changed, 103 insertions(+), 59 deletions(-) diff --git a/controllers/external/util.go b/controllers/external/util.go index 528c4e228db2..845f7208c170 100644 --- a/controllers/external/util.go +++ b/controllers/external/util.go @@ -49,9 +49,35 @@ func Get(ctx context.Context, c client.Client, ref *corev1.ObjectReference, name return obj, nil } +type CloneTemplateInput struct { + // Client is the controller runtime client. + // +required + Client client.Client + + // TemplateRef is a reference to the template that needs to be cloned. + // +required + TemplateRef *corev1.ObjectReference + + // Namespace is the Kuberentes namespace the cloned object should be created into. + // +required + Namespace string + + // ClusterName is the cluster this object is linked to. + // +required + ClusterName string + + // OwnerRef is an optional OwnerReference to attach to the cloned object. + // +optional + OwnerRef *metav1.OwnerReference + + // Labels is an optional map of labels to be added to the object. + // +optional + Labels map[string]string +} + // CloneTemplate uses the client and the reference to create a new object from the template. -func CloneTemplate(ctx context.Context, c client.Client, ref *corev1.ObjectReference, namespace, clusterName string, owner *metav1.OwnerReference) (*corev1.ObjectReference, error) { - from, err := Get(ctx, c, ref, namespace) +func CloneTemplate(ctx context.Context, in *CloneTemplateInput) (*corev1.ObjectReference, error) { + from, err := Get(ctx, in.Client, in.TemplateRef, in.Namespace) if err != nil { return nil, err } @@ -65,41 +91,50 @@ func CloneTemplate(ctx context.Context, c client.Client, ref *corev1.ObjectRefer // Create the unstructured object from the template. to := &unstructured.Unstructured{Object: template} to.SetResourceVersion("") - to.SetLabels(map[string]string{clusterv1.ClusterLabelName: clusterName}) to.SetFinalizers(nil) to.SetUID("") to.SetSelfLink("") to.SetName(names.SimpleNameGenerator.GenerateName(from.GetName() + "-")) - to.SetNamespace(namespace) + to.SetNamespace(in.Namespace) - if owner != nil { - to.SetOwnerReferences([]metav1.OwnerReference{*owner}) + // Set labels. + labels := to.GetLabels() + if labels == nil { + labels = map[string]string{} + } + for key, value := range in.Labels { + labels[key] = value + } + labels[clusterv1.ClusterLabelName] = in.ClusterName + to.SetLabels(labels) + + // Set the owner reference. + if in.OwnerRef != nil { + to.SetOwnerReferences([]metav1.OwnerReference{*in.OwnerRef}) } // Set the object APIVersion. if to.GetAPIVersion() == "" { - to.SetAPIVersion(ref.APIVersion) + to.SetAPIVersion(in.TemplateRef.APIVersion) } // Set the object Kind and strip the word "Template" if it's a suffix. if to.GetKind() == "" { - to.SetKind(strings.TrimSuffix(ref.Kind, TemplateSuffix)) + to.SetKind(strings.TrimSuffix(in.TemplateRef.Kind, TemplateSuffix)) } // Create the external clone. - if err := c.Create(context.Background(), to); err != nil { + if err := in.Client.Create(context.Background(), to); err != nil { return nil, err } - toRef := &corev1.ObjectReference{ + return &corev1.ObjectReference{ APIVersion: to.GetAPIVersion(), Kind: to.GetKind(), Name: to.GetName(), Namespace: to.GetNamespace(), UID: to.GetUID(), - } - - return toRef, nil + }, nil } // FailuresFrom returns the FailureReason and FailureMessage fields from the external object status. diff --git a/controllers/external/util_test.go b/controllers/external/util_test.go index 46514545c1c7..e1e28b40c739 100644 --- a/controllers/external/util_test.go +++ b/controllers/external/util_test.go @@ -92,14 +92,12 @@ func TestCloneTemplateResourceNotFound(t *testing.T) { } fakeClient := fake.NewFakeClientWithScheme(runtime.NewScheme()) - _, err := CloneTemplate( - context.Background(), - fakeClient, - testResourceReference, - namespace, - testClusterName, - nil, - ) + _, err := CloneTemplate(context.Background(), &CloneTemplateInput{ + Client: fakeClient, + TemplateRef: testResourceReference, + Namespace: namespace, + ClusterName: testClusterName, + }) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(apierrors.IsNotFound(errors.Cause(err))).To(gomega.BeTrue()) } @@ -147,7 +145,6 @@ func TestCloneTemplateResourceFound(t *testing.T) { expectedKind := "Purple" expectedAPIVersion := templateAPIVersion - expectedLabels := (map[string]string{clusterv1.ClusterLabelName: testClusterName}) expectedSpec, ok, err := unstructured.NestedMap(template.UnstructuredContent(), "spec", "template", "spec") g.Expect(err).NotTo(gomega.HaveOccurred()) @@ -156,14 +153,16 @@ func TestCloneTemplateResourceFound(t *testing.T) { fakeClient := fake.NewFakeClientWithScheme(runtime.NewScheme(), template.DeepCopy()) - ref, err := CloneTemplate( - context.Background(), - fakeClient, - templateRef.DeepCopy(), - namespace, - testClusterName, - owner.DeepCopy(), - ) + ref, err := CloneTemplate(context.Background(), &CloneTemplateInput{ + Client: fakeClient, + TemplateRef: templateRef.DeepCopy(), + Namespace: namespace, + ClusterName: testClusterName, + OwnerRef: owner.DeepCopy(), + Labels: map[string]string{ + "test-label-1": "value-1", + }, + }) g.Expect(err).NotTo(gomega.HaveOccurred()) g.Expect(ref).NotTo(gomega.BeNil()) g.Expect(ref.Kind).To(gomega.Equal(expectedKind)) @@ -174,15 +173,20 @@ func TestCloneTemplateResourceFound(t *testing.T) { clone := &unstructured.Unstructured{} clone.SetKind(expectedKind) clone.SetAPIVersion(expectedAPIVersion) + key := client.ObjectKey{Name: ref.Name, Namespace: ref.Namespace} g.Expect(fakeClient.Get(context.Background(), key, clone)).To(gomega.Succeed()) - g.Expect(clone.GetLabels()).To(gomega.Equal(expectedLabels)) g.Expect(clone.GetOwnerReferences()).To(gomega.HaveLen(1)) g.Expect(clone.GetOwnerReferences()).To(gomega.ContainElement(owner)) + cloneSpec, ok, err := unstructured.NestedMap(clone.UnstructuredContent(), "spec") g.Expect(err).NotTo(gomega.HaveOccurred()) g.Expect(ok).To(gomega.BeTrue()) g.Expect(cloneSpec).To(gomega.Equal(expectedSpec)) + + cloneLabels := clone.GetLabels() + g.Expect(cloneLabels).To(gomega.HaveKeyWithValue(clusterv1.ClusterLabelName, testClusterName)) + g.Expect(cloneLabels).To(gomega.HaveKeyWithValue("test-label-1", "value-1")) } func TestCloneTemplateResourceFoundNoOwner(t *testing.T) { @@ -231,14 +235,12 @@ func TestCloneTemplateResourceFoundNoOwner(t *testing.T) { fakeClient := fake.NewFakeClientWithScheme(runtime.NewScheme(), template.DeepCopy()) - ref, err := CloneTemplate( - context.Background(), - fakeClient, - templateRef, - namespace, - testClusterName, - nil, - ) + ref, err := CloneTemplate(context.Background(), &CloneTemplateInput{ + Client: fakeClient, + TemplateRef: templateRef, + Namespace: namespace, + ClusterName: testClusterName, + }) g.Expect(err).NotTo(gomega.HaveOccurred()) g.Expect(ref).NotTo(gomega.BeNil()) g.Expect(ref.Kind).To(gomega.Equal(expectedKind)) @@ -290,13 +292,11 @@ func TestCloneTemplateMissingSpecTemplate(t *testing.T) { fakeClient := fake.NewFakeClientWithScheme(runtime.NewScheme(), template.DeepCopy()) - _, err := CloneTemplate( - context.Background(), - fakeClient, - templateRef, - namespace, - testClusterName, - nil, - ) + _, err := CloneTemplate(context.Background(), &CloneTemplateInput{ + Client: fakeClient, + TemplateRef: templateRef, + Namespace: namespace, + ClusterName: testClusterName, + }) g.Expect(err).To(gomega.HaveOccurred()) } diff --git a/controllers/machineset_controller.go b/controllers/machineset_controller.go index 48fa793c7249..0640a17cf025 100644 --- a/controllers/machineset_controller.go +++ b/controllers/machineset_controller.go @@ -324,14 +324,26 @@ func (r *MachineSetReconciler) syncReplicas(ctx context.Context, ms *clusterv1.M ) if machine.Spec.Bootstrap.ConfigRef != nil { - bootstrapRef, err = external.CloneTemplate(ctx, r.Client, machine.Spec.Bootstrap.ConfigRef, machine.Namespace, machine.Spec.ClusterName, nil) + bootstrapRef, err = external.CloneTemplate(ctx, &external.CloneTemplateInput{ + Client: r.Client, + TemplateRef: machine.Spec.Bootstrap.ConfigRef, + Namespace: machine.Namespace, + ClusterName: machine.Spec.ClusterName, + Labels: machine.Labels, + }) if err != nil { return errors.Wrapf(err, "failed to clone bootstrap configuration for MachineSet %q in namespace %q", ms.Name, ms.Namespace) } machine.Spec.Bootstrap.ConfigRef = bootstrapRef } - infraRef, err = external.CloneTemplate(ctx, r.Client, &machine.Spec.InfrastructureRef, machine.Namespace, machine.Spec.ClusterName, nil) + infraRef, err = external.CloneTemplate(ctx, &external.CloneTemplateInput{ + Client: r.Client, + TemplateRef: &machine.Spec.InfrastructureRef, + Namespace: machine.Namespace, + ClusterName: machine.Spec.ClusterName, + Labels: machine.Labels, + }) if err != nil { return errors.Wrapf(err, "failed to clone infrastructure configuration for MachineSet %q in namespace %q", ms.Name, ms.Namespace) } diff --git a/controlplane/kubeadm/controllers/kubeadm_control_plane_controller.go b/controlplane/kubeadm/controllers/kubeadm_control_plane_controller.go index 644f2368420f..2e7c4f0548bd 100644 --- a/controlplane/kubeadm/controllers/kubeadm_control_plane_controller.go +++ b/controlplane/kubeadm/controllers/kubeadm_control_plane_controller.go @@ -339,17 +339,14 @@ func (r *KubeadmControlPlaneReconciler) initializeControlPlane(ctx context.Conte func (r *KubeadmControlPlaneReconciler) cloneConfigsAndGenerateMachine(ctx context.Context, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, bootstrapSpec *bootstrapv1.KubeadmConfigSpec) error { var errs []error - ownerRef := metav1.NewControllerRef(kcp, controlplanev1.GroupVersion.WithKind("KubeadmControlPlane")) - // Clone the infrastructure template - infraRef, err := external.CloneTemplate( - ctx, - r.Client, - &kcp.Spec.InfrastructureTemplate, - kcp.Namespace, - cluster.Name, - ownerRef, - ) + infraRef, err := external.CloneTemplate(ctx, &external.CloneTemplateInput{ + Client: r.Client, + TemplateRef: &kcp.Spec.InfrastructureTemplate, + Namespace: kcp.Namespace, + OwnerRef: metav1.NewControllerRef(kcp, controlplanev1.GroupVersion.WithKind("KubeadmControlPlane")), + ClusterName: cluster.Name, + }) if err != nil { // Safe to return early here since no resources have been created yet. return errors.Wrap(err, "failed to clone infrastructure template")