Merge pull request #850 from akutz/feature/createOrPatch

✨CreateOrPatch

pkg/controller/controllerutil/controllerutil.go

@@ -19,10 +19,12 @@ package controllerutil
 import (
 	"context"
 	"fmt"
+	"reflect"
 
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/utils/pointer"
@@ -179,6 +181,10 @@ const ( // They should complete the sentence "Deployment default/foo has been ..
 	OperationResultCreated OperationResult = "created"
 	// OperationResultUpdated means that an existing resource is updated
 	OperationResultUpdated OperationResult = "updated"
+	// OperationResultUpdatedStatus means that an existing resource and its status is updated
+	OperationResultUpdatedStatus OperationResult = "updatedStatus"
+	// OperationResultUpdatedStatusOnly means that only an existing status is updated
+	OperationResultUpdatedStatusOnly OperationResult = "updatedStatusOnly"
 )
 
 // CreateOrUpdate creates or updates the given object in the Kubernetes
@@ -222,6 +228,108 @@ func CreateOrUpdate(ctx context.Context, c client.Client, obj runtime.Object, f
 	return OperationResultUpdated, nil
 }
 
+// CreateOrPatch creates or patches the given object in the Kubernetes
+// cluster. The object's desired state must be reconciled with the before
+// state inside the passed in callback MutateFn.
+//
+// The MutateFn is called regardless of creating or updating an object.
+//
+// It returns the executed operation and an error.
+func CreateOrPatch(ctx context.Context, c client.Client, obj runtime.Object, f MutateFn) (OperationResult, error) {
+	key, err := client.ObjectKeyFromObject(obj)
+	if err != nil {
+		return OperationResultNone, err
+	}
+
+	if err := c.Get(ctx, key, obj); err != nil {
+		if !errors.IsNotFound(err) {
+			return OperationResultNone, err
+		}
+		if f != nil {
+			if err := mutate(f, key, obj); err != nil {
+				return OperationResultNone, err
+			}
+		}
+		if err := c.Create(ctx, obj); err != nil {
+			return OperationResultNone, err
+		}
+		return OperationResultCreated, nil
+	}
+
+	// Create patches for the object and its possible status.
+	objPatch := client.MergeFrom(obj.DeepCopyObject())
+	statusPatch := client.MergeFrom(obj.DeepCopyObject())
+
+	// Create a copy of the original object as well as converting that copy to
+	// unstructured data.
+	before, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj.DeepCopyObject())
+	if err != nil {
+		return OperationResultNone, err
+	}
+
+	// Attempt to extract the status from the resource for easier comparison later
+	beforeStatus, hasBeforeStatus, err := unstructured.NestedFieldCopy(before, "status")
+	if err != nil {
+		return OperationResultNone, err
+	}
+
+	// If the resource contains a status then remove it from the unstructured
+	// copy to avoid unnecessary patching later.
+	if hasBeforeStatus {
+		unstructured.RemoveNestedField(before, "status")
+	}
+
+	// Mutate the original object.
+	if f != nil {
+		if err := mutate(f, key, obj); err != nil {
+			return OperationResultNone, err
+		}
+	}
+
+	// Convert the resource to unstructured to compare against our before copy.
+	after, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
+	if err != nil {
+		return OperationResultNone, err
+	}
+
+	// Attempt to extract the status from the resource for easier comparison later
+	afterStatus, hasAfterStatus, err := unstructured.NestedFieldCopy(after, "status")
+	if err != nil {
+		return OperationResultNone, err
+	}
+
+	// If the resource contains a status then remove it from the unstructured
+	// copy to avoid unnecessary patching later.
+	if hasAfterStatus {
+		unstructured.RemoveNestedField(after, "status")
+	}
+
+	result := OperationResultNone
+
+	if !reflect.DeepEqual(before, after) {
+		// Only issue a Patch if the before and after resources (minus status) differ
+		if err := c.Patch(ctx, obj, objPatch); err != nil {
+			return result, err
+		}
+		result = OperationResultUpdated
+	}
+
+	if (hasBeforeStatus || hasAfterStatus) && !reflect.DeepEqual(beforeStatus, afterStatus) {
+		// Only issue a Status Patch if the resource has a status and the beforeStatus
+		// and afterStatus copies differ
+		if err := c.Status().Patch(ctx, obj, statusPatch); err != nil {
+			return result, err
+		}
+		if result == OperationResultUpdated {
+			result = OperationResultUpdatedStatus
+		} else {
+			result = OperationResultUpdatedStatusOnly
+		}
+	}
+
+	return result, nil
+}
+
 // mutate wraps a MutateFn and applies validation to its result
 func mutate(f MutateFn, key client.ObjectKey, obj runtime.Object) error {
 	if err := f(); err != nil {

pkg/controller/controllerutil/controllerutil_test.go

@@ -406,6 +406,213 @@ var _ = Describe("Controllerutil", func() {
 		})
 	})
 
+	Describe("CreateOrPatch", func() {
+		var deploy *appsv1.Deployment
+		var deplSpec appsv1.DeploymentSpec
+		var deplKey types.NamespacedName
+		var specr controllerutil.MutateFn
+
+		BeforeEach(func() {
+			deploy = &appsv1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      fmt.Sprintf("deploy-%d", rand.Int31()),
+					Namespace: "default",
+				},
+			}
+
+			deplSpec = appsv1.DeploymentSpec{
+				Selector: &metav1.LabelSelector{
+					MatchLabels: map[string]string{"foo": "bar"},
+				},
+				Template: corev1.PodTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Labels: map[string]string{
+							"foo": "bar",
+						},
+					},
+					Spec: corev1.PodSpec{
+						Containers: []corev1.Container{
+							{
+								Name:  "busybox",
+								Image: "busybox",
+							},
+						},
+					},
+				},
+			}
+
+			deplKey = types.NamespacedName{
+				Name:      deploy.Name,
+				Namespace: deploy.Namespace,
+			}
+
+			specr = deploymentSpecr(deploy, deplSpec)
+		})
+
+		assertLocalDeployWasUpdated := func(fetched *appsv1.Deployment) {
+			By("local deploy object was updated during patch & has same spec, status, resource version as fetched")
+			if fetched == nil {
+				fetched = &appsv1.Deployment{}
+				ExpectWithOffset(1, c.Get(context.TODO(), deplKey, fetched)).To(Succeed())
+			}
+			ExpectWithOffset(1, fetched.ResourceVersion).To(Equal(deploy.ResourceVersion))
+			ExpectWithOffset(1, fetched.Spec).To(BeEquivalentTo(deploy.Spec))
+			ExpectWithOffset(1, fetched.Status).To(BeEquivalentTo(deploy.Status))
+		}
+
+		It("creates a new object if one doesn't exists", func() {
+			op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
+
+			By("returning no error")
+			Expect(err).NotTo(HaveOccurred())
+
+			By("returning OperationResultCreated")
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
+
+			By("actually having the deployment created")
+			fetched := &appsv1.Deployment{}
+			Expect(c.Get(context.TODO(), deplKey, fetched)).To(Succeed())
+
+			By("being mutated by MutateFn")
+			Expect(fetched.Spec.Template.Spec.Containers).To(HaveLen(1))
+			Expect(fetched.Spec.Template.Spec.Containers[0].Name).To(Equal(deplSpec.Template.Spec.Containers[0].Name))
+			Expect(fetched.Spec.Template.Spec.Containers[0].Image).To(Equal(deplSpec.Template.Spec.Containers[0].Image))
+		})
+
+		It("patches existing object", func() {
+			var scale int32 = 2
+			op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
+
+			op, err = controllerutil.CreateOrPatch(context.TODO(), c, deploy, deploymentScaler(deploy, scale))
+			By("returning no error")
+			Expect(err).NotTo(HaveOccurred())
+
+			By("returning OperationResultUpdated")
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultUpdated))
+
+			By("actually having the deployment scaled")
+			fetched := &appsv1.Deployment{}
+			Expect(c.Get(context.TODO(), deplKey, fetched)).To(Succeed())
+			Expect(*fetched.Spec.Replicas).To(Equal(scale))
+			assertLocalDeployWasUpdated(fetched)
+		})
+
+		It("patches only changed objects", func() {
+			op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
+
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
+			Expect(err).NotTo(HaveOccurred())
+
+			op, err = controllerutil.CreateOrPatch(context.TODO(), c, deploy, deploymentIdentity)
+			By("returning no error")
+			Expect(err).NotTo(HaveOccurred())
+
+			By("returning OperationResultNone")
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultNone))
+
+			assertLocalDeployWasUpdated(nil)
+		})
+
+		It("patches only changed status", func() {
+			op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
+
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
+			Expect(err).NotTo(HaveOccurred())
+
+			deployStatus := appsv1.DeploymentStatus{
+				ReadyReplicas: 1,
+				Replicas:      3,
+			}
+			op, err = controllerutil.CreateOrPatch(context.TODO(), c, deploy, deploymentStatusr(deploy, deployStatus))
+			By("returning no error")
+			Expect(err).NotTo(HaveOccurred())
+
+			By("returning OperationResultUpdatedStatusOnly")
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultUpdatedStatusOnly))
+
+			assertLocalDeployWasUpdated(nil)
+		})
+
+		It("patches resource and status", func() {
+			op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
+
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
+			Expect(err).NotTo(HaveOccurred())
+
+			replicas := int32(3)
+			deployStatus := appsv1.DeploymentStatus{
+				ReadyReplicas: 1,
+				Replicas:      replicas,
+			}
+			op, err = controllerutil.CreateOrPatch(context.TODO(), c, deploy, func() error {
+				Expect(deploymentScaler(deploy, replicas)()).To(Succeed())
+				return deploymentStatusr(deploy, deployStatus)()
+			})
+			By("returning no error")
+			Expect(err).NotTo(HaveOccurred())
+
+			By("returning OperationResultUpdatedStatus")
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultUpdatedStatus))
+
+			assertLocalDeployWasUpdated(nil)
+		})
+
+		It("errors when MutateFn changes object name on creation", func() {
+			op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, func() error {
+				Expect(specr()).To(Succeed())
+				return deploymentRenamer(deploy)()
+			})
+
+			By("returning error")
+			Expect(err).To(HaveOccurred())
+
+			By("returning OperationResultNone")
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultNone))
+		})
+
+		It("errors when MutateFn renames an object", func() {
+			op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
+
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
+			Expect(err).NotTo(HaveOccurred())
+
+			op, err = controllerutil.CreateOrPatch(context.TODO(), c, deploy, deploymentRenamer(deploy))
+
+			By("returning error")
+			Expect(err).To(HaveOccurred())
+
+			By("returning OperationResultNone")
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultNone))
+		})
+
+		It("errors when object namespace changes", func() {
+			op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
+
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
+			Expect(err).NotTo(HaveOccurred())
+
+			op, err = controllerutil.CreateOrPatch(context.TODO(), c, deploy, deploymentNamespaceChanger(deploy))
+
+			By("returning error")
+			Expect(err).To(HaveOccurred())
+
+			By("returning OperationResultNone")
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultNone))
+		})
+
+		It("aborts immediately if there was an error initially retrieving the object", func() {
+			op, err := controllerutil.CreateOrPatch(context.TODO(), errorReader{c}, deploy, func() error {
+				Fail("Mutation method should not run")
+				return nil
+			})
+
+			Expect(op).To(BeEquivalentTo(controllerutil.OperationResultNone))
+			Expect(err).To(HaveOccurred())
+		})
+	})
+
 	Describe("Finalizers", func() {
 		var deploy *appsv1.Deployment
 
@@ -478,6 +685,13 @@ func deploymentSpecr(deploy *appsv1.Deployment, spec appsv1.DeploymentSpec) cont
 	}
 }
 
+func deploymentStatusr(deploy *appsv1.Deployment, status appsv1.DeploymentStatus) controllerutil.MutateFn {
+	return func() error {
+		deploy.Status = status
+		return nil
+	}
+}
+
 var deploymentIdentity controllerutil.MutateFn = func() error {
 	return nil
 }