Merge pull request #3258 from vincepri/add-opts-patch

🌱 Allow Patch to specify options as well
kubernetes-sigs · Jun 25, 2020 · bf704ba · bf704ba
2 parents fd73b18 + 75b02ce
commit bf704ba
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 18 deletions.
diff --git a/util/patch/patch.go b/util/patch/patch.go
@@ -34,8 +34,6 @@ import (
 
 // Helper is a utility for ensuring the proper patching of objects.
 type Helper struct {
-	opts *HelperOptions
-
 	client       client.Client
 	beforeObject runtime.Object
 	before       *unstructured.Unstructured
@@ -46,20 +44,14 @@ type Helper struct {
 }
 
 // NewHelper returns an initialized Helper
-func NewHelper(obj runtime.Object, crClient client.Client, opts ...Option) (*Helper, error) {
+func NewHelper(obj runtime.Object, crClient client.Client) (*Helper, error) {
 	// Return early if the object is nil.
 	// If you're wondering why we need reflection to do this check, see https://golang.org/doc/faq#nil_error.
 	// TODO(vincepri): Remove this check and let it panic if used improperly in a future minor release.
 	if obj == nil || (reflect.ValueOf(obj).IsValid() && reflect.ValueOf(obj).IsNil()) {
 		return nil, errors.Errorf("expected non-nil object")
 	}
 
-	// Calculate the options to pass to the helper.
-	options := &HelperOptions{}
-	for _, opt := range opts {
-		opt.ApplyToHelper(options)
-	}
-
 	// Convert the object to unstructured to compare against our before copy.
 	raw, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj.DeepCopyObject())
 	if err != nil {
@@ -71,7 +63,6 @@ func NewHelper(obj runtime.Object, crClient client.Client, opts ...Option) (*Hel
 	_, canInterfaceConditions := obj.(conditions.Setter)
 
 	return &Helper{
-		opts:               options,
 		client:             crClient,
 		before:             unstructuredObj,
 		beforeObject:       obj.DeepCopyObject(),
@@ -80,11 +71,17 @@ func NewHelper(obj runtime.Object, crClient client.Client, opts ...Option) (*Hel
 }
 
 // Patch will attempt to patch the given object, including its status.
-func (h *Helper) Patch(ctx context.Context, obj runtime.Object) error {
+func (h *Helper) Patch(ctx context.Context, obj runtime.Object, opts ...Option) error {
 	if obj == nil {
 		return errors.Errorf("expected non-nil object")
 	}
 
+	// Calculate the options.
+	options := &HelperOptions{}
+	for _, opt := range opts {
+		opt.ApplyToHelper(options)
+	}
+
 	// Convert the object to unstructured to compare against our before copy.
 	raw, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj.DeepCopyObject())
 	if err != nil {
@@ -94,7 +91,7 @@ func (h *Helper) Patch(ctx context.Context, obj runtime.Object) error {
 
 	// Determine if the object has status.
 	if unstructuredHasStatus(h.after) {
-		if h.opts.IncludeStatusObservedGeneration {
+		if options.IncludeStatusObservedGeneration {
 			// Set status.observedGeneration if we're asked to do so.
 			if err := unstructured.SetNestedField(h.after.Object, h.after.GetGeneration(), "status", "observedGeneration"); err != nil {
 				return err

diff --git a/util/patch/patch_test.go b/util/patch/patch_test.go
@@ -530,14 +530,14 @@ var _ = Describe("Patch Helper", func() {
 			}()
 
 			By("Creating a new patch helper")
-			patcher, err := NewHelper(obj, testEnv, WithStatusObservedGeneration{})
+			patcher, err := NewHelper(obj, testEnv)
 			Expect(err).NotTo(HaveOccurred())
 
 			By("Updating the object spec")
 			obj.Spec.Replicas = pointer.Int32Ptr(10)
 
 			By("Patching the object")
-			Expect(patcher.Patch(ctx, obj)).To(Succeed())
+			Expect(patcher.Patch(ctx, obj, WithStatusObservedGeneration{})).To(Succeed())
 
 			By("Validating the object has been updated")
 			Eventually(func() bool {
@@ -562,7 +562,7 @@ var _ = Describe("Patch Helper", func() {
 			}()
 
 			By("Creating a new patch helper")
-			patcher, err := NewHelper(obj, testEnv, WithStatusObservedGeneration{})
+			patcher, err := NewHelper(obj, testEnv)
 			Expect(err).NotTo(HaveOccurred())
 
 			By("Updating the object spec")
@@ -578,7 +578,7 @@ var _ = Describe("Patch Helper", func() {
 			}
 
 			By("Patching the object")
-			Expect(patcher.Patch(ctx, obj)).To(Succeed())
+			Expect(patcher.Patch(ctx, obj, WithStatusObservedGeneration{})).To(Succeed())
 
 			By("Validating the object has been updated")
 			Eventually(func() bool {
@@ -607,11 +607,11 @@ var _ = Describe("Patch Helper", func() {
 			Expect(testEnv.Status().Update(ctx, obj))
 
 			By("Creating a new patch helper")
-			patcher, err := NewHelper(obj, testEnv, WithStatusObservedGeneration{})
+			patcher, err := NewHelper(obj, testEnv)
 			Expect(err).NotTo(HaveOccurred())
 
 			By("Patching the object")
-			Expect(patcher.Patch(ctx, obj)).To(Succeed())
+			Expect(patcher.Patch(ctx, obj, WithStatusObservedGeneration{})).To(Succeed())
 
 			By("Validating the object has been updated")
 			Eventually(func() bool {