From 06239f60ad981d131af0b59953502729bf3d550b Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sun, 24 Feb 2019 10:06:24 +0000 Subject: [PATCH 1/6] Add CreateOptions to client --- pkg/client/client.go | 2 +- pkg/client/interfaces.go | 53 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index adf46495b6..1ac1ed0393 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -104,7 +104,7 @@ type client struct { } // Create implements client.Client -func (c *client) Create(ctx context.Context, obj runtime.Object) error { +func (c *client) Create(ctx context.Context, obj runtime.Object, opts ...CreateOptionFunc) error { _, ok := obj.(*unstructured.Unstructured) if ok { return c.unstructuredClient.Create(ctx, obj) diff --git a/pkg/client/interfaces.go b/pkg/client/interfaces.go index 53fd4ad212..25b65ac7a4 100644 --- a/pkg/client/interfaces.go +++ b/pkg/client/interfaces.go @@ -57,7 +57,7 @@ type Reader interface { // Writer knows how to create, delete, and update Kubernetes objects. type Writer interface { // Create saves the object obj in the Kubernetes cluster. - Create(ctx context.Context, obj runtime.Object) error + Create(ctx context.Context, obj runtime.Object, opts ...CreateOptionFunc) error // Delete deletes the given obj from Kubernetes cluster. Delete(ctx context.Context, obj runtime.Object, opts ...DeleteOptionFunc) error @@ -106,6 +106,57 @@ type FieldIndexer interface { IndexField(obj runtime.Object, field string, extractValue IndexerFunc) error } +// CreateOptions contains options for create requests. It's generally a subset +// of metav1.CreateOptions. +type CreateOptions struct { + // When present, indicates that modifications should not be + // persisted. An invalid or unrecognized dryRun directive will + // result in an error response and no further processing of the + // request. Valid values are: + // - All: all dry run stages will be processed + DryRun []string + + // Raw represents raw CreateOptions, as passed to the API server. + Raw *metav1.CreateOptions +} + +// AsCreateOptions returns these options as a metav1.CreateOptions. +// This may mutate the Raw field. +func (o *CreateOptions) AsCreateOptions() *metav1.CreateOptions { + + if o == nil { + return &metav1.CreateOptions{} + } + if o.Raw == nil { + o.Raw = &metav1.CreateOptions{} + } + + o.Raw.DryRun = o.DryRun + return o.Raw +} + +// ApplyOptions executes the given CreateOptionFuncs and returns the mutated +// CreateOptions. +func (o *CreateOptions) ApplyOptions(optFuncs []CreateOptionFunc) *CreateOptions { + for _, optFunc := range optFuncs { + optFunc(o) + } + return o +} + +// CreateOptionFunc is a function that mutates a CreateOptions struct. It implements +// the functional options pattern. See +// https://github.com/tmrts/go-patterns/blob/master/idiom/functional-options.md. +type CreateOptionFunc func(*CreateOptions) + +// DryRunAll is a functional option that sets the DryRun +// field of a CreateOptions struct to metav1.DryRunAll. +func DryRunAll() CreateOptionFunc { + return func(opts *CreateOptions) { + opts.DryRun = []string{metav1.DryRunAll} + } +} + // DeleteOptions contains options for delete requests. It's generally a subset // of metav1.DeleteOptions. type DeleteOptions struct { From e2c92af9ec38ee0f00d05112bef1642ea6d73d30 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sun, 24 Feb 2019 10:09:51 +0000 Subject: [PATCH 2/6] Add UpdateOptions to client --- pkg/client/client.go | 2 +- pkg/client/interfaces.go | 57 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 1ac1ed0393..df77cf38bd 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -113,7 +113,7 @@ func (c *client) Create(ctx context.Context, obj runtime.Object, opts ...CreateO } // Update implements client.Client -func (c *client) Update(ctx context.Context, obj runtime.Object) error { +func (c *client) Update(ctx context.Context, obj runtime.Object, opts ...UpdateOptionFunc) error { _, ok := obj.(*unstructured.Unstructured) if ok { return c.unstructuredClient.Update(ctx, obj) diff --git a/pkg/client/interfaces.go b/pkg/client/interfaces.go index 25b65ac7a4..1f075a7ec0 100644 --- a/pkg/client/interfaces.go +++ b/pkg/client/interfaces.go @@ -64,7 +64,7 @@ type Writer interface { // Update updates the given obj in the Kubernetes cluster. obj must be a // struct pointer so that obj can be updated with the content returned by the Server. - Update(ctx context.Context, obj runtime.Object) error + Update(ctx context.Context, obj runtime.Object, opts ...UpdateOptionFunc) error } // StatusClient knows how to create a client which can update status subresource @@ -149,9 +149,9 @@ func (o *CreateOptions) ApplyOptions(optFuncs []CreateOptionFunc) *CreateOptions // https://github.com/tmrts/go-patterns/blob/master/idiom/functional-options.md. type CreateOptionFunc func(*CreateOptions) -// DryRunAll is a functional option that sets the DryRun +// CreateDryRunAll is a functional option that sets the DryRun // field of a CreateOptions struct to metav1.DryRunAll. -func DryRunAll() CreateOptionFunc { +func CreateDryRunAll() CreateOptionFunc { return func(opts *CreateOptions) { opts.DryRun = []string{metav1.DryRunAll} } @@ -377,3 +377,54 @@ func UseListOptions(newOpts *ListOptions) ListOptionFunc { *opts = *newOpts } } + +// UpdateOptions contains options for create requests. It's generally a subset +// of metav1.UpdateOptions. +type UpdateOptions struct { + // When present, indicates that modifications should not be + // persisted. An invalid or unrecognized dryRun directive will + // result in an error response and no further processing of the + // request. Valid values are: + // - All: all dry run stages will be processed + DryRun []string + + // Raw represents raw UpdateOptions, as passed to the API server. + Raw *metav1.UpdateOptions +} + +// AsUpdateOptions returns these options as a metav1.UpdateOptions. +// This may mutate the Raw field. +func (o *UpdateOptions) AsUpdateOptions() *metav1.UpdateOptions { + + if o == nil { + return &metav1.UpdateOptions{} + } + if o.Raw == nil { + o.Raw = &metav1.UpdateOptions{} + } + + o.Raw.DryRun = o.DryRun + return o.Raw +} + +// ApplyOptions executes the given UpdateOptionFuncs and returns the mutated +// UpdateOptions. +func (o *UpdateOptions) ApplyOptions(optFuncs []UpdateOptionFunc) *UpdateOptions { + for _, optFunc := range optFuncs { + optFunc(o) + } + return o +} + +// UpdateOptionFunc is a function that mutates a UpdateOptions struct. It implements +// the functional options pattern. See +// https://github.com/tmrts/go-patterns/blob/master/idiom/functional-options.md. +type UpdateOptionFunc func(*UpdateOptions) + +// UpdateDryRunAll is a functional option that sets the DryRun +// field of a UpdateOptions struct to metav1.DryRunAll. +func UpdateDryRunAll() UpdateOptionFunc { + return func(opts *UpdateOptions) { + opts.DryRun = []string{metav1.DryRunAll} + } +} From b1c26631533bdee4f6399c33e2175ccf09c718a3 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Tue, 26 Feb 2019 14:38:36 +0100 Subject: [PATCH 3/6] Implement options in clients --- pkg/client/client.go | 8 ++++---- pkg/client/typed_client.go | 12 ++++++++++-- pkg/client/unstructured_client.go | 12 ++++++++---- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index df77cf38bd..88c3350c17 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -107,18 +107,18 @@ type client struct { func (c *client) Create(ctx context.Context, obj runtime.Object, opts ...CreateOptionFunc) error { _, ok := obj.(*unstructured.Unstructured) if ok { - return c.unstructuredClient.Create(ctx, obj) + return c.unstructuredClient.Create(ctx, obj, opts...) } - return c.typedClient.Create(ctx, obj) + return c.typedClient.Create(ctx, obj, opts...) } // Update implements client.Client func (c *client) Update(ctx context.Context, obj runtime.Object, opts ...UpdateOptionFunc) error { _, ok := obj.(*unstructured.Unstructured) if ok { - return c.unstructuredClient.Update(ctx, obj) + return c.unstructuredClient.Update(ctx, obj, opts...) } - return c.typedClient.Update(ctx, obj) + return c.typedClient.Update(ctx, obj, opts...) } // Delete implements client.Client diff --git a/pkg/client/typed_client.go b/pkg/client/typed_client.go index 47a90e37d9..82d6cc12ef 100644 --- a/pkg/client/typed_client.go +++ b/pkg/client/typed_client.go @@ -30,31 +30,39 @@ type typedClient struct { } // Create implements client.Client -func (c *typedClient) Create(ctx context.Context, obj runtime.Object) error { +func (c *typedClient) Create(ctx context.Context, obj runtime.Object, opts ...CreateOptionFunc) error { o, err := c.cache.getObjMeta(obj) if err != nil { return err } + + createOpts := &CreateOptions{} + createOpts.ApplyOptions(opts) return o.Post(). NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). Resource(o.resource()). Body(obj). + VersionedParams(createOpts.AsCreateOptions(), c.paramCodec). Context(ctx). Do(). Into(obj) } // Update implements client.Client -func (c *typedClient) Update(ctx context.Context, obj runtime.Object) error { +func (c *typedClient) Update(ctx context.Context, obj runtime.Object, opts ...UpdateOptionFunc) error { o, err := c.cache.getObjMeta(obj) if err != nil { return err } + + updateOpts := &UpdateOptions{} + updateOpts.ApplyOptions(opts) return o.Put(). NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). Resource(o.resource()). Name(o.GetName()). Body(obj). + VersionedParams(updateOpts.AsUpdateOptions(), c.paramCodec). Context(ctx). Do(). Into(obj) diff --git a/pkg/client/unstructured_client.go b/pkg/client/unstructured_client.go index 4e616b8946..c7a199586e 100644 --- a/pkg/client/unstructured_client.go +++ b/pkg/client/unstructured_client.go @@ -37,16 +37,18 @@ type unstructuredClient struct { } // Create implements client.Client -func (uc *unstructuredClient) Create(_ context.Context, obj runtime.Object) error { +func (uc *unstructuredClient) Create(_ context.Context, obj runtime.Object, opts ...CreateOptionFunc) error { u, ok := obj.(*unstructured.Unstructured) if !ok { return fmt.Errorf("unstructured client did not understand object: %T", obj) } + createOpts := CreateOptions{} + createOpts.ApplyOptions(opts) r, err := uc.getResourceInterface(u.GroupVersionKind(), u.GetNamespace()) if err != nil { return err } - i, err := r.Create(u, metav1.CreateOptions{}) + i, err := r.Create(u, *createOpts.AsCreateOptions()) if err != nil { return err } @@ -55,16 +57,18 @@ func (uc *unstructuredClient) Create(_ context.Context, obj runtime.Object) erro } // Update implements client.Client -func (uc *unstructuredClient) Update(_ context.Context, obj runtime.Object) error { +func (uc *unstructuredClient) Update(_ context.Context, obj runtime.Object, opts ...UpdateOptionFunc) error { u, ok := obj.(*unstructured.Unstructured) if !ok { return fmt.Errorf("unstructured client did not understand object: %T", obj) } + updateOpts := UpdateOptions{} + updateOpts.ApplyOptions(opts) r, err := uc.getResourceInterface(u.GroupVersionKind(), u.GetNamespace()) if err != nil { return err } - i, err := r.Update(u, metav1.UpdateOptions{}) + i, err := r.Update(u, *updateOpts.AsUpdateOptions()) if err != nil { return err } From 164ec470ea588dca683fb81d12fe02db1ad249da Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Tue, 26 Feb 2019 15:00:08 +0100 Subject: [PATCH 4/6] Add tests for DryRun on Create/Update --- pkg/client/client_test.go | 78 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index c8d23e1915..371c7f2218 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -255,6 +255,25 @@ var _ = Describe("Client", func() { // TODO(seans3): implement these // Example: ListOptions }) + + Context("with the DryRun option", func() { + It("should not create a new object", func(done Done) { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + By("creating the object (with DryRun)") + err = cl.Create(context.TODO(), dep, client.CreateDryRunAll()) + Expect(err).NotTo(HaveOccurred()) + + actual, err := clientset.AppsV1().Deployments(ns).Get(dep.Name, metav1.GetOptions{}) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(BeTrue()) + Expect(actual).To(Equal(&appsv1.Deployment{})) + + close(done) + }) + }) }) Context("with unstructured objects", func() { @@ -367,6 +386,33 @@ var _ = Describe("Client", func() { }) + Context("with the DryRun option", func() { + It("should not create a new object from a go struct", func(done Done) { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + By("encoding the deployment as unstructured") + u := &unstructured.Unstructured{} + scheme.Convert(dep, u, nil) + u.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "apps", + Kind: "Deployment", + Version: "v1", + }) + + By("creating the object") + err = cl.Create(context.TODO(), u, client.CreateDryRunAll()) + Expect(err).NotTo(HaveOccurred()) + + actual, err := clientset.AppsV1().Deployments(ns).Get(dep.Name, metav1.GetOptions{}) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(BeTrue()) + Expect(actual).To(Equal(&appsv1.Deployment{})) + + close(done) + }) + }) }) Describe("Update", func() { @@ -1722,6 +1768,22 @@ var _ = Describe("Client", func() { }) }) + Describe("CreateOptions", func() { + It("should allow setting DryRun to 'all'", func() { + co := &client.CreateOptions{} + client.CreateDryRunAll()(co) + all := []string{metav1.DryRunAll} + Expect(co.AsCreateOptions().DryRun).To(Equal(all)) + }) + + It("should produce empty metav1.CreateOptions if nil", func() { + var co *client.CreateOptions + Expect(co.AsCreateOptions()).To(Equal(&metav1.CreateOptions{})) + co = &client.CreateOptions{} + Expect(co.AsCreateOptions()).To(Equal(&metav1.CreateOptions{})) + }) + }) + Describe("DeleteOptions", func() { It("should allow setting GracePeriodSeconds", func() { do := &client.DeleteOptions{} @@ -1846,6 +1908,22 @@ var _ = Describe("Client", func() { Expect(lo.Namespace).To(Equal("test")) }) }) + + Describe("UpdateOptions", func() { + It("should allow setting DryRun to 'all'", func() { + uo := &client.UpdateOptions{} + client.UpdateDryRunAll()(uo) + all := []string{metav1.DryRunAll} + Expect(uo.AsUpdateOptions().DryRun).To(Equal(all)) + }) + + It("should produce empty metav1.UpdateOptions if nil", func() { + var co *client.UpdateOptions + Expect(co.AsUpdateOptions()).To(Equal(&metav1.UpdateOptions{})) + co = &client.UpdateOptions{} + Expect(co.AsUpdateOptions()).To(Equal(&metav1.UpdateOptions{})) + }) + }) }) var _ = Describe("DelegatingReader", func() { From 134e3d31969c2261da7052d3f84a8af473b3a982 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Fri, 1 Mar 2019 15:53:20 +0100 Subject: [PATCH 5/6] Add Create/Update options to fake client --- pkg/client/fake/client.go | 23 +++++++++++++-- pkg/client/fake/client_test.go | 51 ++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go index f59b31a787..905f34cbb3 100644 --- a/pkg/client/fake/client.go +++ b/pkg/client/fake/client.go @@ -24,6 +24,7 @@ import ( "strings" "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes/scheme" @@ -138,7 +139,16 @@ func (c *fakeClient) List(ctx context.Context, obj runtime.Object, opts ...clien return nil } -func (c *fakeClient) Create(ctx context.Context, obj runtime.Object) error { +func (c *fakeClient) Create(ctx context.Context, obj runtime.Object, opts ...client.CreateOptionFunc) error { + createOptions := &client.CreateOptions{} + createOptions.ApplyOptions(opts) + + for _, dryRunOpt := range createOptions.DryRun { + if dryRunOpt == metav1.DryRunAll { + return nil + } + } + gvr, err := getGVRFromObject(obj, c.scheme) if err != nil { return err @@ -163,7 +173,16 @@ func (c *fakeClient) Delete(ctx context.Context, obj runtime.Object, opts ...cli return c.tracker.Delete(gvr, accessor.GetNamespace(), accessor.GetName()) } -func (c *fakeClient) Update(ctx context.Context, obj runtime.Object) error { +func (c *fakeClient) Update(ctx context.Context, obj runtime.Object, opts ...client.UpdateOptionFunc) error { + updateOptions := &client.UpdateOptions{} + updateOptions.ApplyOptions(opts) + + for _, dryRunOpt := range updateOptions.DryRun { + if dryRunOpt == metav1.DryRunAll { + return nil + } + } + gvr, err := getGVRFromObject(obj, c.scheme) if err != nil { return err diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go index c61afd805f..c96f996a20 100644 --- a/pkg/client/fake/client_test.go +++ b/pkg/client/fake/client_test.go @@ -22,6 +22,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -154,6 +155,56 @@ var _ = Describe("Fake client", func() { Expect(list.Items).To(HaveLen(1)) Expect(list.Items).To(ConsistOf(*dep2)) }) + + Context("with the DryRun option", func() { + It("should not create a new object", func() { + By("Creating a new configmap with DryRun") + newcm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-test-cm", + Namespace: "ns2", + }, + } + err := cl.Create(nil, newcm, client.CreateDryRunAll()) + Expect(err).To(BeNil()) + + By("Getting the new configmap") + namespacedName := types.NamespacedName{ + Name: "new-test-cm", + Namespace: "ns2", + } + obj := &corev1.ConfigMap{} + err = cl.Get(nil, namespacedName, obj) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(BeTrue()) + Expect(obj).NotTo(Equal(newcm)) + }) + + It("should not Update the object", func() { + By("Updating a new configmap with DryRun") + newcm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cm", + Namespace: "ns2", + }, + Data: map[string]string{ + "test-key": "new-value", + }, + } + err := cl.Update(nil, newcm, client.UpdateDryRunAll()) + Expect(err).To(BeNil()) + + By("Getting the new configmap") + namespacedName := types.NamespacedName{ + Name: "test-cm", + Namespace: "ns2", + } + obj := &corev1.ConfigMap{} + err = cl.Get(nil, namespacedName, obj) + Expect(err).To(BeNil()) + Expect(obj).To(Equal(cm)) + }) + }) } Context("with default scheme.Scheme", func() { From f7bb59134c880df9706f3029f6388d31071f642a Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 13 Mar 2019 17:25:48 +0000 Subject: [PATCH 6/6] Bump Kubernetes version in CI tests --- hack/check-everything.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/check-everything.sh b/hack/check-everything.sh index 50d0c50d08..7563501e71 100755 --- a/hack/check-everything.sh +++ b/hack/check-everything.sh @@ -19,7 +19,7 @@ set -e hack_dir=$(dirname ${BASH_SOURCE}) source ${hack_dir}/common.sh -k8s_version=1.10.1 +k8s_version=1.13.1 goarch=amd64 goos="unknown"