Skip to content

[0.5] ✨ Add Patch MergeFrom optimistic locking option #973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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