Skip to content

Commit

Permalink
Implement Step and Sidecar Overrides for TaskRun
Browse files Browse the repository at this point in the history 
This implements TEP-0094: Configuring Resources at Runtime.
StepOverrides and SidecarOverrides were added to the API in a previous commit.
This commit allows them to override resource requirements specified in the Task spec.
The same merging strategy that is currently used for stepTemplate is also used for
step and sidecar overrides, except that only the resources field of the container is overridden,
and unlike with templates, the overrides take precedence over the Task spec.
  • Loading branch information
@lbernick
lbernick committed Mar 4, 2022
1 parent c8a9e0b commit 9da9add
Show file tree
Hide file tree
Showing 9 changed files with 356 additions and 6 deletions.
12 changes: 10 additions & 2 deletions docs/taskruns.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,10 +404,9 @@ and reasons.
### Overriding Task Steps and Sidecars

**([alpha only](https://github.com/tektoncd/pipeline/blob/main/docs/install.md#alpha-features))**
**Warning: This feature is still under development and is not yet functional. Do not use it.**

A TaskRun can specify `StepOverrides` or `SidecarOverrides` to override Step or Sidecar
configuration specified in a Task.
configuration specified in a Task. Only named Steps and Sidecars may be overridden.

For example, given the following Task definition:

Expand Down Expand Up @@ -450,6 +449,15 @@ spec:
```
`StepOverrides` and `SidecarOverrides` must include the `name` field and may include `resources`.
No other fields can be overridden.
If the overridden `Task` uses a [`StepTemplate`](./tasks.md#specifying-a-step-template), configuration on
`Step` will take precedence over configuration in `StepTemplate`, and configuration in `StepOverride` will
take precedence over both.

When merging resource requirements, different resource types are considered independently.
For example, if a `Step` configures both CPU and memory, and a `StepOverride` configures only memory,
the CPU values from the `Step` will be preserved. Requests and limits are also considered independently.
For example, if a `Step` configures a memory request and limit, and a `StepOverride` configures only a
memory request, the memory limit from the `Step` will be preserved.

### Specifying `LimitRange` values

Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipelinerun_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,8 @@ func (pr *PipelineRun) GetTaskRunSpec(pipelineTaskName string) PipelineTaskRunSp
if task.TaskServiceAccountName != "" {
s.TaskServiceAccountName = task.TaskServiceAccountName
}
s.StepOverrides = task.StepOverrides
s.SidecarOverrides = task.SidecarOverrides
}
}
return s
Expand Down
12 changes: 10 additions & 2 deletions pkg/pod/pod.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,21 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
if err != nil {
return nil, err
}
steps, err = v1beta1.MergeStepsWithOverrides(steps, taskRun.Spec.StepOverrides)
if err != nil {
return nil, err
}
sidecars, err := v1beta1.MergeSidecarsWithOverrides(taskSpec.Sidecars, taskRun.Spec.SidecarOverrides)
if err != nil {
return nil, err
}

// Convert any steps with Script to command+args.
// If any are found, append an init container to initialize scripts.
if alphaAPIEnabled {
scriptsInit, stepContainers, sidecarContainers = convertScripts(b.Images.ShellImage, b.Images.ShellImageWin, steps, taskSpec.Sidecars, taskRun.Spec.Debug)
scriptsInit, stepContainers, sidecarContainers = convertScripts(b.Images.ShellImage, b.Images.ShellImageWin, steps, sidecars, taskRun.Spec.Debug)
} else {
scriptsInit, stepContainers, sidecarContainers = convertScripts(b.Images.ShellImage, "", steps, taskSpec.Sidecars, nil)
scriptsInit, stepContainers, sidecarContainers = convertScripts(b.Images.ShellImage, "", steps, sidecars, nil)
}
if scriptsInit != nil {
initContainers = append(initContainers, *scriptsInit)
Expand Down
211 changes: 211 additions & 0 deletions pkg/pod/pod_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,217 @@ _EOF_
}),
ActiveDeadlineSeconds: &defaultActiveDeadlineSeconds,
},
}, {
desc: "with stepOverrides",
ts: v1beta1.TaskSpec{
Steps: []v1beta1.Step{{Container: corev1.Container{
Name: "step1",
Image: "image",
Command: []string{"cmd"},
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("8"),
corev1.ResourceMemory: resource.MustParse("10Gi"),
},
},
}}},
},
trs: v1beta1.TaskRunSpec{
StepOverrides: []v1beta1.TaskRunStepOverride{{
Name: "step1",
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("6"),
corev1.ResourceMemory: resource.MustParse("5Gi"),
},
},
}},
},
want: &corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{
{Container: corev1.Container{Name: "step1"}},
})},
Containers: []corev1.Container{{
Name: "step-step1",
Image: "image",
Command: []string{"/tekton/bin/entrypoint"},
Args: []string{
"-wait_file",
"/tekton/downward/ready",
"-wait_file_content",
"-post_file",
"/tekton/run/0/out",
"-termination_path",
"/tekton/termination",
"-step_metadata_dir",
"/tekton/run/0/status",
"-entrypoint",
"cmd",
"--",
},
VolumeMounts: append([]corev1.VolumeMount{binROMount, runMount(0, false), downwardMount, {
Name: "tekton-creds-init-home-0",
MountPath: "/tekton/creds",
}}, implicitVolumeMounts...),
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("6"),
corev1.ResourceMemory: resource.MustParse("5Gi"),
},
},
TerminationMessagePath: "/tekton/termination",
}},
Volumes: append(implicitVolumes, binVolume, runVolume(0), downwardVolume, corev1.Volume{
Name: "tekton-creds-init-home-0",
VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}},
}),
ActiveDeadlineSeconds: &defaultActiveDeadlineSeconds,
},
}, {
desc: "with stepOverrides and stepTemplate",
ts: v1beta1.TaskSpec{
StepTemplate: &corev1.Container{
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("8"),
corev1.ResourceMemory: resource.MustParse("10Gi"),
},
},
},
Steps: []v1beta1.Step{{Container: corev1.Container{
Name: "step1",
Image: "image",
Command: []string{"cmd"},
}}},
},
trs: v1beta1.TaskRunSpec{
StepOverrides: []v1beta1.TaskRunStepOverride{{
Name: "step1",
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("6"),
corev1.ResourceMemory: resource.MustParse("5Gi"),
},
},
}},
},
want: &corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{
{Container: corev1.Container{Name: "step1"}},
})},
Containers: []corev1.Container{{
Name: "step-step1",
Image: "image",
Command: []string{"/tekton/bin/entrypoint"},
Args: []string{
"-wait_file",
"/tekton/downward/ready",
"-wait_file_content",
"-post_file",
"/tekton/run/0/out",
"-termination_path",
"/tekton/termination",
"-step_metadata_dir",
"/tekton/run/0/status",
"-entrypoint",
"cmd",
"--",
},
VolumeMounts: append([]corev1.VolumeMount{binROMount, runMount(0, false), downwardMount, {
Name: "tekton-creds-init-home-0",
MountPath: "/tekton/creds",
}}, implicitVolumeMounts...),
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("6"),
corev1.ResourceMemory: resource.MustParse("5Gi"),
},
},
TerminationMessagePath: "/tekton/termination",
}},
Volumes: append(implicitVolumes, binVolume, runVolume(0), downwardVolume, corev1.Volume{
Name: "tekton-creds-init-home-0",
VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}},
}),
ActiveDeadlineSeconds: &defaultActiveDeadlineSeconds,
},
}, {
desc: "with sidecarOverrides",
ts: v1beta1.TaskSpec{
Steps: []v1beta1.Step{{Container: corev1.Container{
Name: "primary-name",
Image: "primary-image",
Command: []string{"cmd"}, // avoid entrypoint lookup.
}}},
Sidecars: []v1beta1.Sidecar{{
Container: corev1.Container{
Name: "sc-name",
Image: "sidecar-image",
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("8"),
corev1.ResourceMemory: resource.MustParse("10Gi"),
},
},
},
}},
},
trs: v1beta1.TaskRunSpec{
SidecarOverrides: []v1beta1.TaskRunSidecarOverride{{
Name: "sc-name",
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("6"),
corev1.ResourceMemory: resource.MustParse("5Gi"),
},
},
}},
},
wantAnnotations: map[string]string{},
want: &corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Container: corev1.Container{Name: "primary-name"}}})},
Containers: []corev1.Container{{
Name: "step-primary-name",
Image: "primary-image",
Command: []string{"/tekton/bin/entrypoint"},
Args: []string{
"-wait_file",
"/tekton/downward/ready",
"-wait_file_content",
"-post_file",
"/tekton/run/0/out",
"-termination_path",
"/tekton/termination",
"-step_metadata_dir",
"/tekton/run/0/status",
"-entrypoint",
"cmd",
"--",
},
VolumeMounts: append([]corev1.VolumeMount{binROMount, runMount(0, false), downwardMount, {
Name: "tekton-creds-init-home-0",
MountPath: "/tekton/creds",
}}, implicitVolumeMounts...),
TerminationMessagePath: "/tekton/termination",
}, {
Name: "sidecar-sc-name",
Image: "sidecar-image",
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("6"),
corev1.ResourceMemory: resource.MustParse("5Gi"),
},
},
}},
Volumes: append(implicitVolumes, binVolume, runVolume(0), downwardVolume, corev1.Volume{
Name: "tekton-creds-init-home-0",
VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}},
}),
ActiveDeadlineSeconds: &defaultActiveDeadlineSeconds,
},
}, {
desc: "step with script and stepTemplate",
ts: v1beta1.TaskSpec{
Expand Down
2 changes: 2 additions & 0 deletions pkg/reconciler/pipelinerun/pipelinerun.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,8 @@ func (c *Reconciler) createTaskRun(ctx context.Context, rprt *resources.Resolved
ServiceAccountName: taskRunSpec.TaskServiceAccountName,
Timeout: getTimeoutFunc(ctx, pr, rprt, c.Clock),
PodTemplate: taskRunSpec.TaskPodTemplate,
StepOverrides: taskRunSpec.StepOverrides,
SidecarOverrides: taskRunSpec.SidecarOverrides,
}}

if rprt.ResolvedTaskResources.TaskName != "" {
Expand Down
8 changes: 6 additions & 2 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4451,6 +4451,8 @@ func TestReconcileAndPropagateCustomPipelineTaskRunSpec(t *testing.T) {
"workloadtype": "tekton",
},
},
StepOverrides: []v1beta1.TaskRunStepOverride{{Name: "foo"}},
SidecarOverrides: []v1beta1.TaskRunSidecarOverride{{Name: "bar"}},
}},
},
}}
Expand Down Expand Up @@ -4487,8 +4489,10 @@ func TestReconcileAndPropagateCustomPipelineTaskRunSpec(t *testing.T) {
"workloadtype": "tekton",
},
},
Resources: &v1beta1.TaskRunResources{},
Timeout: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute},
Resources: &v1beta1.TaskRunResources{},
Timeout: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute},
StepOverrides: []v1beta1.TaskRunStepOverride{{Name: "foo"}},
SidecarOverrides: []v1beta1.TaskRunSidecarOverride{{Name: "bar"}},
},
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/reconciler/taskrun/taskrun.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,12 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1
}
}

if err := ValidateOverrides(ctx, taskSpec, &tr.Spec); err != nil {
logger.Errorf("TaskRun %q step or sidecar overrides are invalid: %v", tr.Name, err)
tr.Status.MarkResourceFailed(podconvert.ReasonFailedValidation, err)
return nil, nil, controller.NewPermanentError(err)
}

// Initialize the cloud events if at least a CloudEventResource is defined
// and they have not been initialized yet.
// FIXME(afrittoli) This resource specific logic will have to be replaced
Expand Down
38 changes: 38 additions & 0 deletions pkg/reconciler/taskrun/validate_resources.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ import (
resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1"
"github.com/tektoncd/pipeline/pkg/list"
"github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources"
"k8s.io/apimachinery/pkg/util/sets"

"github.com/hashicorp/go-multierror"
)

func validateResources(requiredResources []v1beta1.TaskResource, providedResources map[string]*resourcev1alpha1.PipelineResource) error {
Expand Down Expand Up @@ -168,3 +171,38 @@ func validateTaskSpecRequestResources(ctx context.Context, taskSpec *v1beta1.Tas

return nil
}

// ValidateOverrides validates that all stepOverrides map to valid steps, and likewise for sidecarOverrides
func ValidateOverrides(ctx context.Context, ts *v1beta1.TaskSpec, trs *v1beta1.TaskRunSpec) error {
stepErr := validateStepOverrides(ctx, ts, trs)
sidecarErr := validateSidecarOverrides(ctx, ts, trs)
return multierror.Append(stepErr, sidecarErr).ErrorOrNil()
}

func validateStepOverrides(ctx context.Context, ts *v1beta1.TaskSpec, trs *v1beta1.TaskRunSpec) error {
var err error
stepNames := sets.NewString()
for _, step := range ts.Steps {
stepNames.Insert(step.Name)
}
for _, stepOverride := range trs.StepOverrides {
if !stepNames.Has(stepOverride.Name) {
err = multierror.Append(err, fmt.Errorf("invalid StepOverride: No Step named %s", stepOverride.Name))
}
}
return err
}

func validateSidecarOverrides(ctx context.Context, ts *v1beta1.TaskSpec, trs *v1beta1.TaskRunSpec) error {
var err error
sidecarNames := sets.NewString()
for _, sidecar := range ts.Sidecars {
sidecarNames.Insert(sidecar.Name)
}
for _, sidecarOverride := range trs.SidecarOverrides {
if !sidecarNames.Has(sidecarOverride.Name) {
err = multierror.Append(err, fmt.Errorf("invalid SidecarOverride: No Sidecar named %s", sidecarOverride.Name))
}
}
return err
}
Loading

0 comments on commit 9da9add

Please sign in to comment.