Skip to content

Commit

Permalink
Update List implementations to use new signature
Browse files Browse the repository at this point in the history
The fake client now uses similar GVK detection code as the other
implementations and no longer relies on the Raw object having a
TypeMeta.
  • Loading branch information
grantr committed Sep 14, 2018
1 parent 58ae7e5 commit 8ca92a0
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 51 deletions.
21 changes: 12 additions & 9 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(), 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 @@ -143,8 +143,10 @@ 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(), client.InNamespace(testNamespaceTwo).
MatchingLabels(map[string]string{"test-label": "test-pod-2"}), &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 @@ -161,8 +163,9 @@ 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(),
client.MatchingLabels(labels), &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 @@ -177,9 +180,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(),
Expect(informerCache.List(context.Background(), listObj,
client.InNamespace(testNamespaceOne),
listObj)).To(Succeed())
)).To(Succeed())

By("verifying that the returned pods are in test-namespace-1")
Expect(listObj.Items).NotTo(BeEmpty())
Expand Down Expand Up @@ -317,9 +320,9 @@ var _ = Describe("Informer Cache", func() {

By("listing Pods with restartPolicyOnFailure")
listObj := &kcorev1.PodList{}
Expect(informer.List(context.Background(),
Expect(informer.List(context.Background(), listObj,
client.MatchingField("spec.restartPolicy", "OnFailure"),
listObj)).To(Succeed())
)).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, opts *client.ListOptions, out runtime.Object) error {
func (ip *informerCache) List(ctx context.Context, out runtime.Object, opts ...client.ListOptionFunc) 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, 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
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
Original file line number Diff line number Diff line change
Expand Up @@ -86,32 +86,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(ctx context.Context, opts *client.ListOptions, out runtime.Object) error {
func (c *CacheReader) List(ctx 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
23 changes: 19 additions & 4 deletions pkg/client/fake/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ package fake
import (
"context"
"encoding/json"
"fmt"
"os"
"strings"

"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -77,10 +79,23 @@ func (c *fakeClient) Get(ctx context.Context, key client.ObjectKey, obj runtime.
return err
}

func (c *fakeClient) List(ctx context.Context, opts *client.ListOptions, list runtime.Object) error {
gvk := opts.Raw.TypeMeta.GroupVersionKind()
func (c *fakeClient) List(ctx context.Context, obj runtime.Object, opts ...client.ListOptionFunc) error {
gvk, err := apiutil.GVKForObject(obj, scheme.Scheme)
if err != nil {
return err
}

if !strings.HasSuffix(gvk.Kind, "List") {
return fmt.Errorf("non-list type %T (kind %q) passed as output", obj, gvk)
}
// we need the non-list GVK, so chop off the "List" from the end of the kind
gvk.Kind = gvk.Kind[:len(gvk.Kind)-4]

listOpts := client.ListOptions{}
listOpts.ApplyOptions(opts)

gvr, _ := meta.UnsafeGuessKindToResource(gvk)
o, err := c.tracker.List(gvr, gvk, opts.Namespace)
o, err := c.tracker.List(gvr, gvk, listOpts.Namespace)
if err != nil {
return err
}
Expand All @@ -89,7 +104,7 @@ func (c *fakeClient) List(ctx context.Context, opts *client.ListOptions, list ru
return err
}
decoder := scheme.Codecs.UniversalDecoder()
_, _, err = decoder.Decode(j, nil, list)
_, _, err = decoder.Decode(j, nil, obj)
return err
}

Expand Down
32 changes: 5 additions & 27 deletions pkg/client/fake/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,12 @@ limitations under the License.
package fake

import (
"encoding/json"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
)
Expand Down Expand Up @@ -69,22 +66,11 @@ var _ = Describe("Fake client", func() {

It("should be able to List", func() {
By("Listing all deployments in a namespace")
list := &metav1.List{}
err := cl.List(nil, &client.ListOptions{
Namespace: "ns1",
Raw: &metav1.ListOptions{
TypeMeta: metav1.TypeMeta{
APIVersion: "apps/v1",
Kind: "Deployment",
},
},
}, list)
list := &appsv1.DeploymentList{}
err := cl.List(nil, list, client.InNamespace("ns1"))
Expect(err).To(BeNil())
Expect(list.Items).To(HaveLen(1))
j, err := json.Marshal(dep)
Expect(err).To(BeNil())
expectedDep := runtime.RawExtension{Raw: j}
Expect(list.Items).To(ConsistOf(expectedDep))
Expect(list.Items).To(ConsistOf(*dep))
})

It("should be able to Create", func() {
Expand Down Expand Up @@ -140,16 +126,8 @@ var _ = Describe("Fake client", func() {
Expect(err).To(BeNil())

By("Listing all deployments in the namespace")
list := &metav1.List{}
err = cl.List(nil, &client.ListOptions{
Namespace: "ns1",
Raw: &metav1.ListOptions{
TypeMeta: metav1.TypeMeta{
APIVersion: "apps/v1",
Kind: "Deployment",
},
},
}, list)
list := &appsv1.DeploymentList{}
err = cl.List(nil, list, client.InNamespace("ns1"))
Expect(err).To(BeNil())
Expect(list.Items).To(HaveLen(0))
})
Expand Down

0 comments on commit 8ca92a0

Please sign in to comment.