Remove containerTemplate (now called stepTemplate) 👠

In tektoncd#931 we added `stepTemplate` in addition to `containerTemplate` and this chagne went out with 0.5. In this release we can now remove containerTemplate completely. Fixes tektoncd#977
bobcatfish · Aug 7, 2019 · 1d527ec · 1d527ec
1 parent cff3043
commit 1d527ec
Show file tree

Hide file tree

Showing 11 changed files with 10 additions and 156 deletions.
diff --git a/docs/tasks.md b/docs/tasks.md
@@ -77,7 +77,6 @@ following fields:
     available to your `Task`'s steps.
   - [`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
@@ -370,9 +369,6 @@ 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
@@ -56,10 +56,6 @@ type TaskSpec struct {
 	// 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"`
 }
 
 // Check that Task may be validated and defaulted.

diff --git a/pkg/apis/pipeline/v1alpha1/task_validation.go b/pkg/apis/pipeline/v1alpha1/task_validation.go
@@ -55,15 +55,6 @@ func (ts *TaskSpec) Validate(ctx context.Context) *apis.FieldError {
 		}
 	}
 
-	// The ContainerTemplate field is deprecated (#977)
-	mergedSteps, err = merge.CombineStepsWithStepTemplate(ts.ContainerTemplate, mergedSteps)
-	if err != nil {
-		return &apis.FieldError{
-			Message: fmt.Sprintf("error merging containerTemplate and steps: %s", err),
-			Paths:   []string{"stepTemplate"},
-		}
-	}
-
 	if err := validateSteps(mergedSteps).ViaField("steps"); err != nil {
 		return err
 	}

diff --git a/pkg/apis/pipeline/v1alpha1/task_validation_test.go b/pkg/apis/pipeline/v1alpha1/task_validation_test.go
@@ -51,11 +51,10 @@ var invalidBuildSteps = []corev1.Container{{
 
 func TestTaskSpecValidate(t *testing.T) {
 	type fields struct {
-		Inputs            *v1alpha1.Inputs
-		Outputs           *v1alpha1.Outputs
-		BuildSteps        []corev1.Container
-		StepTemplate      *corev1.Container
-		ContainerTemplate *corev1.Container
+		Inputs       *v1alpha1.Inputs
+		Outputs      *v1alpha1.Outputs
+		BuildSteps   []corev1.Container
+		StepTemplate *corev1.Container
 	}
 	tests := []struct {
 		name   string
@@ -193,45 +192,14 @@ func TestTaskSpecValidate(t *testing.T) {
 				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",
-			},
-		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ts := &v1alpha1.TaskSpec{
-				Inputs:            tt.fields.Inputs,
-				Outputs:           tt.fields.Outputs,
-				Steps:             tt.fields.BuildSteps,
-				StepTemplate:      tt.fields.StepTemplate,
-				ContainerTemplate: tt.fields.ContainerTemplate,
+				Inputs:       tt.fields.Inputs,
+				Outputs:      tt.fields.Outputs,
+				Steps:        tt.fields.BuildSteps,
+				StepTemplate: tt.fields.StepTemplate,
 			}
 			ctx := context.Background()
 			ts.SetDefaults(ctx)

diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/apply.go b/pkg/reconciler/v1alpha1/taskrun/resources/apply.go
@@ -76,12 +76,6 @@ func ApplyReplacements(spec *v1alpha1.TaskSpec, stringReplacements map[string]st
 		templating.ApplyContainerReplacements(&steps[i], stringReplacements, arrayReplacements)
 	}
 
-	// Apply variable expansion to containerTemplate fields.
-	// Should eventually be removed; ContainerTemplate is the deprecated previous name of the StepTemplate field (#977).
-	if spec.ContainerTemplate != nil {
-		templating.ApplyContainerReplacements(spec.ContainerTemplate, stringReplacements, arrayReplacements)
-	}
-
 	// Apply variable expansion to stepTemplate fields.
 	if spec.StepTemplate != nil {
 		templating.ApplyContainerReplacements(spec.StepTemplate, stringReplacements, arrayReplacements)

diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/apply_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/apply_test.go
@@ -81,27 +81,6 @@ var envTaskSpec = &v1alpha1.TaskSpec{
 	}},
 }
 
-// containerTemplate is deprecated but is functional (and tested) for now (#977).
-var containerTemplateTaskSpec = &v1alpha1.TaskSpec{
-	ContainerTemplate: &corev1.Container{
-		Env: []corev1.EnvVar{{
-			Name:  "template-var",
-			Value: "${inputs.params.FOO}",
-		}},
-	},
-	Steps: []corev1.Container{{
-		Name:  "simple-image",
-		Image: "${inputs.params.myimage}",
-	}, {
-		Name:  "image-with-env-specified",
-		Image: "some-other-image",
-		Env: []corev1.EnvVar{{
-			Name:  "template-var",
-			Value: "overridden-value",
-		}},
-	}},
-}
-
 var stepTemplateTaskSpec = &v1alpha1.TaskSpec{
 	StepTemplate: &corev1.Container{
 		Env: []corev1.EnvVar{{
@@ -486,32 +465,6 @@ func TestApplyParameters(t *testing.T) {
 			spec.Steps[0].EnvFrom[1].SecretRef.LocalObjectReference.Name = "secret-world"
 			spec.Steps[0].Image = "busybox:world"
 		}),
-	}, {
-		// containerTemplate is deprecated but is functional (and tested) for now (#977).
-		name: "containerTemplate parameter",
-		args: args{
-			ts: containerTemplateTaskSpec,
-			tr: &v1alpha1.TaskRun{
-				Spec: v1alpha1.TaskRunSpec{
-					Inputs: v1alpha1.TaskRunInputs{
-						Params: []v1alpha1.Param{{
-							Name:  "FOO",
-							Value: *builder.ArrayOrString("BAR"),
-						}},
-					},
-				},
-			},
-			dp: []v1alpha1.ParamSpec{
-				{
-					Name:    "myimage",
-					Default: builder.ArrayOrString("replaced-image-name"),
-				},
-			},
-		},
-		want: applyMutation(containerTemplateTaskSpec, func(spec *v1alpha1.TaskSpec) {
-			spec.ContainerTemplate.Env[0].Value = "BAR"
-			spec.Steps[0].Image = "replaced-image-name"
-		}),
 	}, {
 		name: "stepTemplate parameter",
 		args: args{

diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/pod.go b/pkg/reconciler/v1alpha1/taskrun/resources/pod.go
@@ -324,16 +324,6 @@ func MakePod(taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient k
 		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
-	}
-
 	podTemplate := v1alpha1.CombinedPodTemplate(taskRun.Spec.PodTemplate, taskRun.Spec.NodeSelector, taskRun.Spec.Tolerations, taskRun.Spec.Affinity)
 
 	return &corev1.Pod{

diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go
@@ -100,10 +100,6 @@ 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"),
 		),
@@ -894,8 +890,8 @@ func TestReconcile(t *testing.T) {
 			tb.PodSpec(
 				tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume),
 				tb.PodRestartPolicy(corev1.RestartPolicyNever),
-				getCredentialsInitContainer("9l9zj", tb.EnvVar("FRUIT", "APPLE"), tb.EnvVar("BREAD", "PUMPERNICKEL")),
-				getPlaceToolsInitContainer(tb.EnvVar("FRUIT", "APPLE"), tb.EnvVar("BREAD", "PUMPERNICKEL")),
+				getCredentialsInitContainer("9l9zj", tb.EnvVar("FRUIT", "APPLE")),
+				getPlaceToolsInitContainer(tb.EnvVar("FRUIT", "APPLE")),
 				tb.PodContainer("step-env-step", "foo", tb.Command(entrypointLocation),
 					tb.Command(entrypointLocation),
 					tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/mycmd", "--"),
@@ -912,7 +908,6 @@ func TestReconcile(t *testing.T) {
 					tb.VolumeMount("home", "/builder/home"),
 					tb.EnvVar("ANOTHER", "VARIABLE"),
 					tb.EnvVar("FRUIT", "LEMON"),
-					tb.EnvVar("BREAD", "PUMPERNICKEL"),
 				),
 			),
 		),

diff --git a/test/builder/task.go b/test/builder/task.go
@@ -166,19 +166,6 @@ func TaskStepTemplate(ops ...ContainerOp) TaskSpecOp {
 	}
 }
 
-// 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{}
-
-		for _, op := range ops {
-			op(base)
-		}
-		spec.ContainerTemplate = base
-	}
-}
-
 // TaskVolume adds a volume with specified name to the TaskSpec.
 // Any number of Volume modifier can be passed to transform it.
 func TaskVolume(name string, ops ...VolumeOp) TaskSpecOp {

diff --git a/test/builder/task_test.go b/test/builder/task_test.go
@@ -57,10 +57,6 @@ func TestTask(t *testing.T) {
 		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"},
@@ -106,13 +102,6 @@ func TestTask(t *testing.T) {
 					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 != "" {