From 6847b3f5665d4a06004a6d48092d1de95354ed8a 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 Support Task-level Resources Requirements --- .../pipeline/v1beta1/openapi_generated.go | 41 ++++++- .../pipeline/v1beta1/pipelinerun_types.go | 6 + pkg/apis/pipeline/v1beta1/resource_types.go | 10 ++ pkg/apis/pipeline/v1beta1/swagger.json | 16 +++ pkg/apis/pipeline/v1beta1/task_validation.go | 37 ++++++ .../pipeline/v1beta1/task_validation_test.go | 116 ++++++++++++++++++ .../pipeline/v1beta1/taskrun_validation.go | 14 +++ .../v1beta1/taskrun_validation_test.go | 29 +++++ .../pipeline/v1beta1/zz_generated.deepcopy.go | 15 +++ 9 files changed, 282 insertions(+), 2 deletions(-) diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index bf89f787a11..16f44d1fb2e 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 this container. 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"}, } } @@ -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. 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. 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"}, } } diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go index 31dd1d94ea0..172e540733b 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 this container. + // 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/resource_types.go b/pkg/apis/pipeline/v1beta1/resource_types.go index 0eda026817a..242f3c3e2b1 100644 --- a/pkg/apis/pipeline/v1beta1/resource_types.go +++ b/pkg/apis/pipeline/v1beta1/resource_types.go @@ -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. + // 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. + // 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 diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index f218f3f2a84..a4dfaf75618 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2424,6 +2424,14 @@ }, "x-kubernetes-list-type": "atomic" }, + "limits": { + "description": "Limits describes the maximum amount of compute resources allowed. 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 +2440,14 @@ "$ref": "#/definitions/v1beta1.TaskResource" }, "x-kubernetes-list-type": "atomic" + }, + "requests": { + "description": "Requests describes the minimum amount of compute resources required. 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 a43e05b54f4..00404a2ad1a 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -78,6 +78,7 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(ValidateParameterTypes(ctx, ts.Params).ViaField("params")) errs = errs.Also(ValidateParameterVariables(ts.Steps, ts.Params)) errs = errs.Also(ValidateResourcesVariables(ts.Steps, ts.Resources)) + errs = errs.Also(ValidateResourcesRequirements(ctx, ts)) errs = errs.Also(validateTaskContextVariables(ts.Steps)) errs = errs.Also(validateResults(ctx, ts.Results).ViaField("results")) return errs @@ -370,6 +371,42 @@ func ValidateResourcesVariables(steps []Step, resources *TaskResources) *apis.Fi return validateVariables(steps, "resources.(?:inputs|outputs)", resourceNames) } +// ValidateResourcesRequirements() validates if only step-level or task-level resources requirements are configured +func ValidateResourcesRequirements(ctx context.Context, taskSpec *TaskSpec) *apis.FieldError { + if isResourcesRequirementsInTaskLevel(taskSpec) { + if err := ValidateEnabledAPIFields(ctx, "task-level resources requirements", config.AlphaAPIFields); err != nil { + return err + } + } + + if isResourcesRequirementsInStepLevel(taskSpec) && isResourcesRequirementsInTaskLevel(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 +} + +// isResourcesRequirementsInStepLevel() checks if any resources requirements specified under step-level +func isResourcesRequirementsInStepLevel(taskSpec *TaskSpec) bool { + for _, step := range taskSpec.Steps { + if step.Resources.Size() > 0 { + return true + } + } + return taskSpec.StepTemplate != nil && taskSpec.StepTemplate.Resources.Size() > 0 +} + +// isResourcesRequirementsInTaskLevel() checks if any resources requirements specified under task-level +func isResourcesRequirementsInTaskLevel(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 c51e59a30ad..a6675f7f402 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" ) @@ -372,7 +373,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{ @@ -1115,6 +1181,56 @@ 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"}, + }, + }, { + 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"}, + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go index e4fa03228e7..4cf3f114a0f 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(validateStepOverridesResourcesRequirements(ts.TaskSpec, ts.StepOverrides).ViaField("stepOverrides")) } if ts.SidecarOverrides != nil { errs = errs.Also(ValidateEnabledAPIFields(ctx, "sidecarOverrides", config.AlphaAPIFields).ViaField("sidecarOverrides")) @@ -138,6 +139,19 @@ func validateStepOverrides(overrides []TaskRunStepOverride) (errs *apis.FieldErr return errs } +// validateStepOverridesResourcesRequirements() validates if both step-level and task-level resources requirements are configured +func validateStepOverridesResourcesRequirements(taskSpec *TaskSpec, overrides []TaskRunStepOverride) (errs *apis.FieldError) { + for _, override := range overrides { + if override.Resources.Size() > 0 && isResourcesRequirementsInTaskLevel(taskSpec) { + return &apis.FieldError{ + Message: "TaskRun can't be configured with both step-level(stepOverrides.resources) and task-level(taskSpec.resources) resources requirements", + Paths: []string{"resources"}, + } + } + } + return 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..9a5ed3067b7 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -515,7 +515,36 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { }, wantErr: apis.ErrMissingField("sidecarOverrides[0].name"), wc: enableAlphaAPIFields, + }, { + name: "invalid both step-level(stepOverrides.resources) and task-level(taskSpec.resources) resources requirements", + spec: v1beta1.TaskRunSpec{ + TaskSpec: &v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{ + Name: "step-1", + Image: "my-image", + }}, + Resources: &v1beta1.TaskResources{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: corev1resources.MustParse("2Gi"), + }, + }, + }, + StepOverrides: []v1beta1.TaskRunStepOverride{{ + Name: "foo", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: corev1resources.MustParse("1Gi"), + }, + }, + }}, + }, + wantErr: &apis.FieldError{ + Message: "TaskRun can't be configured with both step-level(stepOverrides.resources) and task-level(taskSpec.resources) resources requirements", + Paths: []string{"stepOverrides.resources"}, + }, + wc: enableAlphaAPIFields, }} + for _, ts := range tests { t.Run(ts.name, func(t *testing.T) { ctx := context.Background() diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 43ea81e4520..7f60e4fcf80 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 }