diff --git a/docs/tasks.md b/docs/tasks.md index 8d27dac57fa..df4c8eb020d 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -22,7 +22,8 @@ entire Kubernetes cluster. - [Outputs](#outputs) - [Controlling where resources are mounted](#controlling-where-resources-are-mounted) - [Volumes](#volumes) - - [Container Template](#container-template) + - [Container Template **deprecated**](#step-template) + - [Step Template](#step-template) - [Templating](#templating) - [Examples](#examples) @@ -74,8 +75,9 @@ following fields: created by your `Task` - [`volumes`](#volumes) - Specifies one or more volumes that you want to make available to your `Task`'s steps. - - [`containerTemplate`](#container-template) - Specifies a `Container` + - [`stepTemplate`](#step-template) - Specifies a `Container` step definition to use as the basis for all steps within your `Task`. + - [`containerTemplate`](#step-template) - **deprecated** Previous name of `stepTemplate`. [kubernetes-overview]: https://kubernetes.io/docs/concepts/overview/working-with-objects/kubernetes-objects/#required-fields @@ -326,19 +328,19 @@ For example, use volumes to accomplish one of the following common tasks: unsafe_. Use [kaniko](https://github.com/GoogleContainerTools/kaniko) instead. This is used only for the purposes of demonstration. -### Container Template +### Step Template Specifies a [`Container`](https://kubernetes.io/docs/concepts/containers/) configuration that will be used as the basis for all [`steps`](#steps) in your `Task`. Configuration in an individual step will override or merge with the container template's configuration. -In the example below, the `Task` specifies a `containerTemplate` with the +In the example below, the `Task` specifies a `stepTemplate` with the environment variable `FOO` set to `bar`. The first step will use that value for `FOO`, but in the second step, `FOO` is overridden and set to `baz`. ```yaml -containerTemplate: +stepTemplate: env: - name: "FOO" value: "bar" @@ -354,6 +356,9 @@ steps: value: "baz" ``` +_The field `containerTemplate` provides the same functionality but is **deprecated** +and will be removed in a future release ([#977](https://github.com/tektoncd/pipeline/issues/977))._ + ### Templating `Tasks` support templating using values from all [`inputs`](#inputs) and diff --git a/pkg/apis/pipeline/v1alpha1/task_types.go b/pkg/apis/pipeline/v1alpha1/task_types.go index 0843c05f88a..018b33f99fe 100644 --- a/pkg/apis/pipeline/v1alpha1/task_types.go +++ b/pkg/apis/pipeline/v1alpha1/task_types.go @@ -54,8 +54,12 @@ type TaskSpec struct { // steps of the build. Volumes []corev1.Volume `json:"volumes,omitempty"` - // ContainerTemplate can be used as the basis for all step containers within the + // StepTemplate can be used as the basis for all step containers within the // Task, so that the steps inherit settings on the base container. + StepTemplate *corev1.Container `json:"stepTemplate,omitempty"` + + // ContainerTemplate is the deprecated previous name of the StepTemplate + // field (#977). ContainerTemplate *corev1.Container `json:"containerTemplate,omitempty"` } diff --git a/pkg/apis/pipeline/v1alpha1/task_validation.go b/pkg/apis/pipeline/v1alpha1/task_validation.go index c0dff825ff6..6b38efb406f 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation.go +++ b/pkg/apis/pipeline/v1alpha1/task_validation.go @@ -47,11 +47,20 @@ func (ts *TaskSpec) Validate(ctx context.Context) *apis.FieldError { if err := ValidateVolumes(ts.Volumes).ViaField("volumes"); err != nil { return err } - mergedSteps, err := merge.CombineStepsWithContainerTemplate(ts.ContainerTemplate, ts.Steps) + mergedSteps, err := merge.CombineStepsWithStepTemplate(ts.StepTemplate, ts.Steps) if err != nil { return &apis.FieldError{ - Message: fmt.Sprintf("error merging container template and steps: %s", err), - Paths: []string{"containerTemplate"}, + Message: fmt.Sprintf("error merging step template and steps: %s", err), + Paths: []string{"stepTemplate"}, + } + } + + // The ContainerTemplate field is deprecated (#977) + mergedSteps, err = merge.CombineStepsWithStepTemplate(ts.ContainerTemplate, ts.Steps) + if err != nil { + return &apis.FieldError{ + Message: fmt.Sprintf("error merging containerTemplate and steps: %s", err), + Paths: []string{"stepTemplate"}, } } diff --git a/pkg/apis/pipeline/v1alpha1/task_validation_test.go b/pkg/apis/pipeline/v1alpha1/task_validation_test.go index d10c0582492..d2d79d6a76a 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/task_validation_test.go @@ -51,6 +51,7 @@ func TestTaskSpecValidate(t *testing.T) { Inputs *Inputs Outputs *Outputs BuildSteps []corev1.Container + StepTemplate *corev1.Container ContainerTemplate *corev1.Container } tests := []struct { @@ -126,13 +127,43 @@ func TestTaskSpecValidate(t *testing.T) { }}, }, }, { - name: "container template included in validation", + name: "step template included in validation", fields: fields{ BuildSteps: []corev1.Container{{ Name: "astep", Command: []string{"echo"}, Args: []string{"hello"}, }}, + StepTemplate: &corev1.Container{ + Image: "some-image", + }, + }, + }, { + name: "deprecated (#977) container template included in validation", + fields: fields{ + BuildSteps: []corev1.Container{{ + Name: "astep", + Command: []string{"echo"}, + Args: []string{"hello"}, + }}, + ContainerTemplate: &corev1.Container{ + Image: "some-image", + }, + }, + }, { + name: "deprecated (#977) container template supported with step template", + fields: fields{ + BuildSteps: []corev1.Container{{ + Name: "astep", + Command: []string{"echo"}, + Args: []string{"hello"}, + }}, + StepTemplate: &corev1.Container{ + Env: []corev1.EnvVar{{ + Name: "somevar", + Value: "someval", + }}, + }, ContainerTemplate: &corev1.Container{ Image: "some-image", }, @@ -144,6 +175,7 @@ func TestTaskSpecValidate(t *testing.T) { Inputs: tt.fields.Inputs, Outputs: tt.fields.Outputs, Steps: tt.fields.BuildSteps, + StepTemplate: tt.fields.StepTemplate, ContainerTemplate: tt.fields.ContainerTemplate, } ctx := context.Background() diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index 92966758d9c..392d1488974 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -1497,8 +1497,8 @@ func (in *TaskSpec) DeepCopyInto(out *TaskSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.ContainerTemplate != nil { - in, out := &in.ContainerTemplate, &out.ContainerTemplate + if in.StepTemplate != nil { + in, out := &in.StepTemplate, &out.StepTemplate if *in == nil { *out = nil } else { diff --git a/pkg/merge/merge.go b/pkg/merge/merge.go index c474d5ec2a5..04ece1f2232 100644 --- a/pkg/merge/merge.go +++ b/pkg/merge/merge.go @@ -20,9 +20,9 @@ import ( "k8s.io/apimachinery/pkg/util/strategicpatch" ) -// CombineStepsWithContainerTemplate takes a possibly nil container template and a list of step containers, merging each +// CombineStepsWithStepTemplate takes a possibly nil container template and a list of step containers, merging each // of the step containers onto the container template, if it's not nil, and returning the resulting list. -func CombineStepsWithContainerTemplate(template *v1.Container, steps []v1.Container) ([]v1.Container, error) { +func CombineStepsWithStepTemplate(template *v1.Container, steps []v1.Container) ([]v1.Container, error) { if template == nil { return steps, nil } diff --git a/pkg/merge/merge_test.go b/pkg/merge/merge_test.go index 7e63334a8c6..e3c54eb0b05 100644 --- a/pkg/merge/merge_test.go +++ b/pkg/merge/merge_test.go @@ -21,7 +21,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" ) -func TestCombineStepsWithContainerTemplate(t *testing.T) { +func TestCombineStepsWithStepTemplate(t *testing.T) { resourceQuantityCmp := cmp.Comparer(func(x, y resource.Quantity) bool { return x.Cmp(y) == 0 }) @@ -105,7 +105,7 @@ func TestCombineStepsWithContainerTemplate(t *testing.T) { }}, }} { t.Run(tc.name, func(t *testing.T) { - result, err := CombineStepsWithContainerTemplate(tc.template, tc.steps) + result, err := CombineStepsWithStepTemplate(tc.template, tc.steps) if err != nil { t.Errorf("expected no error. Got error %v", err) } diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/pod.go b/pkg/reconciler/v1alpha1/taskrun/resources/pod.go index 107e7292150..e9d0c7444fc 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/pod.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/pod.go @@ -296,11 +296,21 @@ func MakePod(taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient k } podContainers = append(podContainers, *nopContainer) - mergedInitContainers, err := merge.CombineStepsWithContainerTemplate(taskSpec.ContainerTemplate, initContainers) + mergedInitContainers, err := merge.CombineStepsWithStepTemplate(taskSpec.StepTemplate, initContainers) if err != nil { return nil, err } - mergedPodContainers, err := merge.CombineStepsWithContainerTemplate(taskSpec.ContainerTemplate, podContainers) + mergedPodContainers, err := merge.CombineStepsWithStepTemplate(taskSpec.StepTemplate, podContainers) + if err != nil { + return nil, err + } + + // The ContainerTemplate field is deprecated (#977) + mergedInitContainers, err = merge.CombineStepsWithStepTemplate(taskSpec.ContainerTemplate, mergedInitContainers) + if err != nil { + return nil, err + } + mergedPodContainers, err = merge.CombineStepsWithStepTemplate(taskSpec.ContainerTemplate, mergedPodContainers) if err != nil { return nil, err } diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go index 9033014f2eb..5e8b5e26aa2 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go @@ -89,7 +89,11 @@ var ( saTask = tb.Task("test-with-sa", "foo", tb.TaskSpec(tb.Step("sa-step", "foo", tb.Command("/mycmd")))) taskEnvTask = tb.Task("test-task-env", "foo", tb.TaskSpec( + // The ContainerTemplate field is deprecated (#977) tb.TaskContainerTemplate( + tb.EnvVar("BREAD", "PUMPERNICKEL"), + ), + tb.TaskStepTemplate( tb.EnvVar("FRUIT", "APPLE"), ), tb.Step("env-step", "foo", @@ -904,8 +908,8 @@ func TestReconcile(t *testing.T) { tb.PodSpec( tb.PodVolumes(toolsVolume, workspaceVolume, homeVolume), tb.PodRestartPolicy(corev1.RestartPolicyNever), - getCredentialsInitContainer("9l9zj", tb.EnvVar("FRUIT", "APPLE")), - getPlaceToolsInitContainer(tb.EnvVar("FRUIT", "APPLE")), + getCredentialsInitContainer("9l9zj", tb.EnvVar("FRUIT", "APPLE"), tb.EnvVar("BREAD", "PUMPERNICKEL")), + getPlaceToolsInitContainer(tb.EnvVar("FRUIT", "APPLE"), tb.EnvVar("BREAD", "PUMPERNICKEL")), tb.PodContainer("step-env-step", "foo", tb.Command(entrypointLocation), tb.Command(entrypointLocation), tb.Args("-wait_file", "", "-post_file", "/builder/tools/0", "-entrypoint", "/mycmd", "--"), @@ -921,12 +925,14 @@ func TestReconcile(t *testing.T) { tb.VolumeMount("home", "/builder/home"), tb.EnvVar("ANOTHER", "VARIABLE"), tb.EnvVar("FRUIT", "LEMON"), + tb.EnvVar("BREAD", "PUMPERNICKEL"), ), tb.PodContainer("nop", "override-with-nop:latest", tb.Command("/builder/tools/entrypoint"), tb.Args("-wait_file", "/builder/tools/0", "-post_file", "/builder/tools/1", "-entrypoint", "/ko-app/nop", "--"), tb.VolumeMount(entrypoint.MountName, entrypoint.MountPoint), tb.EnvVar("FRUIT", "APPLE"), + tb.EnvVar("BREAD", "PUMPERNICKEL"), ), ), ), @@ -1614,7 +1620,7 @@ func TestUpdateStatusFromPod(t *testing.T) { }, { desc: "failure-terminated", podStatus: corev1.PodStatus{ - Phase: corev1.PodFailed, + Phase: corev1.PodFailed, InitContainerStatuses: []corev1.ContainerStatus{{ // creds-init status; ignored }}, @@ -1682,7 +1688,7 @@ func TestUpdateStatusFromPod(t *testing.T) { }, { desc: "pending-waiting-message", podStatus: corev1.PodStatus{ - Phase: corev1.PodPending, + Phase: corev1.PodPending, InitContainerStatuses: []corev1.ContainerStatus{{ // creds-init status; ignored }}, diff --git a/test/builder/task.go b/test/builder/task.go index 2891b3e5c7d..98ce8b86457 100644 --- a/test/builder/task.go +++ b/test/builder/task.go @@ -153,7 +153,20 @@ func Step(name, image string, ops ...ContainerOp) TaskSpecOp { } } -// TaskContainerTemplate adds a base container for all steps in the task. +// TaskStepTemplate adds a base container for all steps in the task. +func TaskStepTemplate(ops ...ContainerOp) TaskSpecOp { + return func(spec *v1alpha1.TaskSpec) { + base := &corev1.Container{} + + for _, op := range ops { + op(base) + } + spec.StepTemplate = base + } +} + +// TaskContainerTemplate adds the deprecated (#977) base container for +// all steps in the task. ContainerTemplate is now StepTemplate. func TaskContainerTemplate(ops ...ContainerOp) TaskSpecOp { return func(spec *v1alpha1.TaskSpec) { base := &corev1.Container{} diff --git a/test/builder/task_test.go b/test/builder/task_test.go index 4762568f457..54b4e0a6845 100644 --- a/test/builder/task_test.go +++ b/test/builder/task_test.go @@ -49,9 +49,13 @@ func TestTask(t *testing.T) { tb.TaskVolume("foo", tb.VolumeSource(corev1.VolumeSource{ HostPath: &corev1.HostPathVolumeSource{Path: "/foo/bar"}, })), - tb.TaskContainerTemplate( + tb.TaskStepTemplate( tb.EnvVar("FRUIT", "BANANA"), ), + // The ContainerTemplate field is deprecated (#977) + tb.TaskContainerTemplate( + tb.EnvVar("JUICE", "MELON"), + ), )) expectedTask := &v1alpha1.Task{ ObjectMeta: metav1.ObjectMeta{Name: "test-task", Namespace: "foo"}, @@ -82,12 +86,19 @@ func TestTask(t *testing.T) { HostPath: &corev1.HostPathVolumeSource{Path: "/foo/bar"}, }, }}, - ContainerTemplate: &corev1.Container{ + StepTemplate: &corev1.Container{ Env: []corev1.EnvVar{{ Name: "FRUIT", Value: "BANANA", }}, }, + // The ContainerTemplate field is deprecated (#977) + ContainerTemplate: &corev1.Container{ + Env: []corev1.EnvVar{{ + Name: "JUICE", + Value: "MELON", + }}, + }, }, } if d := cmp.Diff(expectedTask, task); d != "" {