Skip to content

Commit

Permalink
Rename containerTemplate to stepTemplate but support both 👠
Browse files Browse the repository at this point in the history
We used the term `containerTemplate` for this template b/c it is a
template of the containers used for each step - however we don't refer
to those as containers, we refer to them as steps, so I think calling
this `stepTemplate` makes slightly more sense and makes the relationship
between this template + the steps a bit more obvious.

This change is backward compatible b/c both `containerTemplate` and
`stepTemplate` will be supported until #977 where we will make the
backward incompatible change and remove `containerTemplate`.
  • Loading branch information
bobcatfish committed Jun 11, 2019
1 parent ccf4757 commit ab7669f
Show file tree
Hide file tree
Showing 11 changed files with 115 additions and 25 deletions.
15 changes: 10 additions & 5 deletions docs/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -326,19 +328,19 @@ For example, use volumes to accomplish one of the following common tasks:
unsafe_. Use [kaniko](https:/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"
Expand All @@ -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:/tektoncd/pipeline/issues/977))._

### Templating

`Tasks` support templating using values from all [`inputs`](#inputs) and
Expand Down
6 changes: 5 additions & 1 deletion pkg/apis/pipeline/v1alpha1/task_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand Down
15 changes: 12 additions & 3 deletions pkg/apis/pipeline/v1alpha1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
}
}

Expand Down
34 changes: 33 additions & 1 deletion pkg/apis/pipeline/v1alpha1/task_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func TestTaskSpecValidate(t *testing.T) {
Inputs *Inputs
Outputs *Outputs
BuildSteps []corev1.Container
StepTemplate *corev1.Container
ContainerTemplate *corev1.Container
}
tests := []struct {
Expand Down Expand Up @@ -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",
},
Expand All @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pkg/merge/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/merge/merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand Down Expand Up @@ -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)
}
Expand Down
14 changes: 12 additions & 2 deletions pkg/reconciler/v1alpha1/taskrun/resources/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
14 changes: 10 additions & 4 deletions pkg/reconciler/v1alpha1/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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", "--"),
Expand All @@ -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"),
),
),
),
Expand Down Expand Up @@ -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
}},
Expand Down Expand Up @@ -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
}},
Expand Down
15 changes: 14 additions & 1 deletion test/builder/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
15 changes: 13 additions & 2 deletions test/builder/task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down Expand Up @@ -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 != "" {
Expand Down

0 comments on commit ab7669f

Please sign in to comment.