From 04b1b70c7796d7481335cc119554da175ff539b3 Mon Sep 17 00:00:00 2001 From: sebgl Date: Fri, 25 Jan 2019 22:20:15 +0100 Subject: [PATCH 1/2] Enable labels list options in fake client This commit allows passing label matches to the fake client List() function. This is done in a way similar to the CachedReader. Both now share a common call to `objectutil.FilterWithLabels`. Signed-off-by: sebgl --- pkg/cache/internal/cache_reader.go | 31 +++++++--------------- pkg/client/fake/client.go | 21 ++++++++++++++- pkg/client/fake/client_test.go | 31 +++++++++++++++++++--- pkg/internal/objectutil/filter.go | 42 ++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 27 deletions(-) create mode 100644 pkg/internal/objectutil/filter.go diff --git a/pkg/cache/internal/cache_reader.go b/pkg/cache/internal/cache_reader.go index 62a16eb1ae..8abf09d10f 100644 --- a/pkg/cache/internal/cache_reader.go +++ b/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 @@ -115,33 +116,19 @@ func (c *CacheReader) List(_ context.Context, opts *client.ListOptions, out runt labelSel = opts.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. diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go index 09055a5bac..a664d97915 100644 --- a/pkg/client/fake/client.go +++ b/pkg/client/fake/client.go @@ -31,6 +31,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" + "sigs.k8s.io/controller-runtime/pkg/internal/objectutil" logf "sigs.k8s.io/controller-runtime/pkg/runtime/log" ) @@ -110,7 +111,25 @@ func (c *fakeClient) List(ctx context.Context, opts *client.ListOptions, list ru } decoder := scheme.Codecs.UniversalDecoder() _, _, err = decoder.Decode(j, nil, list) - return err + if err != nil { + return err + } + + if opts.LabelSelector != nil { + objs, err := meta.ExtractList(list) + if err != nil { + return err + } + filteredObjs, err := objectutil.FilterWithLabels(objs, opts.LabelSelector) + if err != nil { + return err + } + err = meta.SetList(list, filteredObjs) + if err != nil { + return err + } + } + return nil } func (c *fakeClient) Create(ctx context.Context, obj runtime.Object) error { diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go index ac05f9302a..94057763f6 100644 --- a/pkg/client/fake/client_test.go +++ b/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", @@ -71,8 +81,20 @@ var _ = Describe("Fake client", func() { Namespace: "ns1", }, list) 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, + client.InNamespace("ns1").MatchingLabels(map[string]string{ + "test-label": "label-value", + }), list) + 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() { @@ -133,13 +155,14 @@ var _ = Describe("Fake client", func() { Namespace: "ns1", }, list) 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() @@ -150,7 +173,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() diff --git a/pkg/internal/objectutil/filter.go b/pkg/internal/objectutil/filter.go new file mode 100644 index 0000000000..8513846e2c --- /dev/null +++ b/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 +} From e1ec14f3eef685c574a46202d1d5d4c4e95c82a0 Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Fri, 26 Apr 2019 13:31:01 +0200 Subject: [PATCH 2/2] Move some code under filterListItems to decrease cyclomatic complexity To eliminate: pkg/cache/internal/cache_reader.go:91::warning: cyclomatic complexity 12 of function (*CacheReader).List() is high (> 10) (gocyclo) pkg/client/fake/client.go:92::warning: cyclomatic complexity 12 of function (*fakeClient).List() is high (> 10) (gocyclo) --- pkg/cache/internal/cache_reader.go | 17 ++++++++++++++--- pkg/client/fake/client.go | 30 ++++++++++++++++++------------ 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/pkg/cache/internal/cache_reader.go b/pkg/cache/internal/cache_reader.go index 8abf09d10f..a5bac80385 100644 --- a/pkg/cache/internal/cache_reader.go +++ b/pkg/cache/internal/cache_reader.go @@ -116,19 +116,30 @@ func (c *CacheReader) List(_ context.Context, opts *client.ListOptions, out runt labelSel = opts.LabelSelector } + filteredItems, err := c.filterListItems(objs, labelSel) + if err != nil { + return err + } + + return apimeta.SetList(out, filteredItems) +} + +func (c *CacheReader) filterListItems(objs []interface{}, labelSel labels.Selector) ([]runtime.Object, error) { runtimeObjs := make([]runtime.Object, 0, len(objs)) for _, item := range objs { obj, isObj := item.(runtime.Object) if !isObj { - return fmt.Errorf("cache contained %T, which is not an Object", obj) + return nil, fmt.Errorf("cache contained %T, which is not an Object", obj) } runtimeObjs = append(runtimeObjs, obj) } + filteredItems, err := objectutil.FilterWithLabels(runtimeObjs, labelSel) if err != nil { - return err + return nil, err } - return apimeta.SetList(out, filteredItems) + + return filteredItems, nil } // objectKeyToStorageKey converts an object key to store key. diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go index a664d97915..1d730940ca 100644 --- a/pkg/client/fake/client.go +++ b/pkg/client/fake/client.go @@ -24,6 +24,7 @@ import ( "strings" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes/scheme" @@ -116,18 +117,23 @@ func (c *fakeClient) List(ctx context.Context, opts *client.ListOptions, list ru } if opts.LabelSelector != nil { - objs, err := meta.ExtractList(list) - if err != nil { - return err - } - filteredObjs, err := objectutil.FilterWithLabels(objs, opts.LabelSelector) - if err != nil { - return err - } - err = meta.SetList(list, filteredObjs) - if err != nil { - return err - } + return filterListItems(list, opts.LabelSelector) + } + return nil +} + +func filterListItems(list runtime.Object, labSel labels.Selector) error { + objs, err := meta.ExtractList(list) + if err != nil { + return err + } + filteredObjs, err := objectutil.FilterWithLabels(objs, labSel) + if err != nil { + return err + } + err = meta.SetList(list, filteredObjs) + if err != nil { + return err } return nil }