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 71b49ab
Show file tree
Hide file tree
Showing 4 changed files with 168 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
}
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.

1 change: 1 addition & 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 71b49ab

Please sign in to comment.