Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

⚠️ Convert client.List to use functional options #294

Merged
merged 1 commit into from
Jan 16, 2019
Merged

⚠️ Convert client.List to use functional options #294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkg/builder/example_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ func (a *ReplicaSetReconciler) Reconcile(req reconcile.Request) (reconcile.Resul

// List the Pods matching the PodTemplate Labels
pods := &corev1.PodList{}
err = a.List(context.TODO(), client.InNamespace(req.Namespace).MatchingLabels(rs.Spec.Template.Labels), pods)
err = a.List(context.TODO(), pods, client.InNamespace(req.Namespace),
client.MatchingLabels(rs.Spec.Template.Labels))
if err != nil {
return reconcile.Result{}, err
}
Expand Down
53 changes: 21 additions & 32 deletions pkg/cache/cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,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(), nil, listObj)).To(Succeed())
Expect(informerCache.List(context.Background(), listObj)).To(Succeed())

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

By("verifying the returned pods have the correct label")
Expect(out.Items).NotTo(BeEmpty())
Expand All @@ -174,9 +173,7 @@ 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"}
lo := &client.ListOptions{}
lo.MatchingLabels(labels)
Expect(informerCache.List(context.Background(), lo, &out)).To(Succeed())
Expect(informerCache.List(context.Background(), &out, client.MatchingLabels(labels))).To(Succeed())

By("verifying multiple pods with the same label in different namespaces are returned")
Expect(out.Items).NotTo(BeEmpty())
Expand All @@ -191,9 +188,8 @@ 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{}
lo := &client.ListOptions{}
lo.InNamespace(testNamespaceOne)
Expect(informerCache.List(context.Background(), lo, listObj)).To(Succeed())
Expect(informerCache.List(context.Background(), listObj,
client.InNamespace(testNamespaceOne))).To(Succeed())

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

By("listing pods in all namespaces")
out := &kcorev1.PodList{}
Expect(namespacedCache.List(context.Background(), nil, out)).To(Succeed())
Expect(namespacedCache.List(context.Background(), out)).To(Succeed())

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

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

By("verifying the namespace list is not empty")
Expect(namespaceList.Items).NotTo(BeEmpty())
Expand Down Expand Up @@ -267,7 +263,7 @@ var _ = Describe("Informer Cache", func() {
Version: "v1",
Kind: "ServiceList",
})
err := informerCache.List(context.Background(), nil, listObj)
err := informerCache.List(context.Background(), listObj)
Expect(err).To(Succeed())

By("verifying that the returned list contains the Kubernetes service")
Expand Down Expand Up @@ -308,10 +304,9 @@ var _ = Describe("Informer Cache", func() {
Version: "v1",
Kind: "PodList",
})
lo := &client.ListOptions{}
lo.InNamespace(testNamespaceTwo)
lo.MatchingLabels(map[string]string{"test-label": "test-pod-2"})
err := informerCache.List(context.Background(), lo, &out)
err := informerCache.List(context.Background(), &out,
client.InNamespace(testNamespaceTwo),
client.MatchingLabels(map[string]string{"test-label": "test-pod-2"}))
Expect(err).To(Succeed())

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

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

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

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

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

By("listing Pods with restartPolicyOnFailure")
listObj := &kcorev1.PodList{}
lo := &client.ListOptions{}
lo.MatchingField("spec.restartPolicy", "OnFailure")
Expect(informer.List(context.Background(), lo, listObj)).To(Succeed())
Expect(informer.List(context.Background(), listObj,
client.MatchingField("spec.restartPolicy", "OnFailure"))).To(Succeed())

By("verifying that the returned pods have correct restart policy")
Expect(listObj.Items).NotTo(BeEmpty())
Expand Down Expand Up @@ -647,9 +637,8 @@ var _ = Describe("Informer Cache", func() {
Version: "v1",
Kind: "PodList",
})
lo := &client.ListOptions{}
lo.MatchingField("spec.restartPolicy", "OnFailure")
err = informer.List(context.Background(), lo, listObj)
err = informer.List(context.Background(), listObj,
client.MatchingField("spec.restartPolicy", "OnFailure"))
Expect(err).To(Succeed())

By("verifying that the returned pods have correct restart policy")
Expand Down
4 changes: 2 additions & 2 deletions pkg/cache/informer_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, opts *client.ListOptions, out runtime.Object) error {
func (ip *informerCache) List(ctx context.Context, out runtime.Object, opts ...client.ListOptionFunc) error {
gvk, err := apiutil.GVKForObject(out, ip.Scheme)
if err != nil {
return err
Expand Down Expand Up @@ -95,7 +95,7 @@ func (ip *informerCache) List(ctx context.Context, opts *client.ListOptions, out
return err
}

return cache.Reader.List(ctx, opts, out)
return cache.Reader.List(ctx, out, opts...)
}

// GetInformerForKind returns the informer for the GroupVersionKind
Expand Down
2 changes: 1 addition & 1 deletion pkg/cache/informertest/fake_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, opts *client.ListOptions, list runtime.Object) error {
func (c *FakeInformers) List(ctx context.Context, list runtime.Object, opts ...client.ListOptionFunc) error {
return nil
}
19 changes: 11 additions & 8 deletions pkg/cache/internal/cache_reader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,32 +87,35 @@ 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, opts *client.ListOptions, out runtime.Object) error {
func (c *CacheReader) List(_ context.Context, out runtime.Object, opts ...client.ListOptionFunc) error {
var objs []interface{}
var err error

if opts != nil && opts.FieldSelector != nil {
listOpts := client.ListOptions{}
listOpts.ApplyOptions(opts)

if listOpts.FieldSelector != nil {
// TODO(directxman12): support more complicated field selectors by
// combining multiple indicies, GetIndexers, etc
field, val, requiresExact := requiresExactMatch(opts.FieldSelector)
field, val, requiresExact := requiresExactMatch(listOpts.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(opts.Namespace, val))
} else if opts != nil && opts.Namespace != "" {
objs, err = c.indexer.ByIndex(cache.NamespaceIndex, opts.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)
} else {
objs = c.indexer.List()
}
if err != nil {
return err
}
var labelSel labels.Selector
if opts != nil && opts.LabelSelector != nil {
labelSel = opts.LabelSelector
if listOpts.LabelSelector != nil {
labelSel = listOpts.LabelSelector
}

outItems, err := c.getListItems(objs, labelSel)
Expand Down
6 changes: 3 additions & 3 deletions pkg/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,12 @@ func (c *client) Get(ctx context.Context, key ObjectKey, obj runtime.Object) err
}

// List implements client.Client
func (c *client) List(ctx context.Context, opts *ListOptions, obj runtime.Object) error {
func (c *client) List(ctx context.Context, obj runtime.Object, opts ...ListOptionFunc) error {
_, ok := obj.(*unstructured.UnstructuredList)
if ok {
return c.unstructuredClient.List(ctx, opts, obj)
return c.unstructuredClient.List(ctx, obj, opts...)
}
return c.typedClient.List(ctx, opts, obj)
return c.typedClient.List(ctx, obj, opts...)
}

// Status implements client.StatusClient
Expand Down
Loading