Merge pull request #973 from vincepri/backport969

[0.5] ✨ Add Patch MergeFrom optimistic locking option

Author: k8s-ci-robot

pkg/client/client_test.go

@@ -1209,6 +1209,42 @@ var _ = Describe("Client", func() {
 				close(done)
 			})
 
+			It("should patch an existing object from a go struct, using optimistic locking", func(done Done) {
+				cl, err := client.New(cfg, client.Options{})
+				Expect(err).NotTo(HaveOccurred())
+				Expect(cl).NotTo(BeNil())
+
+				By("initially creating a Deployment")
+				dep, err := clientset.AppsV1().Deployments(ns).Create(dep)
+				Expect(err).NotTo(HaveOccurred())
+
+				By("creating a patch from with optimistic lock")
+				patch := client.MergeFromWithOptions(dep.DeepCopy(), client.MergeFromWithOptimisticLock{})
+
+				By("adding a new annotation")
+				dep.Annotations = map[string]string{
+					"foo": "bar",
+				}
+
+				By("patching the Deployment")
+				err = cl.Patch(context.TODO(), dep, patch)
+				Expect(err).NotTo(HaveOccurred())
+
+				By("validating patched Deployment has new annotation")
+				actual, err := clientset.AppsV1().Deployments(ns).Get(dep.Name, metav1.GetOptions{})
+				Expect(err).NotTo(HaveOccurred())
+				Expect(actual).NotTo(BeNil())
+				Expect(actual.Annotations["foo"]).To(Equal("bar"))
+
+				By("validating that a patch should fail with conflict, when it has an outdated resource version")
+				dep.Annotations["should"] = "conflict"
+				err = cl.Patch(context.TODO(), dep, patch)
+				Expect(err).To(HaveOccurred())
+				Expect(apierrors.IsConflict(err)).To(BeTrue())
+
+				close(done)
+			})
+
 			It("should patch and preserve type information", func(done Done) {
 				cl, err := client.New(cfg, client.Options{})
 				Expect(err).NotTo(HaveOccurred())
@@ -2655,8 +2691,9 @@ var _ = Describe("Patch", func() {
 		BeforeEach(func() {
 			cm = &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
-					Namespace: metav1.NamespaceDefault,
-					Name:      "cm",
+					Namespace:       metav1.NamespaceDefault,
+					Name:            "cm",
+					ResourceVersion: "10",
 				},
 			}
 		})
@@ -2685,6 +2722,31 @@ var _ = Describe("Patch", func() {
 			By("returning a patch with data only containing the annotation change")
 			Expect(data).To(Equal([]byte(fmt.Sprintf(`{"metadata":{"annotations":{"%s":"%s"}}}`, annotationKey, annotationValue))))
 		})
+
+		It("creates a merge patch with the modifications applied during the mutation, using optimistic locking", func() {
+			const (
+				annotationKey   = "test"
+				annotationValue = "foo"
+			)
+
+			By("creating a merge patch")
+			patch := client.MergeFromWithOptions(cm.DeepCopy(), client.MergeFromWithOptimisticLock{})
+
+			By("returning a patch with type MergePatch")
+			Expect(patch.Type()).To(Equal(types.MergePatchType))
+
+			By("retrieving modifying the config map")
+			metav1.SetMetaDataAnnotation(&cm.ObjectMeta, annotationKey, annotationValue)
+
+			By("computing the patch data")
+			data, err := patch.Data(cm)
+
+			By("returning no error")
+			Expect(err).NotTo(HaveOccurred())
+
+			By("returning a patch with data containing the annotation change and the resourceVersion change")
+			Expect(data).To(Equal([]byte(fmt.Sprintf(`{"metadata":{"annotations":{"%s":"%s"},"resourceVersion":"%s"}}`, annotationKey, annotationValue, cm.ResourceVersion))))
+		})
 	})
 })
 

pkg/client/patch.go

@@ -17,7 +17,11 @@ limitations under the License.
 package client
 
 import (
+	"fmt"
+
 	jsonpatch "github.com/evanphx/json-patch"
+	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/types"
 	"k8s.io/apimachinery/pkg/util/json"
@@ -59,8 +63,39 @@ func ConstantPatch(patchType types.PatchType, data []byte) Patch {
 	return RawPatch(patchType, data)
 }
 
+// MergeFromWithOptimisticLock can be used if clients want to make sure a patch
+// is being applied to the latest resource version of an object.
+//
+// The behavior is similar to what an Update would do, without the need to send the
+// whole object. Usually this method is useful if you might have multiple clients
+// acting on the same object and the same API version, but with different versions of the Go structs.
+//
+// For example, an "older" copy of a Widget that has fields A and B, and a "newer" copy with A, B, and C.
+// Sending an update using the older struct definition results in C being dropped, whereas using a patch does not.
+type MergeFromWithOptimisticLock struct{}
+
+// ApplyToMergeFrom applies this configuration to the given patch options.
+func (m MergeFromWithOptimisticLock) ApplyToMergeFrom(in *MergeFromOptions) {
+	in.OptimisticLock = true
+}
+
+// MergeFromOption is some configuration that modifies options for a merge-from patch data.
+type MergeFromOption interface {
+	// ApplyToMergeFrom applies this configuration to the given patch options.
+	ApplyToMergeFrom(*MergeFromOptions)
+}
+
+// MergeFromOptions contains options to generate a merge-from patch data.
+type MergeFromOptions struct {
+	// OptimisticLock, when true, includes `metadata.resourceVersion` into the final
+	// patch data. If the `resourceVersion` field doesn't match what's stored,
+	// the operation results in a conflict and clients will need to try again.
+	OptimisticLock bool
+}
+
 type mergeFromPatch struct {
 	from runtime.Object
+	opts MergeFromOptions
 }
 
 // Type implements patch.
@@ -80,12 +115,47 @@ func (s *mergeFromPatch) Data(obj runtime.Object) ([]byte, error) {
 		return nil, err
 	}
 
-	return jsonpatch.CreateMergePatch(originalJSON, modifiedJSON)
+	data, err := jsonpatch.CreateMergePatch(originalJSON, modifiedJSON)
+	if err != nil {
+		return nil, err
+	}
+
+	if s.opts.OptimisticLock {
+		dataMap := map[string]interface{}{}
+		if err := json.Unmarshal(data, &dataMap); err != nil {
+			return nil, err
+		}
+		fromMeta, ok := s.from.(metav1.Object)
+		if !ok {
+			return nil, fmt.Errorf("cannot use OptimisticLock, from object %q is not a valid metav1.Object", s.from)
+		}
+		resourceVersion := fromMeta.GetResourceVersion()
+		if len(resourceVersion) == 0 {
+			return nil, fmt.Errorf("cannot use OptimisticLock, from object %q does not have any resource version we can use", s.from)
+		}
+		u := &unstructured.Unstructured{Object: dataMap}
+		u.SetResourceVersion(resourceVersion)
+		data, err = json.Marshal(u)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	return data, nil
 }
 
 // MergeFrom creates a Patch that patches using the merge-patch strategy with the given object as base.
 func MergeFrom(obj runtime.Object) Patch {
-	return &mergeFromPatch{obj}
+	return &mergeFromPatch{from: obj}
+}
+
+// MergeFromWithOptions creates a Patch that patches using the merge-patch strategy with the given object as base.
+func MergeFromWithOptions(obj runtime.Object, opts ...MergeFromOption) Patch {
+	options := &MergeFromOptions{}
+	for _, opt := range opts {
+		opt.ApplyToMergeFrom(options)
+	}
+	return &mergeFromPatch{from: obj, opts: *options}
 }
 
 // mergePatch uses a raw merge strategy to patch the object.