🏃 Add options to patch helper for ObservedGeneration

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

go.mod

@@ -39,7 +39,7 @@ require (
 	k8s.io/klog v1.0.0
 	k8s.io/kube-openapi v0.0.0-20200121204235-bf4fb3bd569c // indirect
 	k8s.io/utils v0.0.0-20200229041039-0a110f9eb7ab
-	sigs.k8s.io/controller-runtime v0.5.3
+	sigs.k8s.io/controller-runtime v0.5.5-0.20200529152116-5291db5ab70e
 	sigs.k8s.io/kind v0.7.1-0.20200303021537-981bd80d3802
 	sigs.k8s.io/yaml v1.2.0
 )

go.sum

@@ -653,8 +653,8 @@ modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk=
 modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k=
 modernc.org/strutil v1.0.0/go.mod h1:lstksw84oURvj9y3tn8lGvRxyRC1S2+g5uuIzNfIOBs=
 modernc.org/xc v1.0.0/go.mod h1:mRNCo0bvLjGhHO9WsyuKVU4q0ceiDDDoEeWDJHrNx8I=
-sigs.k8s.io/controller-runtime v0.5.3 h1:HGL1kpWxe6WiiN4yCmYA5SFtvUMQAkOwV+Ylh7M87E4=
-sigs.k8s.io/controller-runtime v0.5.3/go.mod h1:JZUwSMVbxDupo0lTJSSFP5pimEyxGynROImSsqIOx1A=
+sigs.k8s.io/controller-runtime v0.5.5-0.20200529152116-5291db5ab70e h1:X/iF0awmtZnNud5vm+LNgPRzZsjyCWqGJtbluTBX9x8=
+sigs.k8s.io/controller-runtime v0.5.5-0.20200529152116-5291db5ab70e/go.mod h1:JZUwSMVbxDupo0lTJSSFP5pimEyxGynROImSsqIOx1A=
 sigs.k8s.io/kind v0.7.1-0.20200303021537-981bd80d3802 h1:L6/8hETA7jvdx3xBcbDifrIN2xaYHE7tA58n+Kdp2Zw=
 sigs.k8s.io/kind v0.7.1-0.20200303021537-981bd80d3802/go.mod h1:HIZ3PWUezpklcjkqpFbnYOqaqsAE1JeCTEwkgvPLXjk=
 sigs.k8s.io/structured-merge-diff v0.0.0-20190525122527-15d366b2352e/go.mod h1:wWxsB5ozmmv/SG7nM11ayaAW51xMvak/t1r0CSlcokI=

test/infrastructure/docker/go.mod

@@ -11,7 +11,7 @@ require (
 	k8s.io/client-go v0.17.2
 	k8s.io/klog v1.0.0
 	sigs.k8s.io/cluster-api v0.3.3
-	sigs.k8s.io/controller-runtime v0.5.3
+	sigs.k8s.io/controller-runtime v0.5.5-0.20200529152116-5291db5ab70e
 	sigs.k8s.io/kind v0.7.1-0.20200303021537-981bd80d3802
 	sigs.k8s.io/yaml v1.2.0
 )

test/infrastructure/docker/go.sum

@@ -580,8 +580,8 @@ modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk=
 modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k=
 modernc.org/strutil v1.0.0/go.mod h1:lstksw84oURvj9y3tn8lGvRxyRC1S2+g5uuIzNfIOBs=
 modernc.org/xc v1.0.0/go.mod h1:mRNCo0bvLjGhHO9WsyuKVU4q0ceiDDDoEeWDJHrNx8I=
-sigs.k8s.io/controller-runtime v0.5.3 h1:HGL1kpWxe6WiiN4yCmYA5SFtvUMQAkOwV+Ylh7M87E4=
-sigs.k8s.io/controller-runtime v0.5.3/go.mod h1:JZUwSMVbxDupo0lTJSSFP5pimEyxGynROImSsqIOx1A=
+sigs.k8s.io/controller-runtime v0.5.5-0.20200529152116-5291db5ab70e h1:X/iF0awmtZnNud5vm+LNgPRzZsjyCWqGJtbluTBX9x8=
+sigs.k8s.io/controller-runtime v0.5.5-0.20200529152116-5291db5ab70e/go.mod h1:JZUwSMVbxDupo0lTJSSFP5pimEyxGynROImSsqIOx1A=
 sigs.k8s.io/kind v0.7.1-0.20200303021537-981bd80d3802 h1:L6/8hETA7jvdx3xBcbDifrIN2xaYHE7tA58n+Kdp2Zw=
 sigs.k8s.io/kind v0.7.1-0.20200303021537-981bd80d3802/go.mod h1:HIZ3PWUezpklcjkqpFbnYOqaqsAE1JeCTEwkgvPLXjk=
 sigs.k8s.io/structured-merge-diff v0.0.0-20190525122527-15d366b2352e/go.mod h1:wWxsB5ozmmv/SG7nM11ayaAW51xMvak/t1r0CSlcokI=

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
+
+// 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
+}
+
+// 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
+}

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.
@@ -21,29 +21,41 @@ import (
 	"reflect"
 
 	"github.com/pkg/errors"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/apimachinery/pkg/util/wait"
+	"sigs.k8s.io/cluster-api/util/conditions"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // 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{}
+	beforeStatus     map[string]interface{}
+	beforeConditions map[string]interface{}
+	resourcePatch    client.Patch
+	statusPatch      client.Patch
+	conditionsPatch  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.
@@ -57,26 +69,25 @@ func NewHelper(resource runtime.Object, crClient client.Client) (*Helper, error)
 		return nil, err
 	}
 
-	hasStatus := false
-	// attempt to extract the status from the resource for easier comparison later
-	beforeStatus, ok, err := unstructured.NestedFieldCopy(before, "status")
+	beforeConditions, err := extractMap(before, "status", "conditions")
 	if err != nil {
 		return nil, err
 	}
-	if ok {
-		hasStatus = true
-		// if the resource contains a status remove it from our unstructured copy
-		// to avoid unnecessary patching later
-		unstructured.RemoveNestedField(before, "status")
+
+	beforeStatus, err := extractMap(before, "status")
+	if err != nil {
+		return nil, err
 	}
 
 	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,
+		beforeConditions: beforeConditions,
+		resourcePatch:    client.MergeFrom(resource.DeepCopyObject()),
+		statusPatch:      client.MergeFrom(resource.DeepCopyObject()),
+		conditionsPatch:  client.MergeFromWithOptions(resource.DeepCopyObject(), client.MergeFromWithOptimisticLock{}),
 	}, nil
 }
 
@@ -86,49 +97,132 @@ 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.
+	if unstructuredHasStatus(unstructuredResource) {
+		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
-	afterStatus, ok, err := unstructured.NestedFieldCopy(after, "status")
+	// Make a copy of the resource to compare with before/beforeStatus.
+	after := unstructuredResource.DeepCopy().Object
+
+	// Attempt to extract status.conditions.
+	afterConditions, err := extractMap(after, "status", "conditions")
 	if err != nil {
 		return err
 	}
-	if ok {
-		hasStatus = true
-		// if the resource contains a status remove it from our unstructured copy
-		// to avoid uneccsary patching.
-		unstructured.RemoveNestedField(after, "status")
+
+	// Attempt to extract the status.
+	afterStatus, err := extractMap(after, "status")
+	if err != nil {
+		return err
 	}
 
 	var errs []error
 
-	if !reflect.DeepEqual(h.before, after) {
-		// only issue a Patch if the before and after resources (minus status) differ
+	// If the before/after map (containing only metadata & spec) is not the same, issue a patch.
+	if shouldPatch(h.before, after) {
 		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 shouldPatch(h.beforeStatus, afterStatus) {
 		if err := h.client.Status().Patch(ctx, resource.DeepCopyObject(), h.statusPatch); err != nil {
 			errs = append(errs, err)
 		}
 	}
 
+	// If the beforeConditions/afterConditions is not the same, issue patch with optimistic locking.
+	if shouldPatch(h.beforeConditions, afterConditions) {
+		conditionPatch := conditions.NewPatch(
+			conditions.UnstructuredGetter(&unstructured.Unstructured{Object: h.before}),
+			conditions.UnstructuredGetter(unstructuredResource),
+		)
+
+		tmp := resource.DeepCopyObject()
+		key, err := client.ObjectKeyFromObject(tmp)
+		if err != nil {
+			return err
+		}
+
+		backoff := wait.Backoff{}
+		if err := wait.ExponentialBackoff(backoff, func() (bool, error) {
+			// Issue a patch.
+			err := h.client.Status().Patch(ctx, tmp, h.conditionsPatch)
+			switch {
+			case apierrors.IsConflict(err):
+				// Handle conflicts.
+				// Get a new copy of the object.
+				if err := h.client.Get(ctx, key, tmp); err != nil {
+					return false, err
+				}
+
+				// Reset the condition patch before merging conditions.
+				h.conditionsPatch = client.MergeFromWithOptions(tmp.DeepCopyObject(), client.MergeFromWithOptimisticLock{})
+
+				// Convert the newly-retrieved object to unstructured.
+				raw, err := runtime.DefaultUnstructuredConverter.ToUnstructured(tmp.DeepCopyObject())
+				if err != nil {
+					return false, err
+				}
+				u := &unstructured.Unstructured{Object: raw}
+
+				// Set the condition patch previously creatd on the new object.
+				if err := conditionPatch.Apply(conditions.UnstructuredSetter(u)); err != nil {
+					return false, err
+				}
+
+				// Restore the changes back to the temporary copy.
+				if err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, tmp); err != nil {
+					return false, err
+				}
+
+				return false, nil
+			case err != nil:
+				return false, err
+			}
+
+			return true, nil
+		}); err != nil {
+			errs = append(errs, err)
+		}
+	}
+
 	return kerrors.NewAggregate(errs)
 }
+
+func unstructuredHasStatus(resource *unstructured.Unstructured) bool {
+	_, ok := resource.Object["status"]
+	return ok
+}
+
+func extractMap(m map[string]interface{}, fields ...string) (map[string]interface{}, error) {
+	x, ok, err := unstructured.NestedMap(m, fields...)
+	if err != nil || !ok {
+		return nil, err
+	}
+	unstructured.RemoveNestedField(m, fields...)
+	return x, nil
+}
+
+func shouldPatch(before, after interface{}) bool {
+	return (before != nil || after != nil) && !reflect.DeepEqual(before, after)
+}