Skip to content

Commit

Permalink
Merge pull request #2091 from vincepri/cloner-ref
Browse files Browse the repository at this point in the history 
🏃Refactor CloneTemplate / propagate clone labels in MS
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Jan 17, 2020
2 parents 09baf60 + 520299e commit 1bab288
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 59 deletions.
61 changes: 48 additions & 13 deletions controllers/external/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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.
Expand Down
68 changes: 34 additions & 34 deletions controllers/external/util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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())
Expand All @@ -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))
Expand All @@ -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) {
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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())
}
16 changes: 14 additions & 2 deletions controllers/machineset_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
17 changes: 7 additions & 10 deletions controlplane/kubeadm/controllers/kubeadm_control_plane_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit 1bab288

Please sign in to comment.