Skip to content

Commit

Permalink
Use an Informer to list LimitRanges šŸ„¼
Browse files Browse the repository at this point in the history 
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 :
#4176 (comment).

Signed-off-by: Vincent Demeester <[email protected]>
  • Loading branch information
@vdemeester
vdemeester committed Sep 16, 2021
1 parent ff860df commit 5002741
Show file tree
Hide file tree
Showing 6 changed files with 220 additions and 28 deletions.
21 changes: 12 additions & 9 deletions pkg/internal/limitrange/limitrange.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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])
Expand All @@ -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 {
Expand Down
68 changes: 49 additions & 19 deletions pkg/internal/limitrange/transformer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
})
Expand Down Expand Up @@ -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,
})
Expand Down Expand Up @@ -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,
})
Expand Down Expand Up @@ -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
}
11 changes: 11 additions & 0 deletions test/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
40 changes: 40 additions & 0 deletions vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/fake/fake.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

106 changes: 106 additions & 0 deletions vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/limitrange.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions vendor/modules.txt
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 5002741

Please sign in to comment.