diff --git a/pkg/internal/limitrange/limitrange.go b/pkg/internal/computeresources/limitrange/limitrange.go similarity index 82% rename from pkg/internal/limitrange/limitrange.go rename to pkg/internal/computeresources/limitrange/limitrange.go index 46cbe5672d6..9c986ab07fb 100644 --- a/pkg/internal/limitrange/limitrange.go +++ b/pkg/internal/computeresources/limitrange/limitrange.go @@ -25,7 +25,18 @@ import ( corev1listers "k8s.io/client-go/listers/core/v1" ) -func getVirtualLimitRange(ctx context.Context, namespace string, lister corev1listers.LimitRangeLister) (*corev1.LimitRange, error) { +// GetVirtualLimitRange returns a pointer to a single LimitRange representing the most restrictive +// requirements of all LimitRanges present in the namespace, or a nil pointer if there are no LimitRanges. +// This LimitRange meets the following constraints: +// - Its max is the smallest max of all the LimitRanges +// - Its min is the largest min of all the LimitRanges +// - Its maxLimitRequestRatio is the smallest maxLimitRequestRatio of all the LimitRanges +// - Its default is the smallest default of any of the LimitRanges that fits within the minimum and maximum +// - Its defaultRequest is the smallest defaultRequest of any of the LimitRanges that fits within the minimum and maximum +// +// This function isn't guaranteed to return a LimitRange with consistent constraints. +// For example, the minimum could be greater than the maximum. +func GetVirtualLimitRange(ctx context.Context, namespace string, lister corev1listers.LimitRangeLister) (*corev1.LimitRange, error) { limitRanges, err := lister.LimitRanges(namespace).List(labels.Everything()) if err != nil { return nil, err @@ -40,12 +51,6 @@ func getVirtualLimitRange(ctx context.Context, namespace string, lister corev1li limitRange = limitRanges[0] default: // Several LimitRange defined - // Create a virtual LimitRange with - // - Maximum of min values - // - Minimum of max values - // - 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 { @@ -95,6 +100,11 @@ func getVirtualLimitRange(ctx context.Context, namespace string, lister corev1li return limitRange, nil } +// IsZero returns true if the resource quantity has a zero value +func IsZero(q resource.Quantity) bool { + return (&q).IsZero() +} + func maxOf(a, b resource.Quantity) resource.Quantity { if (&a).Cmp(b) > 0 { return a @@ -103,14 +113,14 @@ func maxOf(a, b resource.Quantity) resource.Quantity { } func minOf(a, b resource.Quantity) resource.Quantity { - if isZero(a) || (&a).Cmp(b) > 0 { + if IsZero(a) || (&a).Cmp(b) > 0 { return b } return a } func minOfBetween(a, b, min, max resource.Quantity) resource.Quantity { - if isZero(a) || (&a).Cmp(b) > 0 { + if IsZero(a) || (&a).Cmp(b) > 0 { return b } return a diff --git a/pkg/internal/computeresources/limitrange/limitrange_test.go b/pkg/internal/computeresources/limitrange/limitrange_test.go new file mode 100644 index 00000000000..19e347932f0 --- /dev/null +++ b/pkg/internal/computeresources/limitrange/limitrange_test.go @@ -0,0 +1,317 @@ +/* +Copyright 2022 The Tekton 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 limitrange_test + +import ( + "context" + "strconv" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/internal/computeresources/limitrange" + ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" + "github.com/tektoncd/pipeline/test" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + 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" +) + +func setupTestData(t *testing.T, serviceaccounts []corev1.ServiceAccount, limitranges []corev1.LimitRange) (context.Context, 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, cancel +} + +func TestGetVirtualLimitRange(t *testing.T) { + limitRange1 := corev1.LimitRange{ + ObjectMeta: metav1.ObjectMeta{Name: "limitrange1", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Max: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("3"), + corev1.ResourceMemory: resource.MustParse("300Mi"), + corev1.ResourceEphemeralStorage: resource.MustParse("3Gi"), + }, + Min: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("700m"), + corev1.ResourceMemory: resource.MustParse("70Mi"), + corev1.ResourceEphemeralStorage: resource.MustParse("700Mi"), + }, + Default: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("2"), + corev1.ResourceMemory: resource.MustParse("200Mi"), + corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"), + }, + DefaultRequest: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + corev1.ResourceMemory: resource.MustParse("100Mi"), + corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"), + }, + }}}} + + for _, tc := range []struct { + description string + limitranges []corev1.LimitRange + want *corev1.LimitRange + }{{ + description: "No limitRange", + want: nil, + }, { + description: "Single LimitRange", + limitranges: []corev1.LimitRange{limitRange1}, + want: &limitRange1, + }, { + description: "multiple LimitRanges; min only", + limitranges: []corev1.LimitRange{{ + ObjectMeta: metav1.ObjectMeta{Name: "limitrange1", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Min: createResourceList(7), + }}}}, { + ObjectMeta: metav1.ObjectMeta{Name: "limitrange2", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Min: createResourceList(8), + }}}}, + }, + want: &corev1.LimitRange{ + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Min: createResourceList(8), + }}}}, + }, { + description: "multiple LimitRanges; max only", + limitranges: []corev1.LimitRange{{ + ObjectMeta: metav1.ObjectMeta{Name: "limitrange1", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Max: createResourceList(7), + }}}}, { + ObjectMeta: metav1.ObjectMeta{Name: "limitrange2", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Max: createResourceList(8), + }}}}, + }, + want: &corev1.LimitRange{ + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Max: createResourceList(7), + }}}}, + }, { + description: "multiple LimitRanges; maxLimitRequestRatio only", + limitranges: []corev1.LimitRange{{ + ObjectMeta: metav1.ObjectMeta{Name: "limitrange1", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + MaxLimitRequestRatio: createResourceList(7), + }}}}, { + ObjectMeta: metav1.ObjectMeta{Name: "limitrange2", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + MaxLimitRequestRatio: createResourceList(8), + }}}}, + }, + want: &corev1.LimitRange{ + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + MaxLimitRequestRatio: createResourceList(7), + }}}}, + }, { + description: "multiple LimitRanges; default only", + limitranges: []corev1.LimitRange{{ + ObjectMeta: metav1.ObjectMeta{Name: "limitrange1", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Default: createResourceList(7), + }}}}, { + ObjectMeta: metav1.ObjectMeta{Name: "limitrange2", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Default: createResourceList(8), + }}}}, + }, + want: &corev1.LimitRange{ + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Default: createResourceList(7), + }}}}, + }, { + description: "multiple LimitRanges; defaultRequest only", + limitranges: []corev1.LimitRange{{ + ObjectMeta: metav1.ObjectMeta{Name: "limitrange1", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + DefaultRequest: createResourceList(7), + }}}}, { + ObjectMeta: metav1.ObjectMeta{Name: "limitrange2", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + DefaultRequest: createResourceList(8), + }}}}, + }, + want: &corev1.LimitRange{ + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + DefaultRequest: createResourceList(7), + }}}}, + }, { + description: "multiple LimitRanges; all fields", + limitranges: []corev1.LimitRange{{ + ObjectMeta: metav1.ObjectMeta{Name: "limitrange1", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Min: createResourceList(3), + Max: createResourceList(10), + MaxLimitRequestRatio: createResourceList(3), + Default: createResourceList(8), + DefaultRequest: createResourceList(7), + }}}}, { + ObjectMeta: metav1.ObjectMeta{Name: "limitrange2", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Min: createResourceList(1), + Max: createResourceList(5), + MaxLimitRequestRatio: createResourceList(2), + Default: createResourceList(5), + DefaultRequest: createResourceList(3), + }}}}, + }, + want: &corev1.LimitRange{ + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Min: createResourceList(3), + Max: createResourceList(5), + MaxLimitRequestRatio: createResourceList(2), + Default: createResourceList(5), + DefaultRequest: createResourceList(3), + }}}}, + }, { + description: "multiple contradictory LimitRanges", + limitranges: []corev1.LimitRange{{ + ObjectMeta: metav1.ObjectMeta{Name: "limitrange1", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Min: createResourceList(6), + Max: createResourceList(10), + }}}}, { + ObjectMeta: metav1.ObjectMeta{Name: "limitrange2", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Min: createResourceList(1), + Max: createResourceList(5), + }}}}, + }, + want: &corev1.LimitRange{ + Spec: corev1.LimitRangeSpec{ + Limits: []corev1.LimitRangeItem{{ + Type: corev1.LimitTypeContainer, + Min: createResourceList(6), + Max: createResourceList(5), + }}}}, + }} { + t.Run(tc.description, func(t *testing.T) { + ctx, cancel := setupTestData(t, + []corev1.ServiceAccount{{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}}}, + tc.limitranges, + ) + defer cancel() + lister := fakelimitrangeinformer.Get(ctx).Lister() + got, err := limitrange.GetVirtualLimitRange(ctx, "default", lister) + if err != nil { + t.Errorf("Unexpected error %s", err) + } + if d := cmp.Diff(tc.want, got, resourceQuantityCmp, equateEmpty()); d != "" { + t.Errorf("Unexpected output for virtual limit range: %s", d) + } + }) + } +} + +var resourceQuantityCmp = cmp.Comparer(func(x, y resource.Quantity) bool { + return x.Cmp(y) == 0 +}) + +func equateAlways(_, _ interface{}) bool { return true } + +func equateEmpty() cmp.Option { + return cmp.FilterValues(func(x, y corev1.ResourceList) bool { return isEmpty(x) && isEmpty(y) }, cmp.Comparer(equateAlways)) +} + +func isEmpty(x corev1.ResourceList) bool { + if len(x) == 0 { + return true + } + for _, q := range x { + if !q.IsZero() { + return false + } + } + return true +} + +func createResourceList(multiple int) corev1.ResourceList { + cpuStr := strconv.Itoa(multiple*100) + "m" + memStr := strconv.Itoa(multiple*10) + "Mi" + ephemeralStr := strconv.Itoa(multiple*100) + "Mi" + return corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(cpuStr), + corev1.ResourceMemory: resource.MustParse(memStr), + corev1.ResourceEphemeralStorage: resource.MustParse(ephemeralStr), + } +} diff --git a/pkg/internal/limitrange/transformer.go b/pkg/internal/computeresources/transformer.go similarity index 55% rename from pkg/internal/limitrange/transformer.go rename to pkg/internal/computeresources/transformer.go index 90d55d2ec9b..ae14217b057 100644 --- a/pkg/internal/limitrange/transformer.go +++ b/pkg/internal/computeresources/transformer.go @@ -14,11 +14,12 @@ See the License for the specific language governing permissions and limitations under the License. */ -package limitrange +package computeresources import ( "context" + "github.com/tektoncd/pipeline/pkg/internal/computeresources/limitrange" "github.com/tektoncd/pipeline/pkg/pod" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -27,72 +28,79 @@ import ( var resourceNames = []corev1.ResourceName{corev1.ResourceCPU, corev1.ResourceMemory, corev1.ResourceEphemeralStorage} -func isZero(q resource.Quantity) bool { - return (&q).IsZero() -} - // NewTransformer returns a pod.Transformer that will modify limits if needed func NewTransformer(ctx context.Context, namespace string, lister corev1listers.LimitRangeLister) pod.Transformer { return func(p *corev1.Pod) (*corev1.Pod, error) { - limitRange, err := getVirtualLimitRange(ctx, namespace, lister) + limitRange, err := limitrange.GetVirtualLimitRange(ctx, namespace, lister) if err != nil { return p, err } - // No LimitRange defined, nothing to transform, bail early we don't have anything to transform. - if limitRange == nil { - return p, nil - } + return transformPodBasedOnLimitRange(p, limitRange), nil + } +} - // The assumption here is that the min, max, default, ratio have already been - // computed if there is multiple LimitRange to satisfy the most (if we can). - // Count the number of containers (that we know) in the Pod. - // This should help us find the smallest request to apply to containers - nbContainers := len(p.Spec.Containers) - // FIXME(#4230) maxLimitRequestRatio to support later - defaultLimits := getDefaultLimits(limitRange) - defaultInitRequests := getDefaultInitContainerRequest(limitRange) - for i := range p.Spec.InitContainers { - // We are trying to set the smallest requests possible - if p.Spec.InitContainers[i].Resources.Requests == nil { - p.Spec.InitContainers[i].Resources.Requests = defaultInitRequests - } else { - for _, name := range resourceNames { - setRequestsOrLimits(name, p.Spec.InitContainers[i].Resources.Requests, defaultInitRequests) - } +// transformPodBasedOnLimitRange modifies the pod's containers' resource requirements to meet the constraints of the LimitRange. +// The only supported type of LimitRange is "Container". +// For any container: +// - If the container has requests, they are set to the max of (requests, limitRange minimum). +// - If the container doesn't have requests, they are set to the max of (limitRange minimum, "default"), +// where "default" is the LimitRange defaultRequest (for init containers) or the LimitRange defaultRequest / # of app containers +// (for app containers). +// - If the container has limits, they are set to the min of (limits, limitRange maximum). +// - If the container doesn't have limits, they are set to the min of (limitRange maximum, limitRange default). +func transformPodBasedOnLimitRange(p *corev1.Pod, limitRange *corev1.LimitRange) *corev1.Pod { + // No LimitRange defined, nothing to transform, bail early we don't have anything to transform. + if limitRange == nil { + return p + } + + // Count the number of containers (that we know) in the Pod. + // This should help us find the smallest request to apply to containers + nbContainers := len(p.Spec.Containers) + // FIXME(#4230) maxLimitRequestRatio to support later + defaultLimits := getDefaultLimits(limitRange) + defaultInitRequests := getDefaultInitContainerRequest(limitRange) + for i := range p.Spec.InitContainers { + // We are trying to set the smallest requests possible + if p.Spec.InitContainers[i].Resources.Requests == nil { + p.Spec.InitContainers[i].Resources.Requests = defaultInitRequests + } else { + for _, name := range resourceNames { + setRequestsOrLimits(name, p.Spec.InitContainers[i].Resources.Requests, defaultInitRequests) } - // We are trying to set the highest limits possible - if p.Spec.InitContainers[i].Resources.Limits == nil { - p.Spec.InitContainers[i].Resources.Limits = defaultLimits - } else { - for _, name := range resourceNames { - setRequestsOrLimits(name, p.Spec.InitContainers[i].Resources.Limits, defaultLimits) - } + } + // We are trying to set the highest limits possible + if p.Spec.InitContainers[i].Resources.Limits == nil { + p.Spec.InitContainers[i].Resources.Limits = defaultLimits + } else { + for _, name := range resourceNames { + setRequestsOrLimits(name, p.Spec.InitContainers[i].Resources.Limits, defaultLimits) } } + } - defaultRequests := getDefaultAppContainerRequest(limitRange, nbContainers) - for i := range p.Spec.Containers { - if p.Spec.Containers[i].Resources.Requests == nil { - p.Spec.Containers[i].Resources.Requests = defaultRequests - } else { - for _, name := range resourceNames { - setRequestsOrLimits(name, p.Spec.Containers[i].Resources.Requests, defaultRequests) - } + defaultRequests := getDefaultAppContainerRequest(limitRange, nbContainers) + for i := range p.Spec.Containers { + if p.Spec.Containers[i].Resources.Requests == nil { + p.Spec.Containers[i].Resources.Requests = defaultRequests + } else { + for _, name := range resourceNames { + setRequestsOrLimits(name, p.Spec.Containers[i].Resources.Requests, defaultRequests) } - if p.Spec.Containers[i].Resources.Limits == nil { - p.Spec.Containers[i].Resources.Limits = defaultLimits - } else { - for _, name := range resourceNames { - setRequestsOrLimits(name, p.Spec.Containers[i].Resources.Limits, defaultLimits) - } + } + if p.Spec.Containers[i].Resources.Limits == nil { + p.Spec.Containers[i].Resources.Limits = defaultLimits + } else { + for _, name := range resourceNames { + setRequestsOrLimits(name, p.Spec.Containers[i].Resources.Limits, defaultLimits) } } - return p, nil } + return p } func setRequestsOrLimits(name corev1.ResourceName, dst, src corev1.ResourceList) { - if isZero(dst[name]) && !isZero(src[name]) { + if limitrange.IsZero(dst[name]) && !limitrange.IsZero(src[name]) { dst[name] = src[name] } } diff --git a/pkg/internal/limitrange/transformer_test.go b/pkg/internal/computeresources/transformer_test.go similarity index 79% rename from pkg/internal/limitrange/transformer_test.go rename to pkg/internal/computeresources/transformer_test.go index 47a5cc18646..7a42e2c4d2c 100644 --- a/pkg/internal/limitrange/transformer_test.go +++ b/pkg/internal/computeresources/transformer_test.go @@ -13,22 +13,16 @@ 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 limitrange +package computeresources import ( - "context" "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" - 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 { @@ -401,22 +395,12 @@ func TestTransformerOneContainer(t *testing.T) { }, }} { t.Run(tc.description, func(t *testing.T) { - ctx, 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, - }, - }}, - ) - defer cancel() - f := NewTransformer(ctx, "default", fakelimitrangeinformer.Get(ctx).Lister()) - got, err := f(&corev1.Pod{ - Spec: tc.podspec, - }) - if err != nil { - t.Fatalf("Transformer failed: %v", err) - } + pod := corev1.Pod{Spec: tc.podspec} + limitRange := corev1.LimitRange{ObjectMeta: metav1.ObjectMeta{Name: "limitrange", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: tc.limitranges, + }} + got := transformPodBasedOnLimitRange(&pod, &limitRange) // We only care about the request and limit, ignoring the rest of the spec cmpRequestsAndLimits(t, tc.want, got.Spec) }) @@ -807,145 +791,12 @@ func TestTransformerMultipleContainer(t *testing.T) { }, }} { t.Run(tc.description, func(t *testing.T) { - ctx, 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, - }, - }}, - ) - defer cancel() - f := NewTransformer(ctx, "default", fakelimitrangeinformer.Get(ctx).Lister()) - got, err := f(&corev1.Pod{ - Spec: tc.podspec, - }) - if err != nil { - t.Fatalf("Transformer failed: %v", err) - } - // We only care about the request and limit, ignoring the rest of the spec - cmpRequestsAndLimits(t, tc.want, got.Spec) - }) - } -} - -func TestTransformerOneContainerMultipleLimitRange(t *testing.T) { - for _, tc := range []struct { - description string - limitranges []corev1.LimitRange - podspec corev1.PodSpec - want corev1.PodSpec - }{{ - description: "limitRange with default requests and limits, min and max and no resources on containers", - limitranges: []corev1.LimitRange{{ - ObjectMeta: metav1.ObjectMeta{Name: "limitrange1", Namespace: "default"}, - Spec: corev1.LimitRangeSpec{ - Limits: []corev1.LimitRangeItem{{ - Type: corev1.LimitTypeContainer, - Max: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("3"), - corev1.ResourceMemory: resource.MustParse("300Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("3Gi"), - }, - Min: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("700m"), - corev1.ResourceMemory: resource.MustParse("70Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("700Mi"), - }, - Default: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("2"), - corev1.ResourceMemory: resource.MustParse("200Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"), - }, - DefaultRequest: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1"), - corev1.ResourceMemory: resource.MustParse("100Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"), - }, - }}, - }, - }, { - ObjectMeta: metav1.ObjectMeta{Name: "limitrange2", Namespace: "default"}, - Spec: corev1.LimitRangeSpec{ - Limits: []corev1.LimitRangeItem{{ - Type: corev1.LimitTypeContainer, - Max: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("4"), - corev1.ResourceMemory: resource.MustParse("400Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("4Gi"), - }, - Min: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("800m"), - corev1.ResourceMemory: resource.MustParse("80Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("800Mi"), - }, - Default: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("3"), - corev1.ResourceMemory: resource.MustParse("300Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("3Gi"), - }, - DefaultRequest: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1500m"), - corev1.ResourceMemory: resource.MustParse("150Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("1.5Gi"), - }, - }}, - }, - }}, - podspec: corev1.PodSpec{ - InitContainers: []corev1.Container{{ - Name: "bar", - Image: "foo", - }}, - Containers: []corev1.Container{{ - Name: "foo", - Image: "baz", - }}, - }, - want: corev1.PodSpec{ - InitContainers: []corev1.Container{{ - Resources: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("2"), - corev1.ResourceMemory: resource.MustParse("200Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"), - }, - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1"), - corev1.ResourceMemory: resource.MustParse("100Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"), - }, - }, - }}, - Containers: []corev1.Container{{ - Resources: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("2"), - corev1.ResourceMemory: resource.MustParse("200Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"), - }, - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1"), - corev1.ResourceMemory: resource.MustParse("100Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"), - }, - }, - }}, - }, - }} { - t.Run(tc.description, func(t *testing.T) { - ctx, cancel := setup(t, - []corev1.ServiceAccount{{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}}}, - tc.limitranges, - ) - defer cancel() - f := NewTransformer(ctx, "default", fakelimitrangeinformer.Get(ctx).Lister()) - got, err := f(&corev1.Pod{ - Spec: tc.podspec, - }) - if err != nil { - t.Fatalf("Transformer failed: %v", err) - } + pod := corev1.Pod{Spec: tc.podspec} + limitRange := corev1.LimitRange{ObjectMeta: metav1.ObjectMeta{Name: "limitrange", Namespace: "default"}, + Spec: corev1.LimitRangeSpec{ + Limits: tc.limitranges, + }} + got := transformPodBasedOnLimitRange(&pod, &limitRange) // We only care about the request and limit, ignoring the rest of the spec cmpRequestsAndLimits(t, tc.want, got.Spec) }) @@ -975,27 +826,3 @@ func cmpRequestsAndLimits(t *testing.T, want, got corev1.PodSpec) { } } } - -func setup(t *testing.T, serviceaccounts []corev1.ServiceAccount, limitranges []corev1.LimitRange) (context.Context, 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, cancel -} diff --git a/pkg/internal/limitrange/doc.go b/pkg/internal/limitrange/doc.go deleted file mode 100644 index 05b2449826e..00000000000 --- a/pkg/internal/limitrange/doc.go +++ /dev/null @@ -1,18 +0,0 @@ -/* -Copyright 2020 The Tekton 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 limitrange defines logic for supporting Kubernetes LimitRange for the specific Tekton use cases -package limitrange diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index ddff5bf6967..2a4e7cb5c52 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -36,7 +36,7 @@ import ( listers "github.com/tektoncd/pipeline/pkg/client/listers/pipeline/v1beta1" resourcelisters "github.com/tektoncd/pipeline/pkg/client/resource/listers/resource/v1alpha1" "github.com/tektoncd/pipeline/pkg/internal/affinityassistant" - "github.com/tektoncd/pipeline/pkg/internal/limitrange" + "github.com/tektoncd/pipeline/pkg/internal/computeresources" podconvert "github.com/tektoncd/pipeline/pkg/pod" tknreconciler "github.com/tektoncd/pipeline/pkg/reconciler" "github.com/tektoncd/pipeline/pkg/reconciler/events" @@ -707,7 +707,7 @@ func (c *Reconciler) createPod(ctx context.Context, ts *v1beta1.TaskSpec, tr *v1 EntrypointCache: c.entrypointCache, } pod, err := podbuilder.Build(ctx, tr, *ts, - limitrange.NewTransformer(ctx, tr.Namespace, c.limitrangeLister), + computeresources.NewTransformer(ctx, tr.Namespace, c.limitrangeLister), affinityassistant.NewTransformer(ctx, tr.Annotations), ) if err != nil {