From 5002741679f51dc7caec377401df4cd4495689df Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Thu, 16 Sep 2021 12:31:24 +0200 Subject: [PATCH] =?UTF-8?q?Use=20an=20Informer=20to=20list=20LimitRanges?= =?UTF-8?q?=20=F0=9F=A5=BC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using a LimitRange lister here instead, so this doesn't end up hitting the real API server on each call. Taking into account a review : https://github.com/tektoncd/pipeline/pull/4176#issuecomment-920366660. Signed-off-by: Vincent Demeester --- pkg/internal/limitrange/limitrange.go | 21 ++-- pkg/internal/limitrange/transformer_test.go | 68 +++++++---- test/controller.go | 11 ++ .../informers/core/v1/limitrange/fake/fake.go | 40 +++++++ .../core/v1/limitrange/limitrange.go | 106 ++++++++++++++++++ vendor/modules.txt | 2 + 6 files changed, 220 insertions(+), 28 deletions(-) create mode 100644 vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/fake/fake.go create mode 100644 vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/limitrange.go diff --git a/pkg/internal/limitrange/limitrange.go b/pkg/internal/limitrange/limitrange.go index af3c905fdeb..473d14289cc 100644 --- a/pkg/internal/limitrange/limitrange.go +++ b/pkg/internal/limitrange/limitrange.go @@ -20,23 +20,25 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/client-go/kubernetes" + limitrangeinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange" ) func getVirtualLimitRange(ctx context.Context, namespace string, c kubernetes.Interface) (*corev1.LimitRange, error) { - limitRanges, err := c.CoreV1().LimitRanges(namespace).List(ctx, metav1.ListOptions{}) + limitrangeInformer := limitrangeinformer.Get(ctx) + limitRanges, err := limitrangeInformer.Lister().LimitRanges(namespace).List(labels.Everything()) if err != nil { return nil, err } - var limitRange corev1.LimitRange + var limitRange *corev1.LimitRange switch { - case len(limitRanges.Items) == 0: + case len(limitRanges) == 0: // No LimitRange defined break - case len(limitRanges.Items) == 1: + case len(limitRanges) == 1: // One LimitRange defined - limitRange = limitRanges.Items[0] + limitRange = limitRanges[0] default: // Several LimitRange defined // Create a virtual LimitRange with @@ -45,8 +47,9 @@ func getVirtualLimitRange(ctx context.Context, namespace string, c kubernetes.In // - Default that "fits" into min/max taken above // - Default request that "fits" into min/max taken above // - Smallest ratio (aka the most restrictive one) + limitRange = &corev1.LimitRange{} m := map[corev1.LimitType]corev1.LimitRangeItem{} - for _, lr := range limitRanges.Items { + for _, lr := range limitRanges { for _, item := range lr.Spec.Limits { _, exists := m[item.Type] if !exists { @@ -74,7 +77,7 @@ func getVirtualLimitRange(ctx context.Context, namespace string, c kubernetes.In } } // Handle Default and DefaultRequest - for _, lr := range limitRanges.Items { + for _, lr := range limitRanges { for _, item := range lr.Spec.Limits { // Default m[item.Type].Default[corev1.ResourceCPU] = minOfBetween(m[item.Type].Default[corev1.ResourceCPU], item.Default[corev1.ResourceCPU], m[item.Type].Min[corev1.ResourceCPU], m[item.Type].Max[corev1.ResourceCPU]) @@ -90,7 +93,7 @@ func getVirtualLimitRange(ctx context.Context, namespace string, c kubernetes.In limitRange.Spec.Limits = append(limitRange.Spec.Limits, v) } } - return &limitRange, nil + return limitRange, nil } func maxOf(a, b resource.Quantity) resource.Quantity { diff --git a/pkg/internal/limitrange/transformer_test.go b/pkg/internal/limitrange/transformer_test.go index 54ad05a069c..3e0b26e1039 100644 --- a/pkg/internal/limitrange/transformer_test.go +++ b/pkg/internal/limitrange/transformer_test.go @@ -20,12 +20,17 @@ import ( "testing" "github.com/google/go-cmp/cmp" + ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" + "github.com/tektoncd/pipeline/test" "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - fakek8s "k8s.io/client-go/kubernetes/fake" + // "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes" + fakekubeclient "knative.dev/pkg/client/injection/kube/client/fake" + fakelimitrangeinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/fake" + fakeserviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/fake" ) var resourceQuantityCmp = cmp.Comparer(func(x, y resource.Quantity) bool { @@ -405,15 +410,16 @@ func TestTransformerOneContainer(t *testing.T) { }, }} { t.Run(tc.description, func(t *testing.T) { - kubeclient := fakek8s.NewSimpleClientset( - &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}}, - &corev1.LimitRange{ObjectMeta: metav1.ObjectMeta{Name: "limitrange", Namespace: "default"}, + ctx, kubeclient, cancel := setup(t, + []corev1.ServiceAccount{{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}}}, + []corev1.LimitRange{{ObjectMeta: metav1.ObjectMeta{Name: "limitrange", Namespace: "default"}, Spec: corev1.LimitRangeSpec{ Limits: tc.limitranges, }, - }, + }}, ) - f := NewTransformer(context.Background(), "default", kubeclient) + defer cancel() + f := NewTransformer(ctx, "default", kubeclient) got, err := f(&corev1.Pod{ Spec: tc.podspec, }) @@ -817,15 +823,16 @@ func TestTransformerMultipleContainer(t *testing.T) { }, }} { t.Run(tc.description, func(t *testing.T) { - kubeclient := fakek8s.NewSimpleClientset( - &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}}, - &corev1.LimitRange{ObjectMeta: metav1.ObjectMeta{Name: "limitrange", Namespace: "default"}, + ctx, kubeclient, cancel := setup(t, + []corev1.ServiceAccount{{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}}}, + []corev1.LimitRange{{ObjectMeta: metav1.ObjectMeta{Name: "limitrange", Namespace: "default"}, Spec: corev1.LimitRangeSpec{ Limits: tc.limitranges, }, - }, + }}, ) - f := NewTransformer(context.Background(), "default", kubeclient) + defer cancel() + f := NewTransformer(ctx, "default", kubeclient) got, err := f(&corev1.Pod{ Spec: tc.podspec, }) @@ -943,13 +950,12 @@ func TestTransformerOneContainerMultipleLimitRange(t *testing.T) { }, }} { t.Run(tc.description, func(t *testing.T) { - runtimeObjects := []runtime.Object{&corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}}} - for _, l := range tc.limitranges { - l := l // because we use pointer, we need to descope this... - runtimeObjects = append(runtimeObjects, &l) - } - kubeclient := fakek8s.NewSimpleClientset(runtimeObjects...) - f := NewTransformer(context.Background(), "default", kubeclient) + ctx, kubeclient, cancel := setup(t, + []corev1.ServiceAccount{{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}}}, + tc.limitranges, + ) + defer cancel() + f := NewTransformer(ctx, "default", kubeclient) got, err := f(&corev1.Pod{ Spec: tc.podspec, }) @@ -985,3 +991,27 @@ func cmpRequestsAndLimits(t *testing.T, want, got corev1.PodSpec) { } } } + +func setup(t *testing.T, serviceaccounts []corev1.ServiceAccount, limitranges []corev1.LimitRange) (context.Context, kubernetes.Interface, func()) { + ctx, _ := ttesting.SetupFakeContext(t) + ctx, cancel := context.WithCancel(ctx) + kubeclient := fakekubeclient.Get(ctx) + // LimitRange + limitRangeInformer := fakelimitrangeinformer.Get(ctx) + kubeclient.PrependReactor("*", "limitranges", test.AddToInformer(t, limitRangeInformer.Informer().GetIndexer())) + for _, tl := range limitranges { + if _, err := kubeclient.CoreV1().LimitRanges(tl.Namespace).Create(ctx, &tl, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + } + // ServiceAccount + serviceAccountInformer := fakeserviceaccountinformer.Get(ctx) + kubeclient.PrependReactor("*", "serviceaccounts", test.AddToInformer(t, serviceAccountInformer.Informer().GetIndexer())) + for _, ts := range serviceaccounts { + if _, err := kubeclient.CoreV1().ServiceAccounts(ts.Namespace).Create(ctx, &ts, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + } + kubeclient.ClearActions() + return ctx, kubeclient, cancel +} diff --git a/test/controller.go b/test/controller.go index 435a99ac398..48fbf49ea51 100644 --- a/test/controller.go +++ b/test/controller.go @@ -54,6 +54,7 @@ import ( "k8s.io/client-go/tools/record" fakekubeclient "knative.dev/pkg/client/injection/kube/client/fake" fakeconfigmapinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake" + fakelimitrangeinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/fake" fakefilteredpodinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/pod/filtered/fake" fakeserviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/fake" "knative.dev/pkg/controller" @@ -74,6 +75,7 @@ type Data struct { Namespaces []*corev1.Namespace ConfigMaps []*corev1.ConfigMap ServiceAccounts []*corev1.ServiceAccount + LimitRanges []*corev1.LimitRange } // Clients holds references to clients which are useful for reconciler tests. @@ -97,6 +99,7 @@ type Informers struct { Pod coreinformers.PodInformer ConfigMap coreinformers.ConfigMapInformer ServiceAccount coreinformers.ServiceAccountInformer + LimitRange coreinformers.LimitRangeInformer } // Assets holds references to the controller, logs, clients, and informers. @@ -177,6 +180,7 @@ func SeedTestData(t *testing.T, ctx context.Context, d Data) (Clients, Informers Pod: fakefilteredpodinformer.Get(ctx, v1beta1.ManagedByLabelKey), ConfigMap: fakeconfigmapinformer.Get(ctx), ServiceAccount: fakeserviceaccountinformer.Get(ctx), + LimitRange: fakelimitrangeinformer.Get(ctx), } // Attach reactors that add resource mutations to the appropriate @@ -265,6 +269,13 @@ func SeedTestData(t *testing.T, ctx context.Context, d Data) (Clients, Informers t.Fatal(err) } } + c.Kube.PrependReactor("*", "limitranges", AddToInformer(t, i.LimitRange.Informer().GetIndexer())) + for _, lr := range d.LimitRanges { + lr := lr.DeepCopy() // Avoid assumptions that the informer's copy is modified. + if _, err := c.Kube.CoreV1().LimitRanges(lr.Namespace).Create(ctx, lr, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + } c.Pipeline.ClearActions() c.Kube.ClearActions() return c, i diff --git a/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/fake/fake.go b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/fake/fake.go new file mode 100644 index 00000000000..0d5f06be923 --- /dev/null +++ b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/fake/fake.go @@ -0,0 +1,40 @@ +/* +Copyright 2021 The Knative 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. +*/ + +// Code generated by injection-gen. DO NOT EDIT. + +package fake + +import ( + context "context" + + limitrange "knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange" + fake "knative.dev/pkg/client/injection/kube/informers/factory/fake" + controller "knative.dev/pkg/controller" + injection "knative.dev/pkg/injection" +) + +var Get = limitrange.Get + +func init() { + injection.Fake.RegisterInformer(withInformer) +} + +func withInformer(ctx context.Context) (context.Context, controller.Informer) { + f := fake.Get(ctx) + inf := f.Core().V1().LimitRanges() + return context.WithValue(ctx, limitrange.Key{}, inf), inf.Informer() +} diff --git a/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/limitrange.go b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/limitrange.go new file mode 100644 index 00000000000..d86077f7db7 --- /dev/null +++ b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/limitrange.go @@ -0,0 +1,106 @@ +/* +Copyright 2021 The Knative 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. +*/ + +// Code generated by injection-gen. DO NOT EDIT. + +package limitrange + +import ( + context "context" + + apicorev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + labels "k8s.io/apimachinery/pkg/labels" + v1 "k8s.io/client-go/informers/core/v1" + kubernetes "k8s.io/client-go/kubernetes" + corev1 "k8s.io/client-go/listers/core/v1" + cache "k8s.io/client-go/tools/cache" + client "knative.dev/pkg/client/injection/kube/client" + factory "knative.dev/pkg/client/injection/kube/informers/factory" + controller "knative.dev/pkg/controller" + injection "knative.dev/pkg/injection" + logging "knative.dev/pkg/logging" +) + +func init() { + injection.Default.RegisterInformer(withInformer) + injection.Dynamic.RegisterDynamicInformer(withDynamicInformer) +} + +// Key is used for associating the Informer inside the context.Context. +type Key struct{} + +func withInformer(ctx context.Context) (context.Context, controller.Informer) { + f := factory.Get(ctx) + inf := f.Core().V1().LimitRanges() + return context.WithValue(ctx, Key{}, inf), inf.Informer() +} + +func withDynamicInformer(ctx context.Context) context.Context { + inf := &wrapper{client: client.Get(ctx)} + return context.WithValue(ctx, Key{}, inf) +} + +// Get extracts the typed informer from the context. +func Get(ctx context.Context) v1.LimitRangeInformer { + untyped := ctx.Value(Key{}) + if untyped == nil { + logging.FromContext(ctx).Panic( + "Unable to fetch k8s.io/client-go/informers/core/v1.LimitRangeInformer from context.") + } + return untyped.(v1.LimitRangeInformer) +} + +type wrapper struct { + client kubernetes.Interface + + namespace string +} + +var _ v1.LimitRangeInformer = (*wrapper)(nil) +var _ corev1.LimitRangeLister = (*wrapper)(nil) + +func (w *wrapper) Informer() cache.SharedIndexInformer { + return cache.NewSharedIndexInformer(nil, &apicorev1.LimitRange{}, 0, nil) +} + +func (w *wrapper) Lister() corev1.LimitRangeLister { + return w +} + +func (w *wrapper) LimitRanges(namespace string) corev1.LimitRangeNamespaceLister { + return &wrapper{client: w.client, namespace: namespace} +} + +func (w *wrapper) List(selector labels.Selector) (ret []*apicorev1.LimitRange, err error) { + lo, err := w.client.CoreV1().LimitRanges(w.namespace).List(context.TODO(), metav1.ListOptions{ + LabelSelector: selector.String(), + // TODO(mattmoor): Incorporate resourceVersion bounds based on staleness criteria. + }) + if err != nil { + return nil, err + } + for idx := range lo.Items { + ret = append(ret, &lo.Items[idx]) + } + return ret, nil +} + +func (w *wrapper) Get(name string) (*apicorev1.LimitRange, error) { + return w.client.CoreV1().LimitRanges(w.namespace).Get(context.TODO(), name, metav1.GetOptions{ + // TODO(mattmoor): Incorporate resourceVersion bounds based on staleness criteria. + }) +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 8bffb17a773..9dedc592c72 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -937,6 +937,8 @@ knative.dev/pkg/client/injection/kube/informers/admissionregistration/v1/mutatin knative.dev/pkg/client/injection/kube/informers/admissionregistration/v1/validatingwebhookconfiguration knative.dev/pkg/client/injection/kube/informers/core/v1/configmap knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake +knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange +knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/fake knative.dev/pkg/client/injection/kube/informers/core/v1/pod/filtered knative.dev/pkg/client/injection/kube/informers/core/v1/pod/filtered/fake knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount