Merge pull request kubernetes-sigs#311 from sebgl/fake-client-list-options

🐛 fix labels list option in fake client

Author: k8s-ci-robot

pkg/cache/internal/cache_reader.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/apimachinery/pkg/selection"
 	"k8s.io/client-go/tools/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/internal/objectutil"
 )
 
 // CacheReader is a CacheReader
@@ -118,33 +119,19 @@ func (c *CacheReader) List(_ context.Context, out runtime.Object, opts ...client
 		labelSel = listOpts.LabelSelector
 	}
 
-	outItems, err := c.getListItems(objs, labelSel)
-	if err != nil {
-		return err
-	}
-	return apimeta.SetList(out, outItems)
-}
-
-func (c *CacheReader) getListItems(objs []interface{}, labelSel labels.Selector) ([]runtime.Object, error) {
-	outItems := make([]runtime.Object, 0, len(objs))
+	runtimeObjs := make([]runtime.Object, 0, len(objs))
 	for _, item := range objs {
 		obj, isObj := item.(runtime.Object)
 		if !isObj {
-			return nil, fmt.Errorf("cache contained %T, which is not an Object", obj)
+			return fmt.Errorf("cache contained %T, which is not an Object", obj)
 		}
-		meta, err := apimeta.Accessor(obj)
-		if err != nil {
-			return nil, err
-		}
-		if labelSel != nil {
-			lbls := labels.Set(meta.GetLabels())
-			if !labelSel.Matches(lbls) {
-				continue
-			}
-		}
-		outItems = append(outItems, obj.DeepCopyObject())
+		runtimeObjs = append(runtimeObjs, obj)
+	}
+	filteredItems, err := objectutil.FilterWithLabels(runtimeObjs, labelSel)
+	if err != nil {
+		return err
 	}
-	return outItems, nil
+	return apimeta.SetList(out, filteredItems)
 }
 
 // objectKeyToStorageKey converts an object key to store key.

pkg/client/fake/client.go

@@ -32,6 +32,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	logf "sigs.k8s.io/controller-runtime/pkg/internal/log"
+	"sigs.k8s.io/controller-runtime/pkg/internal/objectutil"
 )
 
 var (
@@ -114,7 +115,25 @@ func (c *fakeClient) List(ctx context.Context, obj runtime.Object, opts ...clien
 	}
 	decoder := scheme.Codecs.UniversalDecoder()
 	_, _, err = decoder.Decode(j, nil, obj)
-	return err
+	if err != nil {
+		return err
+	}
+
+	if listOpts.LabelSelector != nil {
+		objs, err := meta.ExtractList(obj)
+		if err != nil {
+			return err
+		}
+		filteredObjs, err := objectutil.FilterWithLabels(objs, listOpts.LabelSelector)
+		if err != nil {
+			return err
+		}
+		err = meta.SetList(obj, filteredObjs)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 func (c *fakeClient) Create(ctx context.Context, obj runtime.Object) error {

pkg/client/fake/client_test.go

@@ -30,6 +30,7 @@ import (
 
 var _ = Describe("Fake client", func() {
 	var dep *appsv1.Deployment
+	var dep2 *appsv1.Deployment
 	var cm *corev1.ConfigMap
 	var cl client.Client
 
@@ -40,6 +41,15 @@ var _ = Describe("Fake client", func() {
 				Namespace: "ns1",
 			},
 		}
+		dep2 = &appsv1.Deployment{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-deployment-2",
+				Namespace: "ns1",
+				Labels: map[string]string{
+					"test-label": "label-value",
+				},
+			},
+		}
 		cm = &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "test-cm",
@@ -69,8 +79,20 @@ var _ = Describe("Fake client", func() {
 			list := &appsv1.DeploymentList{}
 			err := cl.List(nil, list, client.InNamespace("ns1"))
 			Expect(err).To(BeNil())
+			Expect(list.Items).To(HaveLen(2))
+			Expect(list.Items).To(ConsistOf(*dep, *dep2))
+		})
+
+		It("should support filtering by labels", func() {
+			By("Listing deployments with a particular label")
+			list := &appsv1.DeploymentList{}
+			err := cl.List(nil, list, client.InNamespace("ns1"),
+				client.MatchingLabels(map[string]string{
+					"test-label": "label-value",
+				}))
+			Expect(err).To(BeNil())
 			Expect(list.Items).To(HaveLen(1))
-			Expect(list.Items).To(ConsistOf(*dep))
+			Expect(list.Items).To(ConsistOf(*dep2))
 		})
 
 		It("should be able to Create", func() {
@@ -129,13 +151,14 @@ var _ = Describe("Fake client", func() {
 			list := &appsv1.DeploymentList{}
 			err = cl.List(nil, list, client.InNamespace("ns1"))
 			Expect(err).To(BeNil())
-			Expect(list.Items).To(HaveLen(0))
+			Expect(list.Items).To(HaveLen(1))
+			Expect(list.Items).To(ConsistOf(*dep2))
 		})
 	}
 
 	Context("with default scheme.Scheme", func() {
 		BeforeEach(func(done Done) {
-			cl = NewFakeClient(dep, cm)
+			cl = NewFakeClient(dep, dep2, cm)
 			close(done)
 		})
 		AssertClientBehavior()
@@ -146,7 +169,7 @@ var _ = Describe("Fake client", func() {
 			scheme := runtime.NewScheme()
 			corev1.AddToScheme(scheme)
 			appsv1.AddToScheme(scheme)
-			cl = NewFakeClientWithScheme(scheme, dep, cm)
+			cl = NewFakeClientWithScheme(scheme, dep, dep2, cm)
 			close(done)
 		})
 		AssertClientBehavior()

pkg/internal/objectutil/filter.go

@@ -0,0 +1,42 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package objectutil
+
+import (
+	apimeta "k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+// FilterWithLabels returns a copy of the items in objs matching labelSel
+func FilterWithLabels(objs []runtime.Object, labelSel labels.Selector) ([]runtime.Object, error) {
+	outItems := make([]runtime.Object, 0, len(objs))
+	for _, obj := range objs {
+		meta, err := apimeta.Accessor(obj)
+		if err != nil {
+			return nil, err
+		}
+		if labelSel != nil {
+			lbls := labels.Set(meta.GetLabels())
+			if !labelSel.Matches(lbls) {
+				continue
+			}
+		}
+		outItems = append(outItems, obj.DeepCopyObject())
+	}
+	return outItems, nil
+}