Merge pull request #1042 from kubernetes-sigs/master

Prepare for v0.6.1

Author: k8s-ci-robot

OWNERS_ALIASES

@@ -7,6 +7,7 @@ aliases:
   - directxman12
   - droot
   - mengqiy
+  - pwittrock
 
   # non-admin folks who have write-access and can approve any PRs in the repo
   controller-runtime-maintainers:
@@ -36,5 +37,4 @@ aliases:
 
   # folks who may have context on ancient history,
   # but are no longer directly involved
-  controller-runtime-emeritus-maintainers:
-  - pwittrock
+  # controller-runtime-emeritus-maintainers:

examples/builtins/controller.go

@@ -18,6 +18,7 @@ package main
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/go-logr/logr"
 
@@ -50,8 +51,7 @@ func (r *reconcileReplicaSet) Reconcile(request reconcile.Request) (reconcile.Re
 	}
 
 	if err != nil {
-		log.Error(err, "Could not fetch ReplicaSet")
-		return reconcile.Result{}, err
+		return reconcile.Result{}, fmt.Errorf("could not fetch ReplicaSet: %+v", err)
 	}
 
 	// Print the ReplicaSet
@@ -69,8 +69,7 @@ func (r *reconcileReplicaSet) Reconcile(request reconcile.Request) (reconcile.Re
 	rs.Labels["hello"] = "world"
 	err = r.client.Update(context.TODO(), rs)
 	if err != nil {
-		log.Error(err, "Could not write ReplicaSet")
-		return reconcile.Result{}, err
+		return reconcile.Result{}, fmt.Errorf("could not write ReplicaSet: %+v", err)
 	}
 
 	return reconcile.Result{}, nil

go.mod

@@ -3,27 +3,34 @@ module sigs.k8s.io/controller-runtime
 go 1.13
 
 require (
+	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/evanphx/json-patch v4.5.0+incompatible
+	github.com/fsnotify/fsnotify v1.4.9
 	github.com/go-logr/logr v0.1.0
 	github.com/go-logr/zapr v0.1.0
 	github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef // indirect
 	github.com/googleapis/gnostic v0.3.1 // indirect
-	github.com/imdario/mergo v0.3.6 // indirect
-	github.com/onsi/ginkgo v1.11.0
-	github.com/onsi/gomega v1.8.1
+	github.com/hashicorp/golang-lru v0.5.4 // indirect
+	github.com/imdario/mergo v0.3.9 // indirect
+	github.com/json-iterator/go v1.1.10 // indirect
+	github.com/onsi/ginkgo v1.12.1
+	github.com/onsi/gomega v1.10.1
 	github.com/prometheus/client_golang v1.0.0
 	github.com/prometheus/client_model v0.2.0
+	github.com/prometheus/procfs v0.0.11 // indirect
 	github.com/spf13/pflag v1.0.5
 	go.uber.org/atomic v1.4.0 // indirect
 	go.uber.org/zap v1.10.0
+	golang.org/x/text v0.3.3 // indirect
 	golang.org/x/time v0.0.0-20190308202827-9d24e82272b4
 	gomodules.xyz/jsonpatch/v2 v2.0.1
 	gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
-	gopkg.in/fsnotify.v1 v1.4.7
-	k8s.io/api v0.18.2
-	k8s.io/apiextensions-apiserver v0.18.2
-	k8s.io/apimachinery v0.18.2
-	k8s.io/client-go v0.18.2
-	k8s.io/utils v0.0.0-20200324210504-a9aa75ae1b89
+	k8s.io/api v0.18.4
+	k8s.io/apiextensions-apiserver v0.18.4
+	k8s.io/apimachinery v0.18.4
+	k8s.io/client-go v0.18.4
+	k8s.io/utils v0.0.0-20200603063816-c1c6865ac451
 	sigs.k8s.io/yaml v1.2.0
 )
+
+replace github.com/evanphx/json-patch => github.com/evanphx/json-patch v0.0.0-20190815234213-e83c0a1c26c8

go.sum

@@ -53,7 +53,7 @@ func GVKForObject(obj runtime.Object, scheme *runtime.Scheme) (schema.GroupVersi
 		return schema.GroupVersionKind{}, err
 	}
 	if isUnversioned {
-		return schema.GroupVersionKind{}, fmt.Errorf("cannot create a new informer for the unversioned type %T", obj)
+		return schema.GroupVersionKind{}, fmt.Errorf("cannot create group-version-kind for unversioned type %T", obj)
 	}
 
 	if len(gvks) < 1 {

pkg/client/apiutil/apimachinery.go

@@ -1210,6 +1210,42 @@ var _ = Describe("Client", func() {
 				close(done)
 			})
 
+			It("should patch an existing object from a go struct, using optimistic locking", func(done Done) {
+				cl, err := client.New(cfg, client.Options{})
+				Expect(err).NotTo(HaveOccurred())
+				Expect(cl).NotTo(BeNil())
+
+				By("initially creating a Deployment")
+				dep, err := clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{})
+				Expect(err).NotTo(HaveOccurred())
+
+				By("creating a patch from with optimistic lock")
+				patch := client.MergeFromWithOptions(dep.DeepCopy(), client.MergeFromWithOptimisticLock{})
+
+				By("adding a new annotation")
+				dep.Annotations = map[string]string{
+					"foo": "bar",
+				}
+
+				By("patching the Deployment")
+				err = cl.Patch(context.TODO(), dep, patch)
+				Expect(err).NotTo(HaveOccurred())
+
+				By("validating patched Deployment has new annotation")
+				actual, err := clientset.AppsV1().Deployments(ns).Get(ctx, dep.Name, metav1.GetOptions{})
+				Expect(err).NotTo(HaveOccurred())
+				Expect(actual).NotTo(BeNil())
+				Expect(actual.Annotations["foo"]).To(Equal("bar"))
+
+				By("validating that a patch should fail with conflict, when it has an outdated resource version")
+				dep.Annotations["should"] = "conflict"
+				err = cl.Patch(context.TODO(), dep, patch)
+				Expect(err).To(HaveOccurred())
+				Expect(apierrors.IsConflict(err)).To(BeTrue())
+
+				close(done)
+			})
+
 			It("should patch and preserve type information", func(done Done) {
 				cl, err := client.New(cfg, client.Options{})
 				Expect(err).NotTo(HaveOccurred())
@@ -2656,8 +2692,9 @@ var _ = Describe("Patch", func() {
 		BeforeEach(func() {
 			cm = &corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
-					Namespace: metav1.NamespaceDefault,
-					Name:      "cm",
+					Namespace:       metav1.NamespaceDefault,
+					Name:            "cm",
+					ResourceVersion: "10",
 				},
 			}
 		})
@@ -2686,6 +2723,31 @@ var _ = Describe("Patch", func() {
 			By("returning a patch with data only containing the annotation change")
 			Expect(data).To(Equal([]byte(fmt.Sprintf(`{"metadata":{"annotations":{"%s":"%s"}}}`, annotationKey, annotationValue))))
 		})
+
+		It("creates a merge patch with the modifications applied during the mutation, using optimistic locking", func() {
+			const (
+				annotationKey   = "test"
+				annotationValue = "foo"
+			)
+
+			By("creating a merge patch")
+			patch := client.MergeFromWithOptions(cm.DeepCopy(), client.MergeFromWithOptimisticLock{})
+
+			By("returning a patch with type MergePatch")
+			Expect(patch.Type()).To(Equal(types.MergePatchType))
+
+			By("retrieving modifying the config map")
+			metav1.SetMetaDataAnnotation(&cm.ObjectMeta, annotationKey, annotationValue)
+
+			By("computing the patch data")
+			data, err := patch.Data(cm)
+
+			By("returning no error")
+			Expect(err).NotTo(HaveOccurred())
+
+			By("returning a patch with data containing the annotation change and the resourceVersion change")
+			Expect(data).To(Equal([]byte(fmt.Sprintf(`{"metadata":{"annotations":{"%s":"%s"},"resourceVersion":"%s"}}`, annotationKey, annotationValue, cm.ResourceVersion))))
+		})
 	})
 })
 

pkg/client/client_test.go

@@ -20,12 +20,15 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
+
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 )
@@ -97,8 +100,10 @@ func ExampleClient_create() {
 	// Using a unstructured object.
 	u := &unstructured.Unstructured{}
 	u.Object = map[string]interface{}{
-		"name":      "name",
-		"namespace": "namespace",
+		"metadata": map[string]interface{}{
+			"name":      "name",
+			"namespace": "namespace",
+		},
 		"spec": map[string]interface{}{
 			"replicas": 2,
 			"selector": map[string]interface{}{
@@ -173,6 +178,35 @@ func ExampleClient_update() {
 	_ = c.Update(context.Background(), u)
 }
 
+// This example shows how to use the client with typed and unstructured objects to patch objects.
+func ExampleClient_patch() {
+	patch := []byte(`{"metadata":{"annotations":{"version": "v2"}}}`)
+	_ = c.Patch(context.Background(), &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "namespace",
+			Name:      "name",
+		},
+	}, client.RawPatch(types.StrategicMergePatchType, patch))
+}
+
+// This example shows how to use the client with typed and unstructured objects to patch objects' status.
+func ExampleClient_patchStatus() {
+	u := &unstructured.Unstructured{}
+	u.Object = map[string]interface{}{
+		"metadata": map[string]interface{}{
+			"name":      "foo",
+			"namespace": "namespace",
+		},
+	}
+	u.SetGroupVersionKind(schema.GroupVersionKind{
+		Group:   "batch",
+		Version: "v1beta1",
+		Kind:    "CronJob",
+	})
+	patch := []byte(fmt.Sprintf(`{"status":{"lastScheduleTime":"%s"}}`, time.Now().Format(time.RFC3339)))
+	_ = c.Status().Patch(context.Background(), u, client.RawPatch(types.MergePatchType, patch))
+}
+
 // This example shows how to use the client with typed and unstructured objects to delete objects.
 func ExampleClient_delete() {
 	// Using a typed object.

pkg/client/example_test.go

@@ -96,7 +96,11 @@ func (t versionedTracker) Create(gvr schema.GroupVersionResource, obj runtime.Ob
 		return apierrors.NewBadRequest("resourceVersion can not be set for Create requests")
 	}
 	accessor.SetResourceVersion("1")
-	return t.ObjectTracker.Create(gvr, obj, ns)
+	if err := t.ObjectTracker.Create(gvr, obj, ns); err != nil {
+		accessor.SetResourceVersion("")
+		return err
+	}
+	return nil
 }
 
 func (t versionedTracker) Update(gvr schema.GroupVersionResource, obj runtime.Object, ns string) error {
@@ -267,7 +271,7 @@ func (c *fakeClient) Delete(ctx context.Context, obj runtime.Object, opts ...cli
 }
 
 func (c *fakeClient) DeleteAllOf(ctx context.Context, obj runtime.Object, opts ...client.DeleteAllOfOption) error {
-	gvk, err := apiutil.GVKForObject(obj, scheme.Scheme)
+	gvk, err := apiutil.GVKForObject(obj, c.scheme)
 	if err != nil {
 		return err
 	}

pkg/client/fake/client.go

@@ -186,6 +186,14 @@ var _ = Describe("Fake client", func() {
 			Expect(apierrors.IsBadRequest(err)).To(BeTrue())
 		})
 
+		It("should not change the submitted object if Create failed", func() {
+			By("Trying to create an existing configmap")
+			submitted := cm.DeepCopy()
+			err := cl.Create(context.Background(), submitted)
+			Expect(apierrors.IsAlreadyExists(err)).To(BeTrue())
+			Expect(submitted).To(Equal(cm))
+		})
+
 		It("should error on Create with empty Name", func() {
 			By("Creating a new configmap")
 			newcm := &corev1.ConfigMap{

pkg/client/fake/client_test.go

@@ -17,7 +17,11 @@ limitations under the License.
 package client
 
 import (
+	"fmt"
+
 	jsonpatch "github.com/evanphx/json-patch"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/json"
@@ -59,8 +63,39 @@ func ConstantPatch(patchType types.PatchType, data []byte) Patch {
 	return RawPatch(patchType, data)
 }
 
+// MergeFromWithOptimisticLock can be used if clients want to make sure a patch
+// is being applied to the latest resource version of an object.
+//
+// The behavior is similar to what an Update would do, without the need to send the
+// whole object. Usually this method is useful if you might have multiple clients
+// acting on the same object and the same API version, but with different versions of the Go structs.
+//
+// For example, an "older" copy of a Widget that has fields A and B, and a "newer" copy with A, B, and C.
+// Sending an update using the older struct definition results in C being dropped, whereas using a patch does not.
+type MergeFromWithOptimisticLock struct{}
+
+// ApplyToMergeFrom applies this configuration to the given patch options.
+func (m MergeFromWithOptimisticLock) ApplyToMergeFrom(in *MergeFromOptions) {
+	in.OptimisticLock = true
+}
+
+// MergeFromOption is some configuration that modifies options for a merge-from patch data.
+type MergeFromOption interface {
+	// ApplyToMergeFrom applies this configuration to the given patch options.
+	ApplyToMergeFrom(*MergeFromOptions)
+}
+
+// MergeFromOptions contains options to generate a merge-from patch data.
+type MergeFromOptions struct {
+	// OptimisticLock, when true, includes `metadata.resourceVersion` into the final
+	// patch data. If the `resourceVersion` field doesn't match what's stored,
+	// the operation results in a conflict and clients will need to try again.
+	OptimisticLock bool
+}
+
 type mergeFromPatch struct {
 	from runtime.Object
+	opts MergeFromOptions
 }
 
 // Type implements patch.
@@ -80,12 +115,47 @@ func (s *mergeFromPatch) Data(obj runtime.Object) ([]byte, error) {
 		return nil, err
 	}
 
-	return jsonpatch.CreateMergePatch(originalJSON, modifiedJSON)
+	data, err := jsonpatch.CreateMergePatch(originalJSON, modifiedJSON)
+	if err != nil {
+		return nil, err
+	}
+
+	if s.opts.OptimisticLock {
+		dataMap := map[string]interface{}{}
+		if err := json.Unmarshal(data, &dataMap); err != nil {
+			return nil, err
+		}
+		fromMeta, ok := s.from.(metav1.Object)
+		if !ok {
+			return nil, fmt.Errorf("cannot use OptimisticLock, from object %q is not a valid metav1.Object", s.from)
+		}
+		resourceVersion := fromMeta.GetResourceVersion()
+		if len(resourceVersion) == 0 {
+			return nil, fmt.Errorf("cannot use OptimisticLock, from object %q does not have any resource version we can use", s.from)
+		}
+		u := &unstructured.Unstructured{Object: dataMap}
+		u.SetResourceVersion(resourceVersion)
+		data, err = json.Marshal(u)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	return data, nil
 }
 
 // MergeFrom creates a Patch that patches using the merge-patch strategy with the given object as base.
 func MergeFrom(obj runtime.Object) Patch {
-	return &mergeFromPatch{obj}
+	return &mergeFromPatch{from: obj}
+}
+
+// MergeFromWithOptions creates a Patch that patches using the merge-patch strategy with the given object as base.
+func MergeFromWithOptions(obj runtime.Object, opts ...MergeFromOption) Patch {
+	options := &MergeFromOptions{}
+	for _, opt := range opts {
+		opt.ApplyToMergeFrom(options)
+	}
+	return &mergeFromPatch{from: obj, opts: *options}
 }
 
 // mergePatch uses a raw merge strategy to patch the object.