Skip to content

Commit 5734523

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#294 from droot/list-options
⚠️ Convert client.List to use functional options
2 parents 0dd6edb + 3285ad1 commit 5734523

File tree

15 files changed

+159
-164
lines changed

15 files changed

+159
-164
lines changed

‎pkg/builder/example_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ func (a *ReplicaSetReconciler) Reconcile(req reconcile.Request) (reconcile.Resul
8686

8787
// List the Pods matching the PodTemplate Labels
8888
pods := &corev1.PodList{}
89-
err = a.List(context.TODO(), client.InNamespace(req.Namespace).MatchingLabels(rs.Spec.Template.Labels), pods)
89+
err = a.List(context.TODO(), pods, client.InNamespace(req.Namespace),
90+
client.MatchingLabels(rs.Spec.Template.Labels))
9091
if err != nil {
9192
return reconcile.Result{}, err
9293
}

‎pkg/cache/cache_test.go

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ var _ = Describe("Informer Cache", func() {
124124
It("should be able to list objects that haven't been watched previously", func() {
125125
By("listing all services in the cluster")
126126
listObj := &kcorev1.ServiceList{}
127-
Expect(informerCache.List(context.Background(), nil, listObj)).To(Succeed())
127+
Expect(informerCache.List(context.Background(), listObj)).To(Succeed())
128128

129129
By("verifying that the returned list contains the Kubernetes service")
130130
// NB: kubernetes default service is automatically created in testenv.
@@ -154,10 +154,9 @@ var _ = Describe("Informer Cache", func() {
154154
By("listing pods with a particular label")
155155
// NB: each pod has a "test-label": <pod-name>
156156
out := kcorev1.PodList{}
157-
lo := &client.ListOptions{}
158-
lo.InNamespace(testNamespaceTwo)
159-
lo.MatchingLabels(map[string]string{"test-label": "test-pod-2"})
160-
Expect(informerCache.List(context.Background(), lo, &out)).To(Succeed())
157+
Expect(informerCache.List(context.Background(), &out,
158+
client.InNamespace(testNamespaceTwo),
159+
client.MatchingLabels(map[string]string{"test-label": "test-pod-2"}))).To(Succeed())
161160

162161
By("verifying the returned pods have the correct label")
163162
Expect(out.Items).NotTo(BeEmpty())
@@ -174,9 +173,7 @@ var _ = Describe("Informer Cache", func() {
174173
// NB: each pod has a "test-label": <pod-name>
175174
out := kcorev1.PodList{}
176175
labels := map[string]string{"test-label": "test-pod-2"}
177-
lo := &client.ListOptions{}
178-
lo.MatchingLabels(labels)
179-
Expect(informerCache.List(context.Background(), lo, &out)).To(Succeed())
176+
Expect(informerCache.List(context.Background(), &out, client.MatchingLabels(labels))).To(Succeed())
180177

181178
By("verifying multiple pods with the same label in different namespaces are returned")
182179
Expect(out.Items).NotTo(BeEmpty())
@@ -191,9 +188,8 @@ var _ = Describe("Informer Cache", func() {
191188
It("should be able to list objects by namespace", func() {
192189
By("listing pods in test-namespace-1")
193190
listObj := &kcorev1.PodList{}
194-
lo := &client.ListOptions{}
195-
lo.InNamespace(testNamespaceOne)
196-
Expect(informerCache.List(context.Background(), lo, listObj)).To(Succeed())
191+
Expect(informerCache.List(context.Background(), listObj,
192+
client.InNamespace(testNamespaceOne))).To(Succeed())
197193

198194
By("verifying that the returned pods are in test-namespace-1")
199195
Expect(listObj.Items).NotTo(BeEmpty())
@@ -216,7 +212,7 @@ var _ = Describe("Informer Cache", func() {
216212

217213
By("listing pods in all namespaces")
218214
out := &kcorev1.PodList{}
219-
Expect(namespacedCache.List(context.Background(), nil, out)).To(Succeed())
215+
Expect(namespacedCache.List(context.Background(), out)).To(Succeed())
220216

221217
By("verifying the returned pod is from the watched namespace")
222218
Expect(out.Items).NotTo(BeEmpty())
@@ -225,7 +221,7 @@ var _ = Describe("Informer Cache", func() {
225221

226222
By("listing all namespaces - should still be able to get a cluster-scoped resource")
227223
namespaceList := &kcorev1.NamespaceList{}
228-
Expect(namespacedCache.List(context.Background(), nil, namespaceList)).To(Succeed())
224+
Expect(namespacedCache.List(context.Background(), namespaceList)).To(Succeed())
229225

230226
By("verifying the namespace list is not empty")
231227
Expect(namespaceList.Items).NotTo(BeEmpty())
@@ -267,7 +263,7 @@ var _ = Describe("Informer Cache", func() {
267263
Version: "v1",
268264
Kind: "ServiceList",
269265
})
270-
err := informerCache.List(context.Background(), nil, listObj)
266+
err := informerCache.List(context.Background(), listObj)
271267
Expect(err).To(Succeed())
272268

273269
By("verifying that the returned list contains the Kubernetes service")
@@ -308,10 +304,9 @@ var _ = Describe("Informer Cache", func() {
308304
Version: "v1",
309305
Kind: "PodList",
310306
})
311-
lo := &client.ListOptions{}
312-
lo.InNamespace(testNamespaceTwo)
313-
lo.MatchingLabels(map[string]string{"test-label": "test-pod-2"})
314-
err := informerCache.List(context.Background(), lo, &out)
307+
err := informerCache.List(context.Background(), &out,
308+
client.InNamespace(testNamespaceTwo),
309+
client.MatchingLabels(map[string]string{"test-label": "test-pod-2"}))
315310
Expect(err).To(Succeed())
316311

317312
By("verifying the returned pods have the correct label")
@@ -334,9 +329,7 @@ var _ = Describe("Informer Cache", func() {
334329
Kind: "PodList",
335330
})
336331
labels := map[string]string{"test-label": "test-pod-2"}
337-
lo := &client.ListOptions{}
338-
lo.MatchingLabels(labels)
339-
err := informerCache.List(context.Background(), lo, &out)
332+
err := informerCache.List(context.Background(), &out, client.MatchingLabels(labels))
340333
Expect(err).To(Succeed())
341334

342335
By("verifying multiple pods with the same label in different namespaces are returned")
@@ -357,9 +350,7 @@ var _ = Describe("Informer Cache", func() {
357350
Version: "v1",
358351
Kind: "PodList",
359352
})
360-
lo := &client.ListOptions{}
361-
lo.InNamespace(testNamespaceOne)
362-
err := informerCache.List(context.Background(), lo, listObj)
353+
err := informerCache.List(context.Background(), listObj, client.InNamespace(testNamespaceOne))
363354
Expect(err).To(Succeed())
364355

365356
By("verifying that the returned pods are in test-namespace-1")
@@ -388,7 +379,7 @@ var _ = Describe("Informer Cache", func() {
388379
Version: "v1",
389380
Kind: "PodList",
390381
})
391-
Expect(namespacedCache.List(context.Background(), nil, out)).To(Succeed())
382+
Expect(namespacedCache.List(context.Background(), out)).To(Succeed())
392383

393384
By("verifying the returned pod is from the watched namespace")
394385
Expect(out.Items).NotTo(BeEmpty())
@@ -402,7 +393,7 @@ var _ = Describe("Informer Cache", func() {
402393
Version: "v1",
403394
Kind: "NamespaceList",
404395
})
405-
Expect(namespacedCache.List(context.Background(), nil, namespaceList)).To(Succeed())
396+
Expect(namespacedCache.List(context.Background(), namespaceList)).To(Succeed())
406397

407398
By("verifying the namespace list is not empty")
408399
Expect(namespaceList.Items).NotTo(BeEmpty())
@@ -552,9 +543,8 @@ var _ = Describe("Informer Cache", func() {
552543

553544
By("listing Pods with restartPolicyOnFailure")
554545
listObj := &kcorev1.PodList{}
555-
lo := &client.ListOptions{}
556-
lo.MatchingField("spec.restartPolicy", "OnFailure")
557-
Expect(informer.List(context.Background(), lo, listObj)).To(Succeed())
546+
Expect(informer.List(context.Background(), listObj,
547+
client.MatchingField("spec.restartPolicy", "OnFailure"))).To(Succeed())
558548

559549
By("verifying that the returned pods have correct restart policy")
560550
Expect(listObj.Items).NotTo(BeEmpty())
@@ -647,9 +637,8 @@ var _ = Describe("Informer Cache", func() {
647637
Version: "v1",
648638
Kind: "PodList",
649639
})
650-
lo := &client.ListOptions{}
651-
lo.MatchingField("spec.restartPolicy", "OnFailure")
652-
err = informer.List(context.Background(), lo, listObj)
640+
err = informer.List(context.Background(), listObj,
641+
client.MatchingField("spec.restartPolicy", "OnFailure"))
653642
Expect(err).To(Succeed())
654643

655644
By("verifying that the returned pods have correct restart policy")

‎pkg/cache/informer_cache.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func (ip *informerCache) Get(ctx context.Context, key client.ObjectKey, out runt
5858
}
5959

6060
// List implements Reader
61-
func (ip *informerCache) List(ctx context.Context, opts *client.ListOptions, out runtime.Object) error {
61+
func (ip *informerCache) List(ctx context.Context, out runtime.Object, opts ...client.ListOptionFunc) error {
6262
gvk, err := apiutil.GVKForObject(out, ip.Scheme)
6363
if err != nil {
6464
return err
@@ -95,7 +95,7 @@ func (ip *informerCache) List(ctx context.Context, opts *client.ListOptions, out
9595
return err
9696
}
9797

98-
return cache.Reader.List(ctx, opts, out)
98+
return cache.Reader.List(ctx, out, opts...)
9999
}
100100

101101
// GetInformerForKind returns the informer for the GroupVersionKind

‎pkg/cache/informertest/fake_cache.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,6 @@ func (c *FakeInformers) Get(ctx context.Context, key client.ObjectKey, obj runti
136136
}
137137

138138
// List implements Cache
139-
func (c *FakeInformers) List(ctx context.Context, opts *client.ListOptions, list runtime.Object) error {
139+
func (c *FakeInformers) List(ctx context.Context, list runtime.Object, opts ...client.ListOptionFunc) error {
140140
return nil
141141
}

‎pkg/cache/internal/cache_reader.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,32 +87,35 @@ func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out runtime.O
8787
}
8888

8989
// List lists items out of the indexer and writes them to out
90-
func (c *CacheReader) List(_ context.Context, opts *client.ListOptions, out runtime.Object) error {
90+
func (c *CacheReader) List(_ context.Context, out runtime.Object, opts ...client.ListOptionFunc) error {
9191
var objs []interface{}
9292
var err error
9393

94-
if opts != nil && opts.FieldSelector != nil {
94+
listOpts := client.ListOptions{}
95+
listOpts.ApplyOptions(opts)
96+
97+
if listOpts.FieldSelector != nil {
9598
// TODO(directxman12): support more complicated field selectors by
9699
// combining multiple indicies, GetIndexers, etc
97-
field, val, requiresExact := requiresExactMatch(opts.FieldSelector)
100+
field, val, requiresExact := requiresExactMatch(listOpts.FieldSelector)
98101
if !requiresExact {
99102
return fmt.Errorf("non-exact field matches are not supported by the cache")
100103
}
101104
// list all objects by the field selector. If this is namespaced and we have one, ask for the
102105
// namespaced index key. Otherwise, ask for the non-namespaced variant by using the fake "all namespaces"
103106
// namespace.
104-
objs, err = c.indexer.ByIndex(FieldIndexName(field), KeyToNamespacedKey(opts.Namespace, val))
105-
} else if opts != nil && opts.Namespace != "" {
106-
objs, err = c.indexer.ByIndex(cache.NamespaceIndex, opts.Namespace)
107+
objs, err = c.indexer.ByIndex(FieldIndexName(field), KeyToNamespacedKey(listOpts.Namespace, val))
108+
} else if listOpts.Namespace != "" {
109+
objs, err = c.indexer.ByIndex(cache.NamespaceIndex, listOpts.Namespace)
107110
} else {
108111
objs = c.indexer.List()
109112
}
110113
if err != nil {
111114
return err
112115
}
113116
var labelSel labels.Selector
114-
if opts != nil && opts.LabelSelector != nil {
115-
labelSel = opts.LabelSelector
117+
if listOpts.LabelSelector != nil {
118+
labelSel = listOpts.LabelSelector
116119
}
117120

118121
outItems, err := c.getListItems(objs, labelSel)

‎pkg/client/client.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,12 @@ func (c *client) Get(ctx context.Context, key ObjectKey, obj runtime.Object) err
131131
}
132132

133133
// List implements client.Client
134-
func (c *client) List(ctx context.Context, opts *ListOptions, obj runtime.Object) error {
134+
func (c *client) List(ctx context.Context, obj runtime.Object, opts ...ListOptionFunc) error {
135135
_, ok := obj.(*unstructured.UnstructuredList)
136136
if ok {
137-
return c.unstructuredClient.List(ctx, opts, obj)
137+
return c.unstructuredClient.List(ctx, obj, opts...)
138138
}
139-
return c.typedClient.List(ctx, opts, obj)
139+
return c.typedClient.List(ctx, obj, opts...)
140140
}
141141

142142
// Status implements client.StatusClient

0 commit comments

Comments
 (0)