Merge pull request #536 from DirectXMan12/feature/functional-options-redux

⚠️ Functional Options Redux

Author: k8s-ci-robot

pkg/cache/informer_cache.go

@@ -58,7 +58,7 @@ func (ip *informerCache) Get(ctx context.Context, key client.ObjectKey, out runt
 }
 
 // List implements Reader
-func (ip *informerCache) List(ctx context.Context, out runtime.Object, opts ...client.ListOptionFunc) error {
+func (ip *informerCache) List(ctx context.Context, out runtime.Object, opts ...client.ListOption) error {
 	gvk, err := apiutil.GVKForObject(out, ip.Scheme)
 	if err != nil {
 		return err

pkg/cache/informertest/fake_cache.go

@@ -136,6 +136,6 @@ func (c *FakeInformers) Get(ctx context.Context, key client.ObjectKey, obj runti
 }
 
 // List implements Cache
-func (c *FakeInformers) List(ctx context.Context, list runtime.Object, opts ...client.ListOptionFunc) error {
+func (c *FakeInformers) List(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
 	return nil
 }

pkg/cache/internal/cache_reader.go

@@ -88,7 +88,7 @@ func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out runtime.O
 }
 
 // List lists items out of the indexer and writes them to out
-func (c *CacheReader) List(_ context.Context, out runtime.Object, opts ...client.ListOptionFunc) error {
+func (c *CacheReader) List(_ context.Context, out runtime.Object, opts ...client.ListOption) error {
 	var objs []interface{}
 	var err error
 

pkg/cache/multi_namespace_cache.go

@@ -133,7 +133,7 @@ func (c *multiNamespaceCache) Get(ctx context.Context, key client.ObjectKey, obj
 }
 
 // List multi namespace cache will get all the objects in the namespaces that the cache is watching if asked for all namespaces.
-func (c *multiNamespaceCache) List(ctx context.Context, list runtime.Object, opts ...client.ListOptionFunc) error {
+func (c *multiNamespaceCache) List(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
 	listOpts := client.ListOptions{}
 	listOpts.ApplyOptions(opts)
 	if listOpts.Namespace != corev1.NamespaceAll {

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, opts ...CreateOptionFunc) error {
+func (c *client) Create(ctx context.Context, obj runtime.Object, opts ...CreateOption) error {
 	_, ok := obj.(*unstructured.Unstructured)
 	if ok {
 		return c.unstructuredClient.Create(ctx, obj, opts...)
@@ -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, opts ...UpdateOptionFunc) error {
+func (c *client) Update(ctx context.Context, obj runtime.Object, opts ...UpdateOption) error {
 	_, ok := obj.(*unstructured.Unstructured)
 	if ok {
 		return c.unstructuredClient.Update(ctx, obj, opts...)
@@ -122,7 +122,7 @@ func (c *client) Update(ctx context.Context, obj runtime.Object, opts ...UpdateO
 }
 
 // Delete implements client.Client
-func (c *client) Delete(ctx context.Context, obj runtime.Object, opts ...DeleteOptionFunc) error {
+func (c *client) Delete(ctx context.Context, obj runtime.Object, opts ...DeleteOption) error {
 	_, ok := obj.(*unstructured.Unstructured)
 	if ok {
 		return c.unstructuredClient.Delete(ctx, obj, opts...)
@@ -131,7 +131,7 @@ func (c *client) Delete(ctx context.Context, obj runtime.Object, opts ...DeleteO
 }
 
 // Patch implements client.Client
-func (c *client) Patch(ctx context.Context, obj runtime.Object, patch Patch, opts ...PatchOptionFunc) error {
+func (c *client) Patch(ctx context.Context, obj runtime.Object, patch Patch, opts ...PatchOption) error {
 	_, ok := obj.(*unstructured.Unstructured)
 	if ok {
 		return c.unstructuredClient.Patch(ctx, obj, patch, opts...)
@@ -149,7 +149,7 @@ func (c *client) Get(ctx context.Context, key ObjectKey, obj runtime.Object) err
 }
 
 // List implements client.Client
-func (c *client) List(ctx context.Context, obj runtime.Object, opts ...ListOptionFunc) error {
+func (c *client) List(ctx context.Context, obj runtime.Object, opts ...ListOption) error {
 	_, ok := obj.(*unstructured.UnstructuredList)
 	if ok {
 		return c.unstructuredClient.List(ctx, obj, opts...)
@@ -171,7 +171,7 @@ type statusWriter struct {
 var _ StatusWriter = &statusWriter{}
 
 // Update implements client.StatusWriter
-func (sw *statusWriter) Update(ctx context.Context, obj runtime.Object, opts ...UpdateOptionFunc) error {
+func (sw *statusWriter) Update(ctx context.Context, obj runtime.Object, opts ...UpdateOption) error {
 	_, ok := obj.(*unstructured.Unstructured)
 	if ok {
 		return sw.client.unstructuredClient.UpdateStatus(ctx, obj, opts...)
@@ -180,7 +180,7 @@ func (sw *statusWriter) Update(ctx context.Context, obj runtime.Object, opts ...
 }
 
 // Patch implements client.Client
-func (sw *statusWriter) Patch(ctx context.Context, obj runtime.Object, patch Patch, opts ...PatchOptionFunc) error {
+func (sw *statusWriter) Patch(ctx context.Context, obj runtime.Object, patch Patch, opts ...PatchOption) error {
 	_, ok := obj.(*unstructured.Unstructured)
 	if ok {
 		return sw.client.unstructuredClient.PatchStatus(ctx, obj, patch, opts...)

pkg/client/client_test.go

@@ -2000,11 +2000,17 @@ var _ = Describe("Client", func() {
 	Describe("CreateOptions", func() {
 		It("should allow setting DryRun to 'all'", func() {
 			co := &client.CreateOptions{}
-			client.CreateDryRunAll(co)
+			client.DryRunAll.ApplyToCreate(co)
 			all := []string{metav1.DryRunAll}
 			Expect(co.AsCreateOptions().DryRun).To(Equal(all))
 		})
 
+		It("should allow setting the field manager", func() {
+			po := &client.CreateOptions{}
+			client.FieldOwner("some-owner").ApplyToCreate(po)
+			Expect(po.AsCreateOptions().FieldManager).To(Equal("some-owner"))
+		})
+
 		It("should produce empty metav1.CreateOptions if nil", func() {
 			var co *client.CreateOptions
 			Expect(co.AsCreateOptions()).To(Equal(&metav1.CreateOptions{}))
@@ -2016,26 +2022,33 @@ var _ = Describe("Client", func() {
 	Describe("DeleteOptions", func() {
 		It("should allow setting GracePeriodSeconds", func() {
 			do := &client.DeleteOptions{}
-			client.GracePeriodSeconds(1)(do)
+			client.GracePeriodSeconds(1).ApplyToDelete(do)
 			gp := int64(1)
 			Expect(do.AsDeleteOptions().GracePeriodSeconds).To(Equal(&gp))
 		})
 
 		It("should allow setting Precondition", func() {
 			do := &client.DeleteOptions{}
 			pc := metav1.NewUIDPreconditions("uid")
-			client.Preconditions(pc)(do)
+			client.Preconditions(*pc).ApplyToDelete(do)
 			Expect(do.AsDeleteOptions().Preconditions).To(Equal(pc))
 			Expect(do.Preconditions).To(Equal(pc))
 		})
 
 		It("should allow setting PropagationPolicy", func() {
 			do := &client.DeleteOptions{}
-			client.PropagationPolicy(metav1.DeletePropagationForeground)(do)
+			client.PropagationPolicy(metav1.DeletePropagationForeground).ApplyToDelete(do)
 			dp := metav1.DeletePropagationForeground
 			Expect(do.AsDeleteOptions().PropagationPolicy).To(Equal(&dp))
 		})
 
+		It("should allow setting DryRun", func() {
+			do := &client.DeleteOptions{}
+			client.DryRunAll.ApplyToDelete(do)
+			all := []string{metav1.DryRunAll}
+			Expect(do.AsDeleteOptions().DryRun).To(Equal(all))
+		})
+
 		It("should produce empty metav1.DeleteOptions if nil", func() {
 			var do *client.DeleteOptions
 			Expect(do.AsDeleteOptions()).To(Equal(&metav1.DeleteOptions{}))
@@ -2048,9 +2061,9 @@ var _ = Describe("Client", func() {
 			pc := metav1.NewUIDPreconditions("uid")
 			dp := metav1.DeletePropagationForeground
 			do := &client.DeleteOptions{}
-			do.ApplyOptions([]client.DeleteOptionFunc{
+			do.ApplyOptions([]client.DeleteOption{
 				client.GracePeriodSeconds(gp),
-				client.Preconditions(pc),
+				client.Preconditions(*pc),
 				client.PropagationPolicy(dp),
 			})
 			Expect(do.GracePeriodSeconds).To(Equal(&gp))
@@ -2060,79 +2073,35 @@ var _ = Describe("Client", func() {
 	})
 
 	Describe("ListOptions", func() {
-		It("should be able to set a LabelSelector", func() {
-			lo := &client.ListOptions{}
-			err := lo.SetLabelSelector("foo=bar")
-			Expect(err).NotTo(HaveOccurred())
-			Expect(lo.LabelSelector.String()).To(Equal("foo=bar"))
-		})
-
-		It("should be able to set a FieldSelector", func() {
-			lo := &client.ListOptions{}
-			err := lo.SetFieldSelector("field1=bar")
-			Expect(err).NotTo(HaveOccurred())
-			Expect(lo.FieldSelector.String()).To(Equal("field1=bar"))
-		})
-
-		It("should be converted to metav1.ListOptions", func() {
-			lo := &client.ListOptions{}
-			labels := map[string]string{"foo": "bar"}
-			mlo := lo.MatchingLabels(labels).
-				MatchingField("field1", "bar").
-				InNamespace("test-namespace").
-				AsListOptions()
+		It("should be convertable to metav1.ListOptions", func() {
+			lo := (&client.ListOptions{}).ApplyOptions([]client.ListOption{
+				client.MatchingField("field1", "bar"),
+				client.InNamespace("test-namespace"),
+				client.MatchingLabels{"foo": "bar"},
+			})
+			mlo := lo.AsListOptions()
 			Expect(mlo).NotTo(BeNil())
 			Expect(mlo.LabelSelector).To(Equal("foo=bar"))
 			Expect(mlo.FieldSelector).To(Equal("field1=bar"))
 		})
 
-		It("should be able to set MatchingLabels", func() {
+		It("should be populated by MatchingLabels", func() {
 			lo := &client.ListOptions{}
-			Expect(lo.LabelSelector).To(BeNil())
-			labels := map[string]string{"foo": "bar"}
-			lo = lo.MatchingLabels(labels)
-			Expect(lo.LabelSelector.String()).To(Equal("foo=bar"))
-		})
-
-		It("should be able to set MatchingField", func() {
-			lo := &client.ListOptions{}
-			Expect(lo.FieldSelector).To(BeNil())
-			lo = lo.MatchingField("field1", "bar")
-			Expect(lo.FieldSelector.String()).To(Equal("field1=bar"))
-		})
-
-		It("should be able to set InNamespace", func() {
-			lo := &client.ListOptions{}
-			lo = lo.InNamespace("test-namespace")
-			Expect(lo.Namespace).To(Equal("test-namespace"))
-		})
-
-		It("should be created from MatchingLabels", func() {
-			labels := map[string]string{"foo": "bar"}
-			lo := &client.ListOptions{}
-			client.MatchingLabels(labels)(lo)
+			client.MatchingLabels{"foo": "bar"}.ApplyToList(lo)
 			Expect(lo).NotTo(BeNil())
 			Expect(lo.LabelSelector.String()).To(Equal("foo=bar"))
 		})
 
-		It("should be created from MatchingField", func() {
+		It("should be populated by MatchingField", func() {
 			lo := &client.ListOptions{}
-			client.MatchingField("field1", "bar")(lo)
+			client.MatchingField("field1", "bar").ApplyToList(lo)
 			Expect(lo).NotTo(BeNil())
 			Expect(lo.FieldSelector.String()).To(Equal("field1=bar"))
 		})
 
-		It("should be created from InNamespace", func() {
+		It("should be populated by InNamespace", func() {
 			lo := &client.ListOptions{}
-			client.InNamespace("test")(lo)
-			Expect(lo).NotTo(BeNil())
-			Expect(lo.Namespace).To(Equal("test"))
-		})
-
-		It("should allow pre-built ListOptions", func() {
-			lo := &client.ListOptions{}
-			newLo := &client.ListOptions{}
-			client.UseListOptions(newLo.InNamespace("test"))(lo)
+			client.InNamespace("test").ApplyToList(lo)
 			Expect(lo).NotTo(BeNil())
 			Expect(lo.Namespace).To(Equal("test"))
 		})
@@ -2141,11 +2110,17 @@ var _ = Describe("Client", func() {
 	Describe("UpdateOptions", func() {
 		It("should allow setting DryRun to 'all'", func() {
 			uo := &client.UpdateOptions{}
-			client.UpdateDryRunAll(uo)
+			client.DryRunAll.ApplyToUpdate(uo)
 			all := []string{metav1.DryRunAll}
 			Expect(uo.AsUpdateOptions().DryRun).To(Equal(all))
 		})
 
+		It("should allow setting the field manager", func() {
+			po := &client.UpdateOptions{}
+			client.FieldOwner("some-owner").ApplyToUpdate(po)
+			Expect(po.AsUpdateOptions().FieldManager).To(Equal("some-owner"))
+		})
+
 		It("should produce empty metav1.UpdateOptions if nil", func() {
 			var co *client.UpdateOptions
 			Expect(co.AsUpdateOptions()).To(Equal(&metav1.UpdateOptions{}))
@@ -2157,22 +2132,22 @@ var _ = Describe("Client", func() {
 	Describe("PatchOptions", func() {
 		It("should allow setting DryRun to 'all'", func() {
 			po := &client.PatchOptions{}
-			client.PatchDryRunAll(po)
+			client.DryRunAll.ApplyToPatch(po)
 			all := []string{metav1.DryRunAll}
 			Expect(po.AsPatchOptions().DryRun).To(Equal(all))
 		})
 
 		It("should allow setting Force to 'true'", func() {
 			po := &client.PatchOptions{}
-			client.ForceOwnership(po)
+			client.ForceOwnership.ApplyToPatch(po)
 			mpo := po.AsPatchOptions()
 			Expect(mpo.Force).NotTo(BeNil())
 			Expect(*mpo.Force).To(BeTrue())
 		})
 
 		It("should allow setting the field manager", func() {
 			po := &client.PatchOptions{}
-			client.FieldOwner("some-owner")(po)
+			client.FieldOwner("some-owner").ApplyToPatch(po)
 			Expect(po.AsPatchOptions().FieldManager).To(Equal("some-owner"))
 		})
 
@@ -2320,7 +2295,7 @@ func (f *fakeReader) Get(ctx context.Context, key client.ObjectKey, obj runtime.
 	return nil
 }
 
-func (f *fakeReader) List(ctx context.Context, list runtime.Object, opts ...client.ListOptionFunc) error {
+func (f *fakeReader) List(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
 	f.Called = f.Called + 1
 	return nil
 }

pkg/client/doc.go

@@ -34,7 +34,7 @@ limitations under the License.
 // Many client operations in Kubernetes support options.  These options are
 // represented as variadic arguments at the end of a given method call.
 // For instance, to use a label selector on list, you can call
-//  err := someReader.List(context.Background(), &podList, client.MatchingLabels(someLabelMap))
+//  err := someReader.List(context.Background(), &podList, client.MatchingLabels{"somelabel": "someval"})
 //
 // Indexing
 //

pkg/client/fake/client.go

@@ -84,7 +84,7 @@ func (c *fakeClient) Get(ctx context.Context, key client.ObjectKey, obj runtime.
 	return err
 }
 
-func (c *fakeClient) List(ctx context.Context, obj runtime.Object, opts ...client.ListOptionFunc) error {
+func (c *fakeClient) List(ctx context.Context, obj runtime.Object, opts ...client.ListOption) error {
 	gvk, err := apiutil.GVKForObject(obj, c.scheme)
 	if err != nil {
 		return err
@@ -131,7 +131,7 @@ func (c *fakeClient) List(ctx context.Context, obj runtime.Object, opts ...clien
 	return nil
 }
 
-func (c *fakeClient) Create(ctx context.Context, obj runtime.Object, opts ...client.CreateOptionFunc) error {
+func (c *fakeClient) Create(ctx context.Context, obj runtime.Object, opts ...client.CreateOption) error {
 	createOptions := &client.CreateOptions{}
 	createOptions.ApplyOptions(opts)
 
@@ -152,7 +152,7 @@ func (c *fakeClient) Create(ctx context.Context, obj runtime.Object, opts ...cli
 	return c.tracker.Create(gvr, obj, accessor.GetNamespace())
 }
 
-func (c *fakeClient) Delete(ctx context.Context, obj runtime.Object, opts ...client.DeleteOptionFunc) error {
+func (c *fakeClient) Delete(ctx context.Context, obj runtime.Object, opts ...client.DeleteOption) error {
 	gvr, err := getGVRFromObject(obj, c.scheme)
 	if err != nil {
 		return err
@@ -165,7 +165,7 @@ 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, opts ...client.UpdateOptionFunc) error {
+func (c *fakeClient) Update(ctx context.Context, obj runtime.Object, opts ...client.UpdateOption) error {
 	updateOptions := &client.UpdateOptions{}
 	updateOptions.ApplyOptions(opts)
 
@@ -186,7 +186,7 @@ func (c *fakeClient) Update(ctx context.Context, obj runtime.Object, opts ...cli
 	return c.tracker.Update(gvr, obj, accessor.GetNamespace())
 }
 
-func (c *fakeClient) Patch(ctx context.Context, obj runtime.Object, patch client.Patch, opts ...client.PatchOptionFunc) error {
+func (c *fakeClient) Patch(ctx context.Context, obj runtime.Object, patch client.Patch, opts ...client.PatchOption) error {
 	patchOptions := &client.PatchOptions{}
 	patchOptions.ApplyOptions(opts)
 
@@ -244,13 +244,13 @@ type fakeStatusWriter struct {
 	client *fakeClient
 }
 
-func (sw *fakeStatusWriter) Update(ctx context.Context, obj runtime.Object, opts ...client.UpdateOptionFunc) error {
+func (sw *fakeStatusWriter) Update(ctx context.Context, obj runtime.Object, opts ...client.UpdateOption) error {
 	// TODO(droot): This results in full update of the obj (spec + status). Need
 	// a way to update status field only.
 	return sw.client.Update(ctx, obj, opts...)
 }
 
-func (sw *fakeStatusWriter) Patch(ctx context.Context, obj runtime.Object, patch client.Patch, opts ...client.PatchOptionFunc) error {
+func (sw *fakeStatusWriter) Patch(ctx context.Context, obj runtime.Object, patch client.Patch, opts ...client.PatchOption) error {
 	// TODO(droot): This results in full update of the obj (spec + status). Need
 	// a way to update status field only.
 	return sw.client.Patch(ctx, obj, patch, opts...)