Merge pull request #146 from kubernetes-sigs/revert-106-list-options

Revert "Convert client.List to use functional options"

pkg/builder/example_test.go

@@ -77,7 +77,7 @@ func (a *ReplicaSetReconciler) Reconcile(req reconcile.Request) (reconcile.Resul
 
 	// List the Pods matching the PodTemplate Labels
 	pods := &corev1.PodList{}
-	err = a.List(context.TODO(), pods, client.InNamespace(req.Namespace), client.MatchingLabels(rs.Spec.Template.Labels))
+	err = a.List(context.TODO(), client.InNamespace(req.Namespace).MatchingLabels(rs.Spec.Template.Labels), pods)
 	if err != nil {
 		return reconcile.Result{}, err
 	}

pkg/cache/cache_test.go

@@ -113,7 +113,7 @@ var _ = Describe("Informer Cache", func() {
 		It("should be able to list objects that haven't been watched previously", func() {
 			By("listing all services in the cluster")
 			listObj := &kcorev1.ServiceList{}
-			Expect(informerCache.List(context.Background(), listObj)).To(Succeed())
+			Expect(informerCache.List(context.Background(), nil, listObj)).To(Succeed())
 
 			By("verifying that the returned list contains the Kubernetes service")
 			// NB: kubernetes default service is automatically created in testenv.
@@ -143,10 +143,8 @@ var _ = Describe("Informer Cache", func() {
 			By("listing pods with a particular label")
 			// NB: each pod has a "test-label": <pod-name>
 			out := kcorev1.PodList{}
-			Expect(informerCache.List(context.Background(), &out,
-				client.InNamespace(testNamespaceTwo),
-				client.MatchingLabels(map[string]string{"test-label": "test-pod-2"}),
-			)).To(Succeed())
+			Expect(informerCache.List(context.Background(), client.InNamespace(testNamespaceTwo).
+				MatchingLabels(map[string]string{"test-label": "test-pod-2"}), &out)).To(Succeed())
 
 			By("verifying the returned pods have the correct label")
 			Expect(out.Items).NotTo(BeEmpty())
@@ -163,9 +161,8 @@ var _ = Describe("Informer Cache", func() {
 			// NB: each pod has a "test-label": <pod-name>
 			out := kcorev1.PodList{}
 			labels := map[string]string{"test-label": "test-pod-2"}
-			Expect(informerCache.List(context.Background(), &out,
-				client.MatchingLabels(labels),
-			)).To(Succeed())
+			Expect(informerCache.List(context.Background(),
+				client.MatchingLabels(labels), &out)).To(Succeed())
 
 			By("verifying multiple pods with the same label in different namespaces are returned")
 			Expect(out.Items).NotTo(BeEmpty())
@@ -180,9 +177,9 @@ var _ = Describe("Informer Cache", func() {
 		It("should be able to list objects by namespace", func() {
 			By("listing pods in test-namespace-1")
 			listObj := &kcorev1.PodList{}
-			Expect(informerCache.List(context.Background(), listObj,
+			Expect(informerCache.List(context.Background(),
 				client.InNamespace(testNamespaceOne),
-			)).To(Succeed())
+				listObj)).To(Succeed())
 
 			By("verifying that the returned pods are in test-namespace-1")
 			Expect(listObj.Items).NotTo(BeEmpty())
@@ -320,9 +317,9 @@ var _ = Describe("Informer Cache", func() {
 
 			By("listing Pods with restartPolicyOnFailure")
 			listObj := &kcorev1.PodList{}
-			Expect(informer.List(context.Background(), listObj,
+			Expect(informer.List(context.Background(),
 				client.MatchingField("spec.restartPolicy", "OnFailure"),
-			)).To(Succeed())
+				listObj)).To(Succeed())
 
 			By("verifying that the returned pods have correct restart policy")
 			Expect(listObj.Items).NotTo(BeEmpty())

pkg/cache/informer_cache.go

@@ -57,7 +57,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, opts *client.ListOptions, out runtime.Object) error {
 	itemsPtr, err := apimeta.GetItemsPtr(out)
 	if err != nil {
 		return nil
@@ -87,7 +87,7 @@ func (ip *informerCache) List(ctx context.Context, out runtime.Object, opts ...c
 		return err
 	}
 
-	return cache.Reader.List(ctx, out, opts...)
+	return cache.Reader.List(ctx, opts, out)
 }
 
 // GetInformerForKind returns the informer for the GroupVersionKind

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, opts *client.ListOptions, list runtime.Object) error {
 	return nil
 }

pkg/cache/internal/cache_reader.go

@@ -86,35 +86,32 @@ 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(ctx context.Context, out runtime.Object, opts ...client.ListOptionFunc) error {
+func (c *CacheReader) List(ctx context.Context, opts *client.ListOptions, out runtime.Object) error {
 	var objs []interface{}
 	var err error
 
-	listOpts := client.ListOptions{}
-	listOpts.ApplyOptions(opts)
-
-	if listOpts.FieldSelector != nil {
+	if opts != nil && opts.FieldSelector != nil {
 		// TODO(directxman12): support more complicated field selectors by
 		// combining multiple indicies, GetIndexers, etc
-		field, val, requiresExact := requiresExactMatch(listOpts.FieldSelector)
+		field, val, requiresExact := requiresExactMatch(opts.FieldSelector)
 		if !requiresExact {
 			return fmt.Errorf("non-exact field matches are not supported by the cache")
 		}
 		// list all objects by the field selector.  If this is namespaced and we have one, ask for the
 		// namespaced index key.  Otherwise, ask for the non-namespaced variant by using the fake "all namespaces"
 		// namespace.
-		objs, err = c.indexer.ByIndex(FieldIndexName(field), KeyToNamespacedKey(listOpts.Namespace, val))
-	} else if listOpts.Namespace != "" {
-		objs, err = c.indexer.ByIndex(cache.NamespaceIndex, listOpts.Namespace)
+		objs, err = c.indexer.ByIndex(FieldIndexName(field), KeyToNamespacedKey(opts.Namespace, val))
+	} else if opts != nil && opts.Namespace != "" {
+		objs, err = c.indexer.ByIndex(cache.NamespaceIndex, opts.Namespace)
 	} else {
 		objs = c.indexer.List()
 	}
 	if err != nil {
 		return err
 	}
 	var labelSel labels.Selector
-	if listOpts.LabelSelector != nil {
-		labelSel = listOpts.LabelSelector
+	if opts != nil && opts.LabelSelector != nil {
+		labelSel = opts.LabelSelector
 	}
 
 	outItems, err := c.getListItems(objs, labelSel)

pkg/client/client.go

@@ -140,19 +140,20 @@ 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, opts *ListOptions, obj runtime.Object) error {
 	r, err := c.cache.getResource(obj)
 	if err != nil {
 		return err
 	}
-
-	listOpts := ListOptions{}
-	listOpts.ApplyOptions(opts)
+	namespace := ""
+	if opts != nil {
+		namespace = opts.Namespace
+	}
 	return r.Get().
-		NamespaceIfScoped(listOpts.Namespace, r.isNamespaced()).
+		NamespaceIfScoped(namespace, r.isNamespaced()).
 		Resource(r.resource()).
 		Body(obj).
-		VersionedParams(listOpts.AsListOptions(), c.paramCodec).
+		VersionedParams(opts.AsListOptions(), c.paramCodec).
 		Do().
 		Into(obj)
 }