From bb855711f077b43c7948f37f5406bff69b4b29de Mon Sep 17 00:00:00 2001 From: Christie Wilson Date: Wed, 7 Aug 2019 15:37:27 -0400 Subject: [PATCH] =?UTF-8?q?Remove=20containerTemplate=20(now=20called=20st?= =?UTF-8?q?epTemplate)=20=F0=9F=91=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In #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 #977 --- docs/tasks.md | 4 -- pkg/apis/pipeline/v1alpha1/task_types.go | 4 -- pkg/apis/pipeline/v1alpha1/task_validation.go | 9 ---- .../pipeline/v1alpha1/task_validation_test.go | 48 ++++--------------- .../v1alpha1/zz_generated.deepcopy.go | 5 -- .../v1alpha1/taskrun/resources/apply.go | 6 --- .../v1alpha1/taskrun/resources/apply_test.go | 47 ------------------ .../v1alpha1/taskrun/resources/pod.go | 10 ---- .../v1alpha1/taskrun/taskrun_test.go | 9 +--- test/builder/task.go | 13 ----- test/builder/task_test.go | 11 ----- 11 files changed, 10 insertions(+), 156 deletions(-) diff --git a/docs/tasks.md b/docs/tasks.md index 1ebb8d2323f..ab3e155e7d7 100644 --- 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 index f54672cc13d..c933f549f1a 100644 --- 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 index 3e8b9fe995a..4fb60d04d61 100644 --- 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 index 62f4d3447f9..70c663ec21d 100644 --- 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 index 8600dc6cbe7..7e53c3174ae 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -1801,11 +1801,6 @@ func (in *TaskSpec) DeepCopyInto(out *TaskSpec) { *out = new(v1.Container) (*in).DeepCopyInto(*out) } - if in.ContainerTemplate != nil { - in, out := &in.ContainerTemplate, &out.ContainerTemplate - *out = new(v1.Container) - (*in).DeepCopyInto(*out) - } return } diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/apply.go b/pkg/reconciler/v1alpha1/taskrun/resources/apply.go index e7554cc262c..31ce4349bd8 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/apply.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/apply.go @@ -97,12 +97,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 index f48aaf94a20..b34ca78a0dc 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/apply_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/apply_test.go @@ -95,27 +95,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{{ @@ -500,32 +479,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 index 0d592dd8bf0..cef6714b5a5 100644 --- 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 index 313169d7355..eb2e607b6a3 100644 --- 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 index 56dde167eaa..9bb7c6f531f 100644 --- 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 index f1d7daeb7e9..efe96450253 100644 --- 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 != "" {