From 9b99e55db864dedbcd19e0a90041ffbbda16c65e Mon Sep 17 00:00:00 2001 From: Tim Ebert Date: Mon, 1 Mar 2021 16:56:38 +0100 Subject: [PATCH] :sparkles: Add client.StrategicMergeFrom --- pkg/client/client_test.go | 80 ++++++++++++++++++++++++++++++++++++++- pkg/client/patch.go | 47 ++++++++++++++++++++--- 2 files changed, 120 insertions(+), 7 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 2dc152b164..0dc8d1a2a2 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -3240,7 +3240,7 @@ var _ = Describe("DelegatingClient", func() { }) var _ = Describe("Patch", func() { - Describe("CreateMergePatch", func() { + Describe("MergeFrom", func() { var cm *corev1.ConfigMap BeforeEach(func() { @@ -3303,6 +3303,84 @@ var _ = Describe("Patch", func() { Expect(data).To(Equal([]byte(fmt.Sprintf(`{"metadata":{"annotations":{"%s":"%s"},"resourceVersion":"%s"}}`, annotationKey, annotationValue, cm.ResourceVersion)))) }) }) + + Describe("StrategicMergeFrom", func() { + var dep *appsv1.Deployment + + BeforeEach(func() { + dep = &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: "dep", + ResourceVersion: "10", + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{Containers: []corev1.Container{{ + Name: "main", + Image: "foo:v1", + }, { + Name: "sidecar", + Image: "bar:v1", + }}}, + }, + }, + } + }) + + It("creates a strategic merge patch with the modifications applied during the mutation", func() { + By("creating a strategic merge patch") + patch := client.StrategicMergeFrom(dep.DeepCopy()) + + By("returning a patch with type StrategicMergePatchType") + Expect(patch.Type()).To(Equal(types.StrategicMergePatchType)) + + By("updating the main container's image") + for i, c := range dep.Spec.Template.Spec.Containers { + if c.Name == "main" { + c.Image = "foo:v2" + } + dep.Spec.Template.Spec.Containers[i] = c + } + + By("computing the patch data") + data, err := patch.Data(dep) + + By("returning no error") + Expect(err).NotTo(HaveOccurred()) + + By("returning a patch with data only containing the image change") + Expect(data).To(Equal([]byte(`{"spec":{"template":{"spec":{"$setElementOrder/containers":[{"name":"main"},` + + `{"name":"sidecar"}],"containers":[{"image":"foo:v2","name":"main"}]}}}}`))) + }) + + It("creates a strategic merge patch with the modifications applied during the mutation, using optimistic locking", func() { + By("creating a strategic merge patch") + patch := client.StrategicMergeFrom(dep.DeepCopy(), client.MergeFromWithOptimisticLock{}) + + By("returning a patch with type StrategicMergePatchType") + Expect(patch.Type()).To(Equal(types.StrategicMergePatchType)) + + By("updating the main container's image") + for i, c := range dep.Spec.Template.Spec.Containers { + if c.Name == "main" { + c.Image = "foo:v2" + } + dep.Spec.Template.Spec.Containers[i] = c + } + + By("computing the patch data") + data, err := patch.Data(dep) + + By("returning no error") + Expect(err).NotTo(HaveOccurred()) + + By("returning a patch with data containing the image change and the resourceVersion change") + Expect(data).To(Equal([]byte(fmt.Sprintf(`{"metadata":{"resourceVersion":"%s"},`+ + `"spec":{"template":{"spec":{"$setElementOrder/containers":[{"name":"main"},{"name":"sidecar"}],"containers":[{"image":"foo:v2","name":"main"}]}}}}`, + dep.ResourceVersion)))) + }) + }) }) var _ = Describe("IgnoreNotFound", func() { diff --git a/pkg/client/patch.go b/pkg/client/patch.go index 260db896fb..d071e0f159 100644 --- a/pkg/client/patch.go +++ b/pkg/client/patch.go @@ -23,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/json" + "k8s.io/apimachinery/pkg/util/strategicpatch" ) var ( @@ -85,13 +86,15 @@ type MergeFromOptions struct { } type mergeFromPatch struct { - from Object - opts MergeFromOptions + patchType types.PatchType + createPatch func(originalJSON, modifiedJSON []byte, dataStruct interface{}) ([]byte, error) + from Object + opts MergeFromOptions } // Type implements patch. func (s *mergeFromPatch) Type() types.PatchType { - return types.MergePatchType + return s.patchType } // Data implements Patch. @@ -122,7 +125,7 @@ func (s *mergeFromPatch) Data(obj Object) ([]byte, error) { return nil, err } - data, err := jsonpatch.CreateMergePatch(originalJSON, modifiedJSON) + data, err := s.createPatch(originalJSON, modifiedJSON, obj) if err != nil { return nil, err } @@ -130,18 +133,50 @@ func (s *mergeFromPatch) Data(obj Object) ([]byte, error) { return data, nil } +func createMergePatch(originalJSON, modifiedJSON []byte, _ interface{}) ([]byte, error) { + return jsonpatch.CreateMergePatch(originalJSON, modifiedJSON) +} + +func createStrategicMergePatch(originalJSON, modifiedJSON []byte, dataStruct interface{}) ([]byte, error) { + return strategicpatch.CreateTwoWayMergePatch(originalJSON, modifiedJSON, dataStruct) +} + // MergeFrom creates a Patch that patches using the merge-patch strategy with the given object as base. +// The difference between MergeFrom and StrategicMergeFrom lays in the handling of modified list fields. +// When using MergeFrom, existing lists will be completely replaced by new lists. +// When using StrategicMergeFrom, the list field's `patchStrategy` is respected if specified in the API type, +// e.g. the existing list is not replaced completely but rather merged with the new one using the list's `patchMergeKey`. +// See https://kubernetes.io/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/ for more details on +// the difference between merge-patch and strategic-merge-patch. func MergeFrom(obj Object) Patch { - return &mergeFromPatch{from: obj} + return &mergeFromPatch{patchType: types.MergePatchType, createPatch: createMergePatch, from: obj} } // MergeFromWithOptions creates a Patch that patches using the merge-patch strategy with the given object as base. +// See MergeFrom for more details. func MergeFromWithOptions(obj Object, opts ...MergeFromOption) Patch { options := &MergeFromOptions{} for _, opt := range opts { opt.ApplyToMergeFrom(options) } - return &mergeFromPatch{from: obj, opts: *options} + return &mergeFromPatch{patchType: types.MergePatchType, createPatch: createMergePatch, from: obj, opts: *options} +} + +// StrategicMergeFrom creates a Patch that patches using the strategic-merge-patch strategy with the given object as base. +// The difference between MergeFrom and StrategicMergeFrom lays in the handling of modified list fields. +// When using MergeFrom, existing lists will be completely replaced by new lists. +// When using StrategicMergeFrom, the list field's `patchStrategy` is respected if specified in the API type, +// e.g. the existing list is not replaced completely but rather merged with the new one using the list's `patchMergeKey`. +// See https://kubernetes.io/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/ for more details on +// the difference between merge-patch and strategic-merge-patch. +// Please note, that CRDs don't support strategic-merge-patch, see +// https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/#advanced-features-and-flexibility +func StrategicMergeFrom(obj Object, opts ...MergeFromOption) Patch { + options := &MergeFromOptions{} + for _, opt := range opts { + opt.ApplyToMergeFrom(options) + } + return &mergeFromPatch{patchType: types.StrategicMergePatchType, createPatch: createStrategicMergePatch, from: obj, opts: *options} } // mergePatch uses a raw merge strategy to patch the object.