diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 66dd1fa27ce..f8827b444dd 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -20,7 +20,7 @@ Creation of a `PipelineRun` will trigger the creation of - [Cancelling a PipelineRun](#cancelling-a-pipelinerun) - [Examples](https://github.com/tektoncd/pipeline/tree/master/examples/pipelineruns) - [Logs](logs.md) -- [LimitRange Name](#limitrange-name) +- [LimitRanges](#limitrange-name) ## Syntax @@ -51,10 +51,6 @@ following fields: follow the instruction [here](taskruns.md#Configuring-default-timeout) to configure the default timeout, the same way as `TaskRun`. - [`podTemplate`](#pod-template) - Specifies a [pod template](./podtemplates.md) that will be used as the basis for the `Task` pod. - - [`limitRangeName`](#limitrange-name) - Specifies the name of a LimitRange that exists in the namespace of the `PipelineRun`. This LimitRange's minimum - for container resource requests will be used as part of requesting the appropriate amount of CPU, memory, and ephemeral storage for containers that are - part of the `TaskRuns` of a `PipelineRun`. This property only needs to be specified if the `PipelineRun` is happening in a namespace with a LimitRange - minimum specified for container resource requests. [kubernetes-overview]: https://kubernetes.io/docs/concepts/overview/working-with-objects/kubernetes-objects/#required-fields @@ -377,7 +373,7 @@ spec: status: "PipelineRunCancelled" ``` -## LimitRange Name +## LimitRanges In order to request the minimum amount of resources needed to support the containers for `steps` that are part of a `TaskRun`, Tekton only requests the maximum values for CPU, @@ -387,28 +383,10 @@ All requests that are not the max values are set to zero as a result. When a [LimitRange](https://kubernetes.io/docs/concepts/policy/limit-range/) is present in a namespace with a minimum set for container resource requests (i.e. CPU, memory, and ephemeral storage) where `PipelineRuns` -are attempting to run, the `limitRangeName` property must be specified to appropriately apply the LimitRange's -minimum values to containers that are part of a `TaskRun` associated with a `PipelineRun`. This property helps -prevent failures of `TaskRuns` not meeting the minimum requirements specified by a LimitRange for containers. +are attempting to run, Tekton will search through all LimitRanges present in the namespace and use the minimum +set for container resource requests instead of requesting 0. -In the example below, the LimitRange `limit-mem-cpu-per-container` will be applied to all `TaskRuns` associated -with the `PipelineRun`: - -```yaml -apiVersion: tekton.dev/v1alpha1 -kind: PipelineRun -metadata: - creationTimestamp: null - generateName: deploy-pipeline-run- - namespace: default -spec: - pipelineRef: - name: deploy-pipeline-hello - limitRangeName: "limit-mem-cpu-per-container" -status: {} -``` - -An example `PipelineRun` using `limitRangeName` is available [here](../examples/pipelineruns/no-ci/limitrange.yaml). +An example `PipelineRun` with a LimitRange is available [here](../examples/pipelineruns/no-ci/limitrange.yaml). --- diff --git a/docs/taskruns.md b/docs/taskruns.md index 9fb49e8f46c..64893e7ab9c 100644 --- a/docs/taskruns.md +++ b/docs/taskruns.md @@ -26,7 +26,7 @@ A `TaskRun` runs until all `steps` have completed or until a failure occurs. - [Examples](#examples) - [Sidecars](#sidecars) - [Logs](logs.md) -- [LimitRange Name](#limitrange-name) +- [LimitRanges](#limitrange-name) --- @@ -61,9 +61,6 @@ following fields: - [`podTemplate`](#pod-template) - Specifies a [pod template](./podtemplates.md) that will be used as the basis for the `Task` pod. - [`workspaces`](#workspaces) - Specify the actual volumes to use for the [workspaces](tasks.md#workspaces) declared by a `Task` - - [`limitRangeName`](#limitrange-name) - Specifies the name of a LimitRange that exists in the namespace of the `TaskRun`. This LimitRange's minimum - for container resource requests will be used as part of requesting the appropriate amount of CPU, memory, and ephemeral storage for containers that are - part of a `TaskRun`. This property only needs to be specified if the `TaskRun` is happening in a namespace with a LimitRange minimum specified for container resource requests. [kubernetes-overview]: https://kubernetes.io/docs/concepts/overview/working-with-objects/kubernetes-objects/#required-fields @@ -704,7 +701,7 @@ with the `get pods` command. The Pod description will instead show a Status of Failed and the individual container statuses will correctly reflect how and why they exited. -## LimitRange Name +## LimitRanges In order to request the minimum amount of resources needed to support the containers for `steps` that are part of a `TaskRun`, Tekton only requests the maximum values for CPU, @@ -714,31 +711,10 @@ All requests that are not the max values are set to zero as a result. When a [LimitRange](https://kubernetes.io/docs/concepts/policy/limit-range/) is present in a namespace with a minimum set for container resource requests (i.e. CPU, memory, and ephemeral storage) where `TaskRuns` -are attempting to run, the `limitRangeName` property must be specified to appropriately apply the LimitRange's -minimum values to `steps` that are part of a `TaskRun`. This property helps prevent failures of `TaskRuns` not -meeting the minimum requirements specified by a LimitRange for containers. +are attempting to run, Tekton will search through all LimitRanges present in the namespace and use the minimum +set for container resource requests instead of requesting 0. -In the example below, the LimitRange `limit-mem-cpu-per-container` will be applied to all `steps` associated with -a `TaskRun` in namespace `default`: - -```yaml -apiVersion: tekton.dev/v1alpha1 -kind: TaskRun -metadata: - creationTimestamp: null - generateName: echo-hello-world-run- - namespace: default -spec: - inputs: {} - outputs: {} - serviceAccountName: "" - taskRef: - name: echo-hello-world - timeout: 1h0m0s - limitRangeName: "limit-mem-cpu-per-container" -``` - -An example `TaskRun` using `limitRangeName` is available [here](../examples/taskruns/no-ci/limitrange.yaml). +An example `TaskRun` with a LimitRange is available [here](../examples/taskruns/no-ci/limitrange.yaml). --- diff --git a/examples/pipelineruns/no-ci/limitrange.yaml b/examples/pipelineruns/no-ci/limitrange.yaml index e1958376c3b..3cca0f9b16a 100644 --- a/examples/pipelineruns/no-ci/limitrange.yaml +++ b/examples/pipelineruns/no-ci/limitrange.yaml @@ -53,5 +53,4 @@ metadata: spec: pipelineRef: name: pipeline-hello - limitRangeName: "limit-mem-cpu-per-container" status: {} \ No newline at end of file diff --git a/examples/taskruns/no-ci/limitrange.yaml b/examples/taskruns/no-ci/limitrange.yaml index 4148b4ada22..a35e7c23bb7 100644 --- a/examples/taskruns/no-ci/limitrange.yaml +++ b/examples/taskruns/no-ci/limitrange.yaml @@ -42,6 +42,5 @@ spec: taskRef: name: echo-hello-world timeout: 1h0m0s - limitRangeName: "limit-mem-cpu-per-container" status: podName: "" \ No newline at end of file diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_conversion.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_conversion.go index b70bd3e36a6..83544af1db8 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_conversion.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_conversion.go @@ -58,7 +58,6 @@ func (source *PipelineRunSpec) ConvertUp(ctx context.Context, sink *v1alpha2.Pip sink.Timeout = source.Timeout sink.PodTemplate = source.PodTemplate sink.Workspaces = source.Workspaces - sink.LimitRangeName = source.LimitRangeName return nil } @@ -93,6 +92,5 @@ func (sink *PipelineRunSpec) ConvertDown(ctx context.Context, source *v1alpha2.P sink.Timeout = source.Timeout sink.PodTemplate = source.PodTemplate sink.Workspaces = source.Workspaces - sink.LimitRangeName = source.LimitRangeName return nil } diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_conversion_test.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_conversion_test.go index 3e31d4bc67e..90d089dd875 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_conversion_test.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_conversion_test.go @@ -73,7 +73,6 @@ func TestPipelineRunConversion(t *testing.T) { SubPath: "foo", EmptyDir: &corev1.EmptyDirVolumeSource{}, }}, - LimitRangeName: "foo", Params: []Param{{ Name: "p1", Value: v1alpha2.ArrayOrString{StringVal: "baz"}, @@ -134,7 +133,6 @@ func TestPipelineRunConversion(t *testing.T) { SubPath: "foo", EmptyDir: &corev1.EmptyDirVolumeSource{}, }}, - LimitRangeName: "foo", Params: []Param{{ Name: "p1", Value: v1alpha2.ArrayOrString{StringVal: "baz"}, diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go index 842dade553d..60326548119 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go @@ -87,12 +87,6 @@ type PipelineRunSpec struct { // with those declared in the pipeline. // +optional Workspaces []WorkspaceBinding `json:"workspaces,omitempty"` - // Used to specify name of LimitRange that exists in namespace - // where PipelineRun will run so that the LimitRange's minimum for - // container requests can be used by containers of TaskRuns associated - // with PipelineRun - // +optional - LimitRangeName string `json:"limitRangeName"` } // PipelineRunSpecStatus defines the pipelinerun spec status the user can provide diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_conversion.go b/pkg/apis/pipeline/v1alpha1/taskrun_conversion.go index 23b91eed756..3cbeddd214e 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_conversion.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_conversion.go @@ -55,7 +55,6 @@ func (source *TaskRunSpec) ConvertUp(ctx context.Context, sink *v1alpha2.TaskRun sink.Timeout = source.Timeout sink.PodTemplate = source.PodTemplate sink.Workspaces = source.Workspaces - sink.LimitRangeName = source.LimitRangeName sink.Params = source.Params sink.Resources = source.Resources // Deprecated fields @@ -140,7 +139,6 @@ func (sink *TaskRunSpec) ConvertDown(ctx context.Context, source *v1alpha2.TaskR sink.Timeout = source.Timeout sink.PodTemplate = source.PodTemplate sink.Workspaces = source.Workspaces - sink.LimitRangeName = source.LimitRangeName sink.Params = source.Params sink.Resources = source.Resources return nil diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_conversion_test.go b/pkg/apis/pipeline/v1alpha1/taskrun_conversion_test.go index 5a7668844a3..371ac2d2052 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_conversion_test.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_conversion_test.go @@ -69,7 +69,6 @@ func TestTaskRunConversion(t *testing.T) { SubPath: "foo", EmptyDir: &corev1.EmptyDirVolumeSource{}, }}, - LimitRangeName: "foo", Params: []Param{{ Name: "p1", Value: v1alpha2.ArrayOrString{StringVal: "baz"}, @@ -125,7 +124,6 @@ func TestTaskRunConversion(t *testing.T) { SubPath: "foo", EmptyDir: &corev1.EmptyDirVolumeSource{}, }}, - LimitRangeName: "foo", Params: []Param{{ Name: "p1", Value: v1alpha2.ArrayOrString{StringVal: "baz"}, @@ -171,7 +169,6 @@ func TestTaskRunConversion(t *testing.T) { SubPath: "foo", EmptyDir: &corev1.EmptyDirVolumeSource{}, }}, - LimitRangeName: "foo", Params: []Param{{ Name: "p1", Value: v1alpha2.ArrayOrString{StringVal: "baz"}, @@ -209,7 +206,6 @@ func TestTaskRunConversion(t *testing.T) { SubPath: "foo", EmptyDir: &corev1.EmptyDirVolumeSource{}, }}, - LimitRangeName: "foo", Resources: &v1alpha2.TaskRunResources{ Inputs: []v1alpha2.TaskResourceBinding{{ PipelineResourceBinding: v1alpha2.PipelineResourceBinding{ @@ -255,7 +251,6 @@ func TestTaskRunConversion(t *testing.T) { SubPath: "foo", EmptyDir: &corev1.EmptyDirVolumeSource{}, }}, - LimitRangeName: "foo", Resources: &v1alpha2.TaskRunResources{ Outputs: []v1alpha2.TaskResourceBinding{{ PipelineResourceBinding: v1alpha2.PipelineResourceBinding{ diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_types.go b/pkg/apis/pipeline/v1alpha1/taskrun_types.go index 67e61f1f2c0..90be2f90448 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_types.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_types.go @@ -49,18 +49,11 @@ type TaskRunSpec struct { // Workspaces is a list of WorkspaceBindings from volumes to workspaces. // +optional Workspaces []WorkspaceBinding `json:"workspaces,omitempty"` - // Used to specify name of LimitRange that exists in namespace - // where TaskRun will run so that the LimitRange's minimum for - // container requests can be used by containers of TaskRun - // +optional - LimitRangeName string `json:"limitRangeName"` - // From v1alpha2 // +optional Params []Param `json:"params,omitempty"` // +optional Resources *v1alpha2.TaskRunResources `json:"resources,omitempty"` - // Deprecated // +optional Inputs TaskRunInputs `json:"inputs,omitempty"` diff --git a/pkg/apis/pipeline/v1alpha2/pipelinerun_types.go b/pkg/apis/pipeline/v1alpha2/pipelinerun_types.go index 8afb5267ffc..d5790b9f203 100644 --- a/pkg/apis/pipeline/v1alpha2/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1alpha2/pipelinerun_types.go @@ -155,12 +155,6 @@ type PipelineRunSpec struct { // with those declared in the pipeline. // +optional Workspaces []WorkspaceBinding `json:"workspaces,omitempty"` - // Used to specify name of LimitRange that exists in namespace - // where PipelineRun will run so that the LimitRange's minimum for - // container requests can be used by containers of TaskRuns associated - // with PipelineRun - // +optional - LimitRangeName string `json:"limitRangeName"` } // PipelineRunSpecStatus defines the pipelinerun spec status the user can provide diff --git a/pkg/apis/pipeline/v1alpha2/taskrun_types.go b/pkg/apis/pipeline/v1alpha2/taskrun_types.go index b41df6efa13..d15985b8bd0 100644 --- a/pkg/apis/pipeline/v1alpha2/taskrun_types.go +++ b/pkg/apis/pipeline/v1alpha2/taskrun_types.go @@ -53,11 +53,6 @@ type TaskRunSpec struct { // Workspaces is a list of WorkspaceBindings from volumes to workspaces. // +optional Workspaces []WorkspaceBinding `json:"workspaces,omitempty"` - // Used to specify name of LimitRange that exists in namespace - // where TaskRun will run so that the LimitRange's minimum for - // container requests can be used by containers of TaskRun - // +optional - LimitRangeName string `json:"limitRangeName"` } // TaskRunSpecStatus defines the taskrun spec status the user can provide diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index f17004b508f..b28eaeb26d4 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -119,18 +119,13 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha initContainers = append(initContainers, entrypointInit) volumes = append(volumes, toolsVolume, downwardVolume) - // If present on TaskRunSpec, use LimitRangeName to get LimitRange - // so it can be used in resolveResourceRequests - var limitRange *corev1.LimitRange - if taskRun.Spec.LimitRangeName != "" { - limitRange, err = kubeclient.CoreV1().LimitRanges(taskRun.Namespace).Get(taskRun.Spec.LimitRangeName, metav1.GetOptions{}) - if err != nil { - return nil, err - } + limitRangeMin, err := getLimitRangeMinimum(taskRun.Namespace, kubeclient) + if err != nil { + return nil, err } // Zero out non-max resource requests. - stepContainers = resolveResourceRequests(stepContainers, limitRange) + stepContainers = resolveResourceRequests(stepContainers, limitRangeMin) // Add implicit env vars. // They're prepended to the list, so that if the user specified any @@ -262,3 +257,33 @@ func MakeLabels(s *v1alpha1.TaskRun) map[string]string { labels[taskRunLabelKey] = s.Name return labels } + +// getLimitRangeMinimum gets all LimitRanges in a namespace and +// searches for if a container minimum is specified. Due to +// https://github.com/kubernetes/kubernetes/issues/79496, the +// max LimitRange minimum must be found in the event of conflicting +// container minimums specified. +func getLimitRangeMinimum(namespace string, kubeclient kubernetes.Interface) (corev1.ResourceList, error) { + limitRanges, err := kubeclient.CoreV1().LimitRanges(namespace).List(metav1.ListOptions{}) + if err != nil { + return nil, err + } + + min := allZeroQty() + for _, lr := range limitRanges.Items { + lrItems := lr.Spec.Limits + for _, lrItem := range lrItems { + if lrItem.Type == corev1.LimitTypeContainer { + if lrItem.Min != nil { + for k, v := range lrItem.Min { + if v.Cmp(min[k]) > 0 { + min[k] = v + } + } + } + } + } + } + + return min, nil +} diff --git a/pkg/pod/resource_request.go b/pkg/pod/resource_request.go index 7bfe84b0112..dc204364219 100644 --- a/pkg/pod/resource_request.go +++ b/pkg/pod/resource_request.go @@ -32,7 +32,7 @@ func allZeroQty() corev1.ResourceList { } } -func resolveResourceRequests(containers []corev1.Container, limitRange *corev1.LimitRange) []corev1.Container { +func resolveResourceRequests(containers []corev1.Container, limitRangeMin corev1.ResourceList) []corev1.Container { max := allZeroQty() resourceNames := []corev1.ResourceName{corev1.ResourceCPU, corev1.ResourceMemory, corev1.ResourceEphemeralStorage} maxIndicesByResource := make(map[corev1.ResourceName]int, len(resourceNames)) @@ -51,43 +51,27 @@ func resolveResourceRequests(containers []corev1.Container, limitRange *corev1.L } } - // Get limitrange minimum for container requests so they won't - // be zeroed out if minimum is specified in namespace - var limitRangeItems []corev1.LimitRangeItem - if limitRange != nil { - limitRangeItems = limitRange.Spec.Limits - } - min := allZeroQty() - for _, limitRangeItem := range limitRangeItems { - if limitRangeItem.Type == corev1.LimitTypeContainer { - if limitRangeItem.Min != nil { - min = limitRangeItem.Min - } - break - } - } - // Use zeroQty if request value is not set for min - if min[corev1.ResourceCPU] == emptyResourceQuantity { - min[corev1.ResourceCPU] = zeroQty + if limitRangeMin[corev1.ResourceCPU] == emptyResourceQuantity { + limitRangeMin[corev1.ResourceCPU] = zeroQty } - if min[corev1.ResourceMemory] == emptyResourceQuantity { - min[corev1.ResourceMemory] = zeroQty + if limitRangeMin[corev1.ResourceMemory] == emptyResourceQuantity { + limitRangeMin[corev1.ResourceMemory] = zeroQty } - if min[corev1.ResourceEphemeralStorage] == emptyResourceQuantity { - min[corev1.ResourceEphemeralStorage] = zeroQty + if limitRangeMin[corev1.ResourceEphemeralStorage] == emptyResourceQuantity { + limitRangeMin[corev1.ResourceEphemeralStorage] = zeroQty } // Set all non max resource requests to 0. Leave max request at index // originally defined to account for limit of step. for i := range containers { if containers[i].Resources.Requests == nil { - containers[i].Resources.Requests = min + containers[i].Resources.Requests = limitRangeMin continue } for _, resourceName := range resourceNames { if maxIndicesByResource[resourceName] != i { - containers[i].Resources.Requests[resourceName] = min[resourceName] + containers[i].Resources.Requests[resourceName] = limitRangeMin[resourceName] } } } diff --git a/pkg/pod/resource_request_test.go b/pkg/pod/resource_request_test.go index 990960f87af..78168080af7 100644 --- a/pkg/pod/resource_request_test.go +++ b/pkg/pod/resource_request_test.go @@ -206,7 +206,7 @@ func TestResolveResourceRequests_No_LimitRange(t *testing.T) { }, } { t.Run(c.desc, func(t *testing.T) { - got := resolveResourceRequests(c.in, nil) + got := resolveResourceRequests(c.in, allZeroQty()) if d := cmp.Diff(c.want, got, resourceQuantityCmp); d != "" { t.Errorf("Diff(-want, +got): %s", d) } @@ -331,21 +331,12 @@ func TestResolveResourceRequests_LimitRange(t *testing.T) { }, } { t.Run(c.desc, func(t *testing.T) { - got := resolveResourceRequests(c.in, - &corev1.LimitRange{ - Spec: corev1.LimitRangeSpec{ - Limits: []corev1.LimitRangeItem{ - { - Type: "Container", - Min: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("100m"), - corev1.ResourceMemory: resource.MustParse("99Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("100m"), - }, - }, - }, - }, - }) + got := resolveResourceRequests(c.in, corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("99Mi"), + corev1.ResourceEphemeralStorage: resource.MustParse("100m"), + }, + ) if d := cmp.Diff(c.want, got, resourceQuantityCmp); d != "" { t.Errorf("Diff(-want, +got): %s", d) } diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index f7da196712e..75da4ef0875 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -562,7 +562,6 @@ func (c *Reconciler) createTaskRun(rprt *resources.ResolvedPipelineRunTask, pr * ServiceAccountName: pr.GetServiceAccountName(rprt.PipelineTask.Name), Timeout: getTaskRunTimeout(pr), PodTemplate: pr.Spec.PodTemplate, - LimitRangeName: pr.Spec.LimitRangeName, }} if rprt.ResolvedTaskResources.TaskName != "" {