Skip to content

Commit

Permalink
Merge pull request #973 from vincepri/backport969
Browse files Browse the repository at this point in the history
[0.5] ✨ Add Patch MergeFrom optimistic locking option
  • Loading branch information
k8s-ci-robot authored May 29, 2020
2 parents c45adcf + c1e9515 commit 5291db5
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 4 deletions.
66 changes: 64 additions & 2 deletions pkg/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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",
},
}
})
Expand Down Expand Up @@ -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))))
})
})
})

Expand Down
74 changes: 72 additions & 2 deletions pkg/client/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down

0 comments on commit 5291db5

Please sign in to comment.