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.

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

Revert "Convert client.List to use functional options" #146

Merged
merged 1 commit into from
Sep 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion pkg/builder/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
21 changes: 9 additions & 12 deletions pkg/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand All @@ -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())
Expand Down Expand Up @@ -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())
Expand Down
4 changes: 2 additions & 2 deletions pkg/cache/informer_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/cache/informertest/fake_cache.go
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, list runtime.Object, opts ...client.ListOptionFunc) error {
func (c *FakeInformers) List(ctx context.Context, opts *client.ListOptions, list runtime.Object) error {
return nil
}
19 changes: 8 additions & 11 deletions pkg/cache/internal/cache_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 7 additions & 6 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Loading