🏃 Add options to patch helper for ObservedGeneration

Signed-off-by: Vince Prignano <[email protected]>

util/patch/options.go

@@ -0,0 +1,37 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package patch
+
+// WithStatusObservedGeneration sets the status.observedGeneration field
+// on the incoming object to match metadata.generation, only if there is a change.
+type WithStatusObservedGeneration struct{}
+
+// ApplyToHelper applies this configuration to the given an List options.
+func (w WithStatusObservedGeneration) ApplyToHelper(in *HelperOptions) {
+	in.IncludeStatusObservedGeneration = true
+}
+
+// Option is some configuration that modifies options for a patch request.
+type Option interface {
+	// ApplyToHelper applies this configuration to the given Helper options.
+	ApplyToHelper(*HelperOptions)
+}
+
+// HelperOptions contains options for patch options.
+type HelperOptions struct {
+	IncludeStatusObservedGeneration bool
+}

util/patch/patch.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2017 The Kubernetes Authors.
+Copyright 2020 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -30,20 +30,28 @@ import (
 // Helper is a utility for ensuring the proper Patching of resources
 // and their status
 type Helper struct {
-	client        client.Client
-	before        map[string]interface{}
-	hasStatus     bool
-	beforeStatus  interface{}
-	resourcePatch client.Patch
-	statusPatch   client.Patch
+	opts *HelperOptions
+
+	client          client.Client
+	before          map[string]interface{}
+	beforeHasStatus bool
+	beforeStatus    interface{}
+	resourcePatch   client.Patch
+	statusPatch     client.Patch
 }
 
 // NewHelper returns an initialized Helper
-func NewHelper(resource runtime.Object, crClient client.Client) (*Helper, error) {
+func NewHelper(resource runtime.Object, crClient client.Client, opts ...Option) (*Helper, error) {
 	if resource == nil {
 		return nil, errors.Errorf("expected non-nil resource")
 	}
 
+	// Calculate the options to pass to the helper.
+	options := &HelperOptions{}
+	for _, opt := range opts {
+		opt.ApplyToHelper(options)
+	}
+
 	// If the object is already unstructured, we need to perform a deepcopy first
 	// because the `DefaultUnstructuredConverter.ToUnstructured` function returns
 	// the underlying unstructured object map without making a copy.
@@ -71,12 +79,13 @@ func NewHelper(resource runtime.Object, crClient client.Client) (*Helper, error)
 	}
 
 	return &Helper{
-		client:        crClient,
-		before:        before,
-		beforeStatus:  beforeStatus,
-		hasStatus:     hasStatus,
-		resourcePatch: client.MergeFrom(resource.DeepCopyObject()),
-		statusPatch:   client.MergeFrom(resource.DeepCopyObject()),
+		opts:            options,
+		client:          crClient,
+		before:          before,
+		beforeStatus:    beforeStatus,
+		beforeHasStatus: hasStatus,
+		resourcePatch:   client.MergeFrom(resource.DeepCopyObject()),
+		statusPatch:     client.MergeFrom(resource.DeepCopyObject()),
 	}, nil
 }
 
@@ -86,49 +95,63 @@ func (h *Helper) Patch(ctx context.Context, resource runtime.Object) error {
 		return errors.Errorf("expected non-nil resource")
 	}
 
-	// If the object is already unstructured, we need to perform a deepcopy first
-	// because the `DefaultUnstructuredConverter.ToUnstructured` function returns
-	// the underlying unstructured object map without making a copy.
-	if _, ok := resource.(runtime.Unstructured); ok {
-		resource = resource.DeepCopyObject()
-	}
-
 	// Convert the resource to unstructured to compare against our before copy.
-	after, err := runtime.DefaultUnstructuredConverter.ToUnstructured(resource)
+	raw, err := runtime.DefaultUnstructuredConverter.ToUnstructured(resource.DeepCopyObject())
 	if err != nil {
 		return err
 	}
+	unstructuredResource := &unstructured.Unstructured{Object: raw}
+
+	// Determine if the object has status.
+	afterHasStatus := unstructuredHasStatus(unstructuredResource)
+	if afterHasStatus {
+		if h.opts.IncludeStatusObservedGeneration {
+			// Set status.observedGeneration if we're asked to do so.
+			if err := unstructured.SetNestedField(unstructuredResource.Object, unstructuredResource.GetGeneration(), "status", "observedGeneration"); err != nil {
+				return err
+			}
+
+			// Restore the changes back to the original object.
+			if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredResource.Object, resource); err != nil {
+				return err
+			}
+		}
+	}
 
-	hasStatus := false
-	// attempt to extract the status from the resource to compare against our
-	// beforeStatus copy
+	// Make a copy of the resource to compare with before/beforeStatus.
+	after := unstructuredResource.DeepCopy().Object
+
+	// Attempt to extract the status.
 	afterStatus, ok, err := unstructured.NestedFieldCopy(after, "status")
 	if err != nil {
 		return err
 	}
 	if ok {
-		hasStatus = true
-		// if the resource contains a status remove it from our unstructured copy
+		// If the resource contains a status remove it from our unstructured copy
 		// to avoid uneccsary patching.
 		unstructured.RemoveNestedField(after, "status")
 	}
 
 	var errs []error
 
+	// If the before/after map (containing only metadata & spec) is not the same, issue a patch.
 	if !reflect.DeepEqual(h.before, after) {
-		// only issue a Patch if the before and after resources (minus status) differ
 		if err := h.client.Patch(ctx, resource.DeepCopyObject(), h.resourcePatch); err != nil {
 			errs = append(errs, err)
 		}
 	}
 
-	if (h.hasStatus || hasStatus) && !reflect.DeepEqual(h.beforeStatus, afterStatus) {
-		// only issue a Status Patch if the resource has a status and the beforeStatus
-		// and afterStatus copies differ
+	// If the beforeStatus/afterStatus map (containing only status fields) is not the same, issue patch.
+	if (h.beforeHasStatus || afterHasStatus) && !reflect.DeepEqual(h.beforeStatus, afterStatus) {
 		if err := h.client.Status().Patch(ctx, resource.DeepCopyObject(), h.statusPatch); err != nil {
 			errs = append(errs, err)
 		}
 	}
 
 	return kerrors.NewAggregate(errs)
 }
+
+func unstructuredHasStatus(resource *unstructured.Unstructured) bool {
+	_, ok := resource.Object["status"]
+	return ok
+}