From 1230d84b2f9236ded6b8f1f78044ce6df1a06964 Mon Sep 17 00:00:00 2001 From: Austin Zhao Date: Thu, 12 May 2022 01:00:14 +0000 Subject: [PATCH] Support Task-level Resources Requirements: Part #1 Required fields and related webhook validations are added to support a user to configure the computing resources per Task/Pod which significantly reduces the over-asked amount configured by the step-level. --- docs/compute-resources.md | 257 ++++++++++++++++++ .../pipeline/v1beta1/openapi_generated.go | 77 +++++- .../pipeline/v1beta1/pipelinerun_types.go | 6 + .../v1beta1/pipelinerun_validation.go | 15 + .../v1beta1/pipelinerun_validation_test.go | 50 +++- pkg/apis/pipeline/v1beta1/resource_types.go | 26 +- pkg/apis/pipeline/v1beta1/swagger.json | 41 ++- pkg/apis/pipeline/v1beta1/task_validation.go | 37 +++ .../pipeline/v1beta1/task_validation_test.go | 137 +++++++++- .../pipeline/v1beta1/taskrun_validation.go | 22 ++ .../v1beta1/taskrun_validation_test.go | 43 ++- .../pipeline/v1beta1/zz_generated.deepcopy.go | 29 ++ 12 files changed, 725 insertions(+), 15 deletions(-) diff --git a/docs/compute-resources.md b/docs/compute-resources.md index 87559ac930c..e93c50b92e0 100644 --- a/docs/compute-resources.md +++ b/docs/compute-resources.md @@ -52,6 +52,263 @@ requirements for `Step`containers, they must be treated as if they are running i Tekton adjusts `Step` resource requirements to comply with [LimitRanges](#limitrange-support). [ResourceQuotas](#resourcequota-support) are not currently supported. +Instead of specifying resource requirements on each `Step`, users can choose to specify resource requirements at the Task-level. If users specify a Task-level resource request, it will ensure that the kubelet reserves only that amount of resources to execute the `Task`'s `Steps`. +If users specify a Task-level resource limit, no `Step` may use more than that amount of resources. + +Each of these details is explained in more depth below. + +Some points to note: + +- Task-level resource requests and limits do not apply to sidecars which can be configured separately. +- Users may not configure the Task-level and Step-level resource requirements (requests/limits) simultaneously. + +### Configuring Task-level Resource Requirements + +Task-level resource requirements can be configured in `Task.Resources`, `TaskRun.Resources`, or `PipelineRun.TaskRunSpecs`. + +e.g. + +```yaml +apiVersion: tekton.dev/v1beta1 +kind: Task +metadata: + name: foo +spec: + steps: + - name: foo1 + - name: foo2 + resources: + requests: + cpu: 1 + limits: + cpu: 2 +``` + +### Specifying Resource Requirement in Task + +Users can specify compute resources either at the Step-level or the Task-level in `Task`. + +To specify compute resources at the Step-level, use `Task.Step` and `Task.StepTemplate`. +To specify compute resources at the Task-level, use `Task.Resources`. + +e.g. + +Using `Task.Resources`: + +```yaml +kind: Task +spec: + resources: + requests: + cpu: 2 +``` + +Rejected when configuring `Task.Step` and `Task.Resources` + +```yaml +kind: Task +spec: + steps: + - name: foo + resources: + requests: + cpu: 1 + resources: + requests: + cpu: 2 +``` + +Rejected when configuring `Task.StepTemplate` and `Task.Resources`: + +```yaml +kind: Task +spec: + steps: + - name: foo + stepTemplate: + resources: + requests: + cpu: 1 + resources: + requests: + cpu: 2 +``` + +### Specifying Resource Requirement in TaskRun/PipelineRun + +Users can specify compute resources either at the Step-level or the Task-level in `TaskRun`. + +To specify compute resources at the Step-level, use `TaskRun.StepOverrides`. +To specify compute resources at the Task-level, use `TaskRun.Resources` or `PipelineRun.TaskRunSpecs`. + +e.g. + +Using `TaskRun.Resources`: + +```yaml +kind: TaskRun +spec: + resources: + requests: + cpu: 2 +``` + +Rejected when configuring `TaskRun.StepOverrides` and `TaskRun.Resources`: + +```yaml +kind: TaskRun +spec: + stepOverrides: + - name: foo + resources: + requests: + cpu: 1 + resources: + requests: + cpu: 2 +``` + +Rejected when configuring `PipelineRun.TaskRunSpecs.StepOverrides` and `PipelineRun.TaskRunSpecs.Resources`: + +```yaml +kind: PipelineRun +spec: + taskRunSpecs: + - pipelineTaskName: foo + stepOverrides: + - name: foo + resources: + requests: + cpu: 1 + resources: + requests: + cpu: 2 +``` + +### Specifying Resource Requirements with LimitRange + +User can use `LimitRange` to configure min or max compute resources for a `Task` or `Step`. + +Here are examples with a task-level resource requirements: + +e.g. + +Applying min: + +```yaml +kind: LimitRange +spec: + limits: + - min: + cpu: 200m +--- +kind: Task +spec: + steps: + - name: step1 # applied with min + - name: step2 # applied with min + - name: step3 # not applied + resources: + requests: + cpu: 1 +``` + +| Step name | CPU request | +| --------- | ----------- | +| step1 | 800m | +| step2 | 200m | +| step3 | N/A | + +Here the 800m on step1 comes from `200m + (1 - 200m * 2)`. + +Applying max: + +```yaml +kind: LimitRange +spec: + limits: + - max: + cpu: 800m +--- +kind: Task +spec: + steps: + - name: step1 # applied with max + - name: step2 # applied with max + - name: step3 # not applied + resources: + requests: + cpu: 1 +``` + +| Step name | CPU request | +| --------- | ----------- | +| step1 | 333m | +| step2 | 333m | +| step3 | 333m | + +Here the 333m comes from `1 / 3` with number rounding to leave decimals out. + +Applying min and max: + +```yaml +kind: LimitRange +spec: + limits: + - min: + cpu: 200m + - max: + cpu: 700m +--- +kind: Task +spec: + steps: + - name: step1 # applied with min and max + - name: step2 # applied with min and max + - name: step3 # not applied + resources: + requests: + cpu: 1 +``` + +| Step name | CPU request | +| --------- | ----------- | +| step1 | 400m | +| step2 | 400m | +| step3 | 200m |   + +Here the 400m comes from the min `200` + the spread out `(1 - 200m * 2) / 3`. + +### Specifying Resource Requirements with Sidecar + +Users can specify compute resources separately for a sidecar while configuring task-level resource requirements. + +e.g. + +```yaml +kind: Task +spec: + steps: + - name: step1 + - name: step2 + sidecars: + - name: sidecar1 + resources: + requests: + cpu: 750m + limits: + cpu: 1 + resources: + requests: + cpu: 2 +``` + +| Step/Sidecar name | CPU request | CPU limit | +| ----------------- | ----------- | --------- | +| step1 | 2 | N/A | +| step2 | N/A | N/A | +| sidecar1 | 750m | 1 | + ## LimitRange Support Kubernetes allows users to configure [LimitRanges]((https://kubernetes.io/docs/concepts/policy/limit-range/)), diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 96c80e7e6d6..9fd104eb880 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -2881,11 +2881,18 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineTaskRunSpec(ref common.ReferenceCa }, }, }, + "resources": { + SchemaProps: spec.SchemaProps{ + Description: "Compute Resources required by the TaskRun. Cannot be updated. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/", + Default: map[string]interface{}{}, + Ref: ref("k8s.io/api/core/v1.ResourceRequirements"), + }, + }, }, }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod.Template", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunSidecarOverride", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunStepOverride"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod.Template", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunSidecarOverride", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunStepOverride", "k8s.io/api/core/v1.ResourceRequirements"}, } } @@ -4294,7 +4301,7 @@ func schema_pkg_apis_pipeline_v1beta1_TaskResources(ref common.ReferenceCallback return common.OpenAPIDefinition{ Schema: spec.Schema{ SchemaProps: spec.SchemaProps{ - Description: "TaskResources allows a Pipeline to declare how its DeclaredPipelineResources should be provided to a Task as its inputs and outputs.", + Description: "TaskResources is overloaded to represent both PipelineResources and compute resources. PipelineResources are deprecated and will be removed, so this field will only represent compute resources in the future.", Type: []string{"object"}, Properties: map[string]spec.Schema{ "inputs": { @@ -4335,11 +4342,41 @@ func schema_pkg_apis_pipeline_v1beta1_TaskResources(ref common.ReferenceCallback }, }, }, + "limits": { + SchemaProps: spec.SchemaProps{ + Description: "Limits describes the maximum amount of compute resources allowed for each Task. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/", + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("k8s.io/apimachinery/pkg/api/resource.Quantity"), + }, + }, + }, + }, + }, + "requests": { + SchemaProps: spec.SchemaProps{ + Description: "Requests describes the minimum amount of compute resources required for each Task. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/", + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("k8s.io/apimachinery/pkg/api/resource.Quantity"), + }, + }, + }, + }, + }, }, }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskResource"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskResource", "k8s.io/apimachinery/pkg/api/resource.Quantity"}, } } @@ -4596,7 +4633,7 @@ func schema_pkg_apis_pipeline_v1beta1_TaskRunResources(ref common.ReferenceCallb return common.OpenAPIDefinition{ Schema: spec.Schema{ SchemaProps: spec.SchemaProps{ - Description: "TaskRunResources allows a TaskRun to declare inputs and outputs TaskResourceBinding", + Description: "TaskRunResources is overloaded to represent both TaskResourceBinding and compute resources.", Type: []string{"object"}, Properties: map[string]spec.Schema{ "inputs": { @@ -4637,11 +4674,41 @@ func schema_pkg_apis_pipeline_v1beta1_TaskRunResources(ref common.ReferenceCallb }, }, }, + "limits": { + SchemaProps: spec.SchemaProps{ + Description: "Limits describes the maximum amount of compute resources allowed for each TaskRun. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/", + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("k8s.io/apimachinery/pkg/api/resource.Quantity"), + }, + }, + }, + }, + }, + "requests": { + SchemaProps: spec.SchemaProps{ + Description: "Requests describes the minimum amount of compute resources required for each TaskRun. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/", + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("k8s.io/apimachinery/pkg/api/resource.Quantity"), + }, + }, + }, + }, + }, }, }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskResourceBinding"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskResourceBinding", "k8s.io/apimachinery/pkg/api/resource.Quantity"}, } } diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go index 31dd1d94ea0..cd9150811a6 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go @@ -20,6 +20,7 @@ import ( "context" "time" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "github.com/tektoncd/pipeline/pkg/apis/config" @@ -600,6 +601,11 @@ type PipelineTaskRunSpec struct { StepOverrides []TaskRunStepOverride `json:"stepOverrides,omitempty"` // +listType=atomic SidecarOverrides []TaskRunSidecarOverride `json:"sidecarOverrides,omitempty"` + // Compute Resources required by the TaskRun. + // Cannot be updated. + // More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + // +optional + Resources corev1.ResourceRequirements `json:"resources,omitempty" protobuf:"bytes,8,opt,name=resources"` } // GetTaskRunSpec returns the task specific spec for a given diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go index 482cda177eb..bebcde053c1 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go @@ -195,6 +195,9 @@ func validateTaskRunSpec(ctx context.Context, trs PipelineTaskRunSpec) (errs *ap if trs.SidecarOverrides != nil { errs = errs.Also(validateSidecarOverrides(trs.SidecarOverrides).ViaField("sidecarOverrides")) } + if trs.Resources.Size() > 0 { + errs = errs.Also(validateResourceRequirements(trs).ViaField("resources")) + } } else { if trs.StepOverrides != nil { errs = errs.Also(apis.ErrDisallowedFields("stepOverrides")) @@ -205,3 +208,15 @@ func validateTaskRunSpec(ctx context.Context, trs PipelineTaskRunSpec) (errs *ap } return errs } + +// validateResourceRequirements validates if only step-level or task-level resource requirements are configured +func validateResourceRequirements(trs PipelineTaskRunSpec) (errs *apis.FieldError) { + for _, taskRunStepOverride := range trs.StepOverrides { + if taskRunStepOverride.Resources.Size() > 0 { + return &apis.FieldError{ + Message: "PipelineTaskRunSpec can't be configured with both step-level(stepOverrides.resources) and task-level(taskRunSpecs.resources) resource requirements", + } + } + } + return nil +} diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go index 6fc9f58c306..05772263244 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go @@ -610,6 +610,29 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { }, wantErr: apis.ErrMissingField("taskRunSpecs[0].sidecarOverrides[0].name"), withContext: enableAlphaAPIFields, + }, { + name: "invalid both step-level(stepOverrides.resources) and task-level(taskRunSpecs.resources) resource requirements configured", + spec: v1beta1.PipelineRunSpec{ + PipelineRef: &v1beta1.PipelineRef{Name: "foo"}, + TaskRunSpecs: []v1beta1.PipelineTaskRunSpec{ + { + PipelineTaskName: "foo", + StepOverrides: []v1beta1.TaskRunStepOverride{{ + Name: "foo", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }}, + }, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("2Gi")}, + }, + }, + }, + }, + wantErr: &apis.FieldError{ + Message: "PipelineTaskRunSpec can't be configured with both step-level(stepOverrides.resources) and task-level(taskRunSpecs.resources) resource requirements", + }, + withContext: enableAlphaAPIFields, }} for _, ps := range tests { t.Run(ps.name, func(t *testing.T) { @@ -627,8 +650,9 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { func TestPipelineRunSpec_Validate(t *testing.T) { tests := []struct { - name string - spec v1beta1.PipelineRunSpec + name string + spec v1beta1.PipelineRunSpec + withContext func(context.Context) context.Context }{{ name: "PipelineRun without pipelineRef", spec: v1beta1.PipelineRunSpec{ @@ -641,10 +665,30 @@ func TestPipelineRunSpec_Validate(t *testing.T) { }}, }, }, + }, { + name: "valid task-level(taskRunSpecs.resources) resource requirements configured", + spec: v1beta1.PipelineRunSpec{ + PipelineRef: &v1beta1.PipelineRef{Name: "foo"}, + TaskRunSpecs: []v1beta1.PipelineTaskRunSpec{{ + PipelineTaskName: "foo", + StepOverrides: []v1beta1.TaskRunStepOverride{{ + Name: "foo", + }}, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("2Gi")}, + }, + }}, + }, + withContext: enableAlphaAPIFields, }} + for _, ps := range tests { t.Run(ps.name, func(t *testing.T) { - if err := ps.spec.Validate(context.Background()); err != nil { + ctx := context.Background() + if ps.withContext != nil { + ctx = ps.withContext(ctx) + } + if err := ps.spec.Validate(ctx); err != nil { t.Error(err) } }) diff --git a/pkg/apis/pipeline/v1beta1/resource_types.go b/pkg/apis/pipeline/v1beta1/resource_types.go index 0eda026817a..57734832c34 100644 --- a/pkg/apis/pipeline/v1beta1/resource_types.go +++ b/pkg/apis/pipeline/v1beta1/resource_types.go @@ -59,8 +59,8 @@ const ( // AllResourceTypes can be used for validation to check if a provided Resource type is one of the known types. var AllResourceTypes = resource.AllResourceTypes -// TaskResources allows a Pipeline to declare how its DeclaredPipelineResources -// should be provided to a Task as its inputs and outputs. +// TaskResources is overloaded to represent both PipelineResources and compute resources. +// PipelineResources are deprecated and will be removed, so this field will only represent compute resources in the future. type TaskResources struct { // Inputs holds the mapping from the PipelineResources declared in // DeclaredPipelineResources to the input PipelineResources required by the Task. @@ -70,6 +70,16 @@ type TaskResources struct { // DeclaredPipelineResources to the input PipelineResources required by the Task. // +listType=atomic Outputs []TaskResource `json:"outputs,omitempty"` + // Limits describes the maximum amount of compute resources allowed for each Task. + // More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + // +optional + Limits v1.ResourceList `json:"limits,omitempty"` + // Requests describes the minimum amount of compute resources required for each Task. + // If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + // otherwise to an implementation-defined value. + // More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + // +optional + Requests v1.ResourceList `json:"requests,omitempty"` } // TaskResource defines an input or output Resource declared as a requirement @@ -81,7 +91,7 @@ type TaskResource struct { ResourceDeclaration `json:",inline"` } -// TaskRunResources allows a TaskRun to declare inputs and outputs TaskResourceBinding +// TaskRunResources is overloaded to represent both TaskResourceBinding and compute resources. type TaskRunResources struct { // Inputs holds the inputs resources this task was invoked with // +listType=atomic @@ -89,6 +99,16 @@ type TaskRunResources struct { // Outputs holds the inputs resources this task was invoked with // +listType=atomic Outputs []TaskResourceBinding `json:"outputs,omitempty"` + // Limits describes the maximum amount of compute resources allowed for each TaskRun. + // More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + // +optional + Limits v1.ResourceList `json:"limits,omitempty"` + // Requests describes the minimum amount of compute resources required for each TaskRun. + // If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + // otherwise to an implementation-defined value. + // More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + // +optional + Requests v1.ResourceList `json:"requests,omitempty"` } // TaskResourceBinding points to the PipelineResource that diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index bae6550ae1c..0059b8c1887 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -1612,6 +1612,11 @@ "pipelineTaskName": { "type": "string" }, + "resources": { + "description": "Compute Resources required by the TaskRun. Cannot be updated. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/", + "default": {}, + "$ref": "#/definitions/v1.ResourceRequirements" + }, "sidecarOverrides": { "type": "array", "items": { @@ -2412,7 +2417,7 @@ } }, "v1beta1.TaskResources": { - "description": "TaskResources allows a Pipeline to declare how its DeclaredPipelineResources should be provided to a Task as its inputs and outputs.", + "description": "TaskResources is overloaded to represent both PipelineResources and compute resources. PipelineResources are deprecated and will be removed, so this field will only represent compute resources in the future.", "type": "object", "properties": { "inputs": { @@ -2424,6 +2429,14 @@ }, "x-kubernetes-list-type": "atomic" }, + "limits": { + "description": "Limits describes the maximum amount of compute resources allowed for each Task. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/", + "type": "object", + "additionalProperties": { + "default": {}, + "$ref": "#/definitions/k8s.io.apimachinery.pkg.api.resource.Quantity" + } + }, "outputs": { "description": "Outputs holds the mapping from the PipelineResources declared in DeclaredPipelineResources to the input PipelineResources required by the Task.", "type": "array", @@ -2432,6 +2445,14 @@ "$ref": "#/definitions/v1beta1.TaskResource" }, "x-kubernetes-list-type": "atomic" + }, + "requests": { + "description": "Requests describes the minimum amount of compute resources required for each Task. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/", + "type": "object", + "additionalProperties": { + "default": {}, + "$ref": "#/definitions/k8s.io.apimachinery.pkg.api.resource.Quantity" + } } } }, @@ -2563,7 +2584,7 @@ } }, "v1beta1.TaskRunResources": { - "description": "TaskRunResources allows a TaskRun to declare inputs and outputs TaskResourceBinding", + "description": "TaskRunResources is overloaded to represent both TaskResourceBinding and compute resources.", "type": "object", "properties": { "inputs": { @@ -2575,6 +2596,14 @@ }, "x-kubernetes-list-type": "atomic" }, + "limits": { + "description": "Limits describes the maximum amount of compute resources allowed for each TaskRun. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/", + "type": "object", + "additionalProperties": { + "default": {}, + "$ref": "#/definitions/k8s.io.apimachinery.pkg.api.resource.Quantity" + } + }, "outputs": { "description": "Outputs holds the inputs resources this task was invoked with", "type": "array", @@ -2583,6 +2612,14 @@ "$ref": "#/definitions/v1beta1.TaskResourceBinding" }, "x-kubernetes-list-type": "atomic" + }, + "requests": { + "description": "Requests describes the minimum amount of compute resources required for each TaskRun. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/", + "type": "object", + "additionalProperties": { + "default": {}, + "$ref": "#/definitions/k8s.io.apimachinery.pkg.api.resource.Quantity" + } } } }, diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index 771aaeed74c..7f412c098bb 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -79,6 +79,7 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(ValidateParameterVariables(ctx, ts.Steps, ts.Params)) errs = errs.Also(ValidateResourcesVariables(ctx, ts.Steps, ts.Resources)) errs = errs.Also(validateTaskContextVariables(ctx, ts.Steps)) + errs = errs.Also(ValidateResourceRequirements(ctx, ts)) errs = errs.Also(validateResults(ctx, ts.Results).ViaField("results")) return errs } @@ -374,6 +375,42 @@ func ValidateResourcesVariables(ctx context.Context, steps []Step, resources *Ta return validateVariables(ctx, steps, "resources.(?:inputs|outputs)", resourceNames) } +// ValidateResourceRequirements validates if only step-level or task-level resource requirements are configured +func ValidateResourceRequirements(ctx context.Context, taskSpec *TaskSpec) *apis.FieldError { + if isResourceRequirementsInTaskLevel(taskSpec) { + if err := ValidateEnabledAPIFields(ctx, "task-level resources requirements", config.AlphaAPIFields); err != nil { + return err + } + } + + if isResourceRequirementsInStepLevel(taskSpec) && isResourceRequirementsInTaskLevel(taskSpec) { + return &apis.FieldError{ + Message: "TaskSpec can't be configured with both step-level(Task.spec.step.resources or Task.spec.stepTemplate.resources) and task-level(Task.spec.resources) resources requirements", + Paths: []string{"resources"}, + } + } + + return nil +} + +// isResourceRequirementsInStepLevel checks if any resource requirements are specified under step-level +func isResourceRequirementsInStepLevel(taskSpec *TaskSpec) bool { + for _, step := range taskSpec.Steps { + if step.Resources.Size() > 0 { + return true + } + } + return taskSpec.StepTemplate != nil && taskSpec.StepTemplate.Resources.Size() > 0 +} + +// isResourceRequirementsInTaskLevel checks if any resource requirements are specified under task-level +func isResourceRequirementsInTaskLevel(taskSpec *TaskSpec) bool { + if taskSpec == nil || taskSpec.Resources == nil { + return false + } + return taskSpec.Resources.Limits != nil || taskSpec.Resources.Requests != nil +} + // TODO (@chuangw6): Make sure an object param is not used as a whole when providing values for strings. // https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md#variable-replacement-with-object-params // "When providing values for strings, Task and Pipeline authors can access diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index c69e2604756..24a12a1c1c1 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -28,6 +28,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "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" "knative.dev/pkg/apis" ) @@ -398,7 +399,72 @@ func TestTaskSpecValidate(t *testing.T) { hello "$(context.taskRun.namespace)"`, }}, }, + }, { + name: "valid step-level(step.resources) resources requirements", + fields: fields{ + Steps: []v1beta1.Step{{ + Name: "step-1", + Image: "my-image", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("2"), + }, + }, + }}, + }, + }, { + name: "valid step-level(stepTemplate.resources) resources requirements", + fields: fields{ + Steps: []v1beta1.Step{{ + Name: "step-1", + Image: "my-image", + }}, + StepTemplate: &v1beta1.StepTemplate{ + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("2"), + }, + }, + }, + }, + }, { + name: "valid step-level(step.resources and stepTemplate.resources) resources requirements", + fields: fields{ + Steps: []v1beta1.Step{{ + Name: "step-1", + Image: "my-image", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + }, + }, + }}, + StepTemplate: &v1beta1.StepTemplate{ + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + }, { + name: "valid task-level(spec.resources) resources requirements", + fields: fields{ + Steps: []v1beta1.Step{{ + Name: "step-1", + Image: "my-image", + }}, + Resources: &v1beta1.TaskResources{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("4"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("8"), + }, + }, + }, }} + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ts := &v1beta1.TaskSpec{ @@ -432,6 +498,7 @@ func TestTaskSpecValidateError(t *testing.T) { name string fields fields expectedError apis.FieldError + withContext func(context.Context) context.Context }{{ name: "empty spec", expectedError: apis.FieldError{ @@ -555,6 +622,7 @@ func TestTaskSpecValidateError(t *testing.T) { Paths: []string{"params"}, Details: "Names: \nMust only contain alphanumeric characters, hyphens (-), underscores (_), and dots (.)\nMust begin with a letter or an underscore (_)", }, + withContext: enableAlphaAPIFields, }, { name: "duplicated param names", fields: fields{ @@ -659,6 +727,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `"object" type does not match default value's type: "string"`, Paths: []string{"params.task.type", "params.task.default.type"}, }, + withContext: enableAlphaAPIFields, }, { name: "the spec of object type parameter misses the definition of properties", fields: fields{ @@ -670,6 +739,7 @@ func TestTaskSpecValidateError(t *testing.T) { Steps: validSteps, }, expectedError: *apis.ErrMissingField(fmt.Sprintf("params.task.properties")), + withContext: enableAlphaAPIFields, }, { name: "PropertySpec type is set with unsupported type", fields: fields{ @@ -688,6 +758,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: fmt.Sprintf("The value type specified for these keys %v is invalid", []string{"key1"}), Paths: []string{"params.task.properties"}, }, + withContext: enableAlphaAPIFields, }, { name: "keys defined in properties are missed in default", fields: fields{ @@ -709,6 +780,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: fmt.Sprintf("Required key(s) %s for the parameter %s are not provided in default.", []string{"key2"}, "myobjectParam"), Paths: []string{"myobjectParam.properties", "myobjectParam.default"}, }, + withContext: enableAlphaAPIFields, }, { name: "invalid step", fields: fields{ @@ -879,6 +951,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `variable is not properly isolated in "not isolated: $(params.baz)"`, Paths: []string{"steps[0].args[0]"}, }, + withContext: enableAlphaAPIFields, }, { name: "inferred array star not properly isolated", fields: fields{ @@ -901,6 +974,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `variable is not properly isolated in "not isolated: $(params.baz[*])"`, Paths: []string{"steps[0].args[0]"}, }, + withContext: enableAlphaAPIFields, }, { name: "Inexistent param variable in volumeMount with existing", fields: fields{ @@ -923,6 +997,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `non-existent variable in "$(params.inexistent)-foo"`, Paths: []string{"steps[0].volumeMount[0].name"}, }, + withContext: enableAlphaAPIFields, }, { name: "Inexistent param variable with existing", fields: fields{ @@ -941,6 +1016,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `non-existent variable in "$(params.foo) && $(params.inexistent)"`, Paths: []string{"steps[0].args[0]"}, }, + withContext: enableAlphaAPIFields, }, { name: "Multiple volumes with same name", fields: fields{ @@ -1161,6 +1237,60 @@ func TestTaskSpecValidateError(t *testing.T) { Message: "invalid value: -10s", Paths: []string{"steps[0].negative timeout"}, }, + }, { + name: "invalid both step-level(step.resources) and task-level resources requirements", + fields: fields{ + Steps: []v1beta1.Step{{ + Name: "step-1", + Image: "my-image", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("2"), + }, + }, + }}, + Resources: &v1beta1.TaskResources{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("4"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("8"), + }, + }, + }, + expectedError: apis.FieldError{ + Message: "TaskSpec can't be configured with both step-level(Task.spec.step.resources or Task.spec.stepTemplate.resources) and task-level(Task.spec.resources) resources requirements", + Paths: []string{"resources"}, + }, + withContext: enableAlphaAPIFields, + }, { + name: "invalid both step-level(stepTemplate.resources) and task-level resources requirements", + fields: fields{ + Steps: []v1beta1.Step{{ + Name: "step-1", + Image: "my-image", + }}, + StepTemplate: &v1beta1.StepTemplate{ + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + Resources: &v1beta1.TaskResources{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("4"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("8"), + }, + }, + }, + expectedError: apis.FieldError{ + Message: "TaskSpec can't be configured with both step-level(Task.spec.step.resources or Task.spec.stepTemplate.resources) and task-level(Task.spec.resources) resources requirements", + Paths: []string{"resources"}, + }, + withContext: enableAlphaAPIFields, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1173,8 +1303,13 @@ func TestTaskSpecValidateError(t *testing.T) { Workspaces: tt.fields.Workspaces, Results: tt.fields.Results, } - ctx := getContextBasedOnFeatureFlag("alpha") + + ctx := context.Background() + if tt.withContext != nil { + ctx = tt.withContext(ctx) + } ts.SetDefaults(ctx) + err := ts.Validate(ctx) if err == nil { t.Fatalf("Expected an error, got nothing for %v", ts) diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go index e4fa03228e7..cde9da427d7 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go @@ -66,6 +66,7 @@ func (ts *TaskRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) { if ts.StepOverrides != nil { errs = errs.Also(ValidateEnabledAPIFields(ctx, "stepOverrides", config.AlphaAPIFields).ViaField("stepOverrides")) errs = errs.Also(validateStepOverrides(ts.StepOverrides).ViaField("stepOverrides")) + errs = errs.Also(validateStepOverridesResourceRequirements(ts.Resources, ts.StepOverrides).ViaField("stepOverrides")) } if ts.SidecarOverrides != nil { errs = errs.Also(ValidateEnabledAPIFields(ctx, "sidecarOverrides", config.AlphaAPIFields).ViaField("sidecarOverrides")) @@ -138,6 +139,27 @@ func validateStepOverrides(overrides []TaskRunStepOverride) (errs *apis.FieldErr return errs } +// validateStepOverridesResourceRequirements validates if both step-level and task-level resource requirements are configured +func validateStepOverridesResourceRequirements(resources *TaskRunResources, overrides []TaskRunStepOverride) (errs *apis.FieldError) { + for _, override := range overrides { + if override.Resources.Size() > 0 && isResourceRequirementsInTaskRun(resources) { + return &apis.FieldError{ + Message: "TaskRun can't be configured with both step-level(stepOverrides.resources) and task-level(TaskRun.resources) resource requirements", + Paths: []string{"resources"}, + } + } + } + return nil +} + +// isResourceRequirementsInTaskRun checks if resource requirements are configured in TaskRun under task-level +func isResourceRequirementsInTaskRun(resources *TaskRunResources) bool { + if resources == nil { + return false + } + return resources.Limits != nil || resources.Requests != nil +} + func validateSidecarOverrides(overrides []TaskRunSidecarOverride) (errs *apis.FieldError) { var names []string for i, o := range overrides { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go index 89bfffe91ed..cf3e7899860 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -515,7 +515,31 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { }, wantErr: apis.ErrMissingField("sidecarOverrides[0].name"), wc: enableAlphaAPIFields, + }, { + name: "invalid both step-level(stepOverrides.resources) and task-level(TaskRun.resources) resource requirements", + spec: v1beta1.TaskRunSpec{ + TaskRef: &v1beta1.TaskRef{Name: "task"}, + StepOverrides: []v1beta1.TaskRunStepOverride{{ + Name: "foo", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: corev1resources.MustParse("1Gi"), + }, + }, + }}, + Resources: &v1beta1.TaskRunResources{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: corev1resources.MustParse("2Gi"), + }, + }, + }, + wantErr: &apis.FieldError{ + Message: "TaskRun can't be configured with both step-level(stepOverrides.resources) and task-level(TaskRun.resources) resource requirements", + Paths: []string{"stepOverrides.resources"}, + }, + wc: enableAlphaAPIFields, }} + for _, ts := range tests { t.Run(ts.name, func(t *testing.T) { ctx := context.Background() @@ -534,6 +558,7 @@ func TestTaskRunSpec_Validate(t *testing.T) { tests := []struct { name string spec v1beta1.TaskRunSpec + wc func(context.Context) context.Context }{{ name: "taskspec without a taskRef", spec: v1beta1.TaskRunSpec{ @@ -581,10 +606,26 @@ func TestTaskRunSpec_Validate(t *testing.T) { }}, }, }, + }, { + name: "valid task-level(TaskRun.resources) resource requirements", + spec: v1beta1.TaskRunSpec{ + TaskRef: &v1beta1.TaskRef{Name: "task"}, + Resources: &v1beta1.TaskRunResources{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: corev1resources.MustParse("2Gi"), + }, + }, + }, + wc: enableAlphaAPIFields, }} + for _, ts := range tests { t.Run(ts.name, func(t *testing.T) { - if err := ts.spec.Validate(context.Background()); err != nil { + ctx := context.Background() + if ts.wc != nil { + ctx = ts.wc(ctx) + } + if err := ts.spec.Validate(ctx); err != nil { t.Error(err) } }) diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index c739fe1d7f2..8ea8a510741 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -1226,6 +1226,7 @@ func (in *PipelineTaskRunSpec) DeepCopyInto(out *PipelineTaskRunSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + in.Resources.DeepCopyInto(&out.Resources) return } @@ -1774,6 +1775,20 @@ func (in *TaskResources) DeepCopyInto(out *TaskResources) { *out = make([]TaskResource, len(*in)) copy(*out, *in) } + if in.Limits != nil { + in, out := &in.Limits, &out.Limits + *out = make(v1.ResourceList, len(*in)) + for key, val := range *in { + (*out)[key] = val.DeepCopy() + } + } + if in.Requests != nil { + in, out := &in.Requests, &out.Requests + *out = make(v1.ResourceList, len(*in)) + for key, val := range *in { + (*out)[key] = val.DeepCopy() + } + } return } @@ -1955,6 +1970,20 @@ func (in *TaskRunResources) DeepCopyInto(out *TaskRunResources) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Limits != nil { + in, out := &in.Limits, &out.Limits + *out = make(v1.ResourceList, len(*in)) + for key, val := range *in { + (*out)[key] = val.DeepCopy() + } + } + if in.Requests != nil { + in, out := &in.Requests, &out.Requests + *out = make(v1.ResourceList, len(*in)) + for key, val := range *in { + (*out)[key] = val.DeepCopy() + } + } return }