Skip to content

✨ Add Merge patch type and deprecate ConstantPatch in favor of RawPatch #721

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
Jan 6, 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
22 changes: 11 additions & 11 deletions pkg/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1146,7 +1146,7 @@ var _ = Describe("Client", func() {
Expect(err).NotTo(HaveOccurred())

By("patching the Deployment")
err = cl.Patch(context.TODO(), dep, client.ConstantPatch(types.MergePatchType, mergePatch))
err = cl.Patch(context.TODO(), dep, client.RawPatch(types.MergePatchType, mergePatch))
Expect(err).NotTo(HaveOccurred())

By("validating patched Deployment has new annotation")
Expand All @@ -1169,7 +1169,7 @@ var _ = Describe("Client", func() {

By("patching the Deployment")
dep.SetGroupVersionKind(depGvk)
err = cl.Patch(context.TODO(), dep, client.ConstantPatch(types.MergePatchType, mergePatch))
err = cl.Patch(context.TODO(), dep, client.RawPatch(types.MergePatchType, mergePatch))
Expect(err).NotTo(HaveOccurred())

By("validating updated Deployment has type information")
Expand All @@ -1189,7 +1189,7 @@ var _ = Describe("Client", func() {

By("patching the Node")
nodeName := node.Name
err = cl.Patch(context.TODO(), node, client.ConstantPatch(types.MergePatchType, mergePatch))
err = cl.Patch(context.TODO(), node, client.RawPatch(types.MergePatchType, mergePatch))
Expect(err).NotTo(HaveOccurred())

By("validating the Node no longer exists")
Expand All @@ -1207,7 +1207,7 @@ var _ = Describe("Client", func() {
Expect(cl).NotTo(BeNil())

By("Patching node before it is ever created")
err = cl.Patch(context.TODO(), node, client.ConstantPatch(types.MergePatchType, mergePatch))
err = cl.Patch(context.TODO(), node, client.RawPatch(types.MergePatchType, mergePatch))
Expect(err).To(HaveOccurred())

close(done)
Expand All @@ -1229,7 +1229,7 @@ var _ = Describe("Client", func() {
Expect(err).NotTo(HaveOccurred())

By("patching the Deployment fails")
err = cl.Patch(context.TODO(), dep, client.ConstantPatch(types.MergePatchType, mergePatch))
err = cl.Patch(context.TODO(), dep, client.RawPatch(types.MergePatchType, mergePatch))
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("no kind is registered for the type"))

Expand All @@ -1251,7 +1251,7 @@ var _ = Describe("Client", func() {
Expect(err).NotTo(HaveOccurred())

By("patching the Deployment with dry-run")
err = cl.Patch(context.TODO(), dep, client.ConstantPatch(types.MergePatchType, mergePatch), client.PatchDryRunAll)
err = cl.Patch(context.TODO(), dep, client.RawPatch(types.MergePatchType, mergePatch), client.PatchDryRunAll)
Expect(err).NotTo(HaveOccurred())

By("validating patched Deployment doesn't have the new annotation")
Expand Down Expand Up @@ -1280,7 +1280,7 @@ var _ = Describe("Client", func() {
Kind: "Deployment",
Version: "v1",
})
err = cl.Patch(context.TODO(), u, client.ConstantPatch(types.MergePatchType, mergePatch))
err = cl.Patch(context.TODO(), u, client.RawPatch(types.MergePatchType, mergePatch))
Expect(err).NotTo(HaveOccurred())

By("validating patched Deployment has new annotation")
Expand All @@ -1305,7 +1305,7 @@ var _ = Describe("Client", func() {
u := &unstructured.Unstructured{}
Expect(scheme.Convert(dep, u, nil)).To(Succeed())
u.SetGroupVersionKind(depGvk)
err = cl.Patch(context.TODO(), u, client.ConstantPatch(types.MergePatchType, mergePatch))
err = cl.Patch(context.TODO(), u, client.RawPatch(types.MergePatchType, mergePatch))
Expect(err).NotTo(HaveOccurred())

By("validating updated Deployment has type information")
Expand All @@ -1332,7 +1332,7 @@ var _ = Describe("Client", func() {
Kind: "Node",
Version: "v1",
})
err = cl.Patch(context.TODO(), u, client.ConstantPatch(types.MergePatchType, mergePatch))
err = cl.Patch(context.TODO(), u, client.RawPatch(types.MergePatchType, mergePatch))
Expect(err).NotTo(HaveOccurred())

By("validating patched Node has new annotation")
Expand All @@ -1357,7 +1357,7 @@ var _ = Describe("Client", func() {
Kind: "Node",
Version: "v1",
})
err = cl.Patch(context.TODO(), node, client.ConstantPatch(types.MergePatchType, mergePatch))
err = cl.Patch(context.TODO(), node, client.RawPatch(types.MergePatchType, mergePatch))
Expect(err).To(HaveOccurred())

close(done)
Expand All @@ -1382,7 +1382,7 @@ var _ = Describe("Client", func() {
Kind: "Deployment",
Version: "v1",
})
err = cl.Patch(context.TODO(), u, client.ConstantPatch(types.MergePatchType, mergePatch), client.PatchDryRunAll)
err = cl.Patch(context.TODO(), u, client.RawPatch(types.MergePatchType, mergePatch), client.PatchDryRunAll)
Expect(err).NotTo(HaveOccurred())

By("validating patched Deployment does not have the new annotation")
Expand Down
2 changes: 1 addition & 1 deletion pkg/client/fake/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ var _ = Describe("Fake client", func() {
},
})
Expect(err).NotTo(HaveOccurred())
err = cl.Patch(nil, dep, client.ConstantPatch(types.StrategicMergePatchType, mergePatch))
err = cl.Patch(nil, dep, client.RawPatch(types.StrategicMergePatchType, mergePatch))
Expect(err).NotTo(HaveOccurred())

By("Getting the patched deployment")
Expand Down
30 changes: 29 additions & 1 deletion pkg/client/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ import (
var (
// Apply uses server-side apply to patch the given object.
Apply = applyPatch{}

// Merge uses the raw object as a merge patch, without modifications.
// Use MergeFrom if you wish to compute a diff instead.
Merge = mergePatch{}
)

type patch struct {
Expand All @@ -43,9 +47,16 @@ func (s *patch) Data(obj runtime.Object) ([]byte, error) {
return s.data, nil
}

// RawPatch constructs a new Patch with the given PatchType and data.
func RawPatch(patchType types.PatchType, data []byte) Patch {
return &patch{patchType, data}
}

// ConstantPatch constructs a new Patch with the given PatchType and data.
//
// Deprecated: use RawPatch instead
func ConstantPatch(patchType types.PatchType, data []byte) Patch {
return &patch{patchType, data}
return RawPatch(patchType, data)
}

type mergeFromPatch struct {
Expand Down Expand Up @@ -77,6 +88,23 @@ func MergeFrom(obj runtime.Object) Patch {
return &mergeFromPatch{obj}
}

// mergePatch uses a raw merge strategy to patch the object.
type mergePatch struct{}

// Type implements Patch.
func (p mergePatch) Type() types.PatchType {
return types.MergePatchType
}

// Data implements Patch.
func (p mergePatch) Data(obj runtime.Object) ([]byte, error) {
// NB(directxman12): we might technically want to be using an actual encoder
// here (in case some more performant encoder is introduced) but this is
// correct and sufficient for our uses (it's what the JSON serializer in
// client-go does, more-or-less).
return json.Marshal(obj)
}

// applyPatch uses server-side apply to patch the object.
type applyPatch struct{}

Expand Down