Remove deprecated podSpec field in favor of podTemplate 🛄

We introduced `podTemplate` in last release to specify podSpec specific field, and deprecated `nodeSelector`, `tolerations` and `affinity`. This removes those deprecated fields. Signed-off-by: Vincent Demeester <[email protected]>
bobcatfish · Sep 12, 2019 · 5bb204e · 5bb204e
1 parent 037a01b
commit 5bb204e
Show file tree

Hide file tree

Showing 9 changed files with 13 additions and 218 deletions.
diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go
@@ -68,19 +68,6 @@ type PipelineRunSpec struct {
 
 	// PodTemplate holds pod specific configuration
 	PodTemplate PodTemplate `json:"podTemplate,omitempty"`
-
-	// FIXME(vdemeester) Deprecated
-	// NodeSelector is a selector which must be true for the pod to fit on a node.
-	// Selector which must match a node's labels for the pod to be scheduled on that node.
-	// More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
-	// +optional
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
-	// If specified, the pod's tolerations.
-	// +optional
-	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
-	// If specified, the pod's scheduling constraints
-	// +optional
-	Affinity *corev1.Affinity `json:"affinity,omitempty"`
 }
 
 // PipelineRunSpecStatus defines the pipelinerun spec status the user can provide

diff --git a/pkg/apis/pipeline/v1alpha1/pod.go b/pkg/apis/pipeline/v1alpha1/pod.go
@@ -46,17 +46,3 @@ type PodTemplate struct {
 	// +optional
 	Volumes []corev1.Volume `json:"volumes,omitempty" patchStrategy:"merge,retainKeys" patchMergeKey:"name" protobuf:"bytes,1,rep,name=volumes"`
 }
-
-// CombinePodTemplate takes a PodTemplate (either from TaskRun or PipelineRun) and merge it with deprecated field that were inlined.
-func CombinedPodTemplate(template PodTemplate, deprecatedNodeSelector map[string]string, deprecatedTolerations []corev1.Toleration, deprecatedAffinity *corev1.Affinity) PodTemplate {
-	if len(template.NodeSelector) == 0 && len(deprecatedNodeSelector) != 0 {
-		template.NodeSelector = deprecatedNodeSelector
-	}
-	if len(template.Tolerations) == 0 && len(deprecatedTolerations) != 0 {
-		template.Tolerations = deprecatedTolerations
-	}
-	if template.Affinity == nil && deprecatedAffinity != nil {
-		template.Affinity = deprecatedAffinity
-	}
-	return template
-}
diff --git a/pkg/apis/pipeline/v1alpha1/pod_test.go b/pkg/apis/pipeline/v1alpha1/pod_test.go
@@ -15,125 +15,3 @@ limitations under the License.
 */
 
 package v1alpha1_test
-
-import (
-	"testing"
-
-	"github.com/google/go-cmp/cmp"
-	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
-	corev1 "k8s.io/api/core/v1"
-)
-
-func TestCombinedPodTemplateTakesPrecedence(t *testing.T) {
-	affinity := &corev1.Affinity{
-		NodeAffinity: &corev1.NodeAffinity{},
-	}
-	nodeSelector := map[string]string{
-		"banana": "chocolat",
-	}
-	tolerations := []corev1.Toleration{{
-		Key:   "banana",
-		Value: "chocolat",
-	}}
-
-	template := v1alpha1.PodTemplate{
-		NodeSelector: map[string]string{
-			"foo": "bar",
-			"bar": "baz",
-		},
-		Tolerations: []corev1.Toleration{{
-			Key:   "foo",
-			Value: "bar",
-		}},
-		Affinity: &corev1.Affinity{
-			PodAffinity: &corev1.PodAffinity{},
-		},
-	}
-	// Same as template above
-	want := v1alpha1.PodTemplate{
-		NodeSelector: map[string]string{
-			"foo": "bar",
-			"bar": "baz",
-		},
-		Tolerations: []corev1.Toleration{{
-			Key:   "foo",
-			Value: "bar",
-		}},
-		Affinity: &corev1.Affinity{
-			PodAffinity: &corev1.PodAffinity{},
-		},
-	}
-
-	got := v1alpha1.CombinedPodTemplate(template, nodeSelector, tolerations, affinity)
-	if d := cmp.Diff(got, want); d != "" {
-		t.Errorf("Diff:\n%s", d)
-	}
-}
-func TestCombinedPodTemplate(t *testing.T) {
-	nodeSelector := map[string]string{
-		"banana": "chocolat",
-	}
-	tolerations := []corev1.Toleration{{
-		Key:   "banana",
-		Value: "chocolat",
-	}}
-
-	template := v1alpha1.PodTemplate{
-		Tolerations: []corev1.Toleration{{
-			Key:   "foo",
-			Value: "bar",
-		}},
-		Affinity: &corev1.Affinity{
-			PodAffinity: &corev1.PodAffinity{},
-		},
-	}
-	// Same as template above
-	want := v1alpha1.PodTemplate{
-		NodeSelector: map[string]string{
-			"banana": "chocolat",
-		},
-		Tolerations: []corev1.Toleration{{
-			Key:   "foo",
-			Value: "bar",
-		}},
-		Affinity: &corev1.Affinity{
-			PodAffinity: &corev1.PodAffinity{},
-		},
-	}
-
-	got := v1alpha1.CombinedPodTemplate(template, nodeSelector, tolerations, nil)
-	if d := cmp.Diff(got, want); d != "" {
-		t.Errorf("Diff:\n%s", d)
-	}
-}
-
-func TestCombinedPodTemplateOnlyDeprecated(t *testing.T) {
-	template := v1alpha1.PodTemplate{}
-	affinity := &corev1.Affinity{
-		NodeAffinity: &corev1.NodeAffinity{},
-	}
-
-	nodeSelector := map[string]string{
-		"foo": "bar",
-	}
-	tolerations := []corev1.Toleration{{
-		Key:   "foo",
-		Value: "bar",
-	}}
-
-	want := v1alpha1.PodTemplate{
-		NodeSelector: map[string]string{
-			"foo": "bar",
-		},
-		Tolerations: []corev1.Toleration{{
-			Key:   "foo",
-			Value: "bar",
-		}},
-		Affinity: affinity,
-	}
-
-	got := v1alpha1.CombinedPodTemplate(template, nodeSelector, tolerations, affinity)
-	if d := cmp.Diff(got, want); d != "" {
-		t.Errorf("Diff:\n%s", d)
-	}
-}
diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_types.go b/pkg/apis/pipeline/v1alpha1/taskrun_types.go
@@ -59,19 +59,6 @@ type TaskRunSpec struct {
 
 	// PodTemplate holds pod specific configuration
 	PodTemplate PodTemplate `json:"podTemplate,omitempty"`
-
-	// FIXME(vdemeester) Deprecated
-	// NodeSelector is a selector which must be true for the pod to fit on a node.
-	// Selector which must match a node's labels for the pod to be scheduled on that node.
-	// More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
-	// +optional
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
-	// If specified, the pod's tolerations.
-	// +optional
-	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
-	// If specified, the pod's scheduling constraints
-	// +optional
-	Affinity *corev1.Affinity `json:"affinity,omitempty"`
 }
 
 // TaskRunSpecStatus defines the taskrun spec status the user can provide

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/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go
@@ -490,7 +490,6 @@ func (c *Reconciler) createTaskRun(rprt *resources.ResolvedPipelineRunTask, pr *
 		return c.PipelineClientSet.TektonV1alpha1().TaskRuns(pr.Namespace).UpdateStatus(tr)
 	}
 
-	podTemplate := v1alpha1.CombinedPodTemplate(pr.Spec.PodTemplate, pr.Spec.NodeSelector, pr.Spec.Tolerations, pr.Spec.Affinity)
 	tr = &v1alpha1.TaskRun{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:            rprt.TaskRunName,
@@ -509,7 +508,7 @@ func (c *Reconciler) createTaskRun(rprt *resources.ResolvedPipelineRunTask, pr *
 			},
 			ServiceAccount: getServiceAccount(pr, rprt.PipelineTask.Name),
 			Timeout:        getTaskRunTimeout(pr),
-			PodTemplate:    podTemplate,
+			PodTemplate:    pr.Spec.PodTemplate,
 		}}
 
 	resources.WrapSteps(&tr.Spec, rprt.PipelineTask, rprt.ResolvedTaskResources.Inputs, rprt.ResolvedTaskResources.Outputs, storageBasePath)
@@ -645,10 +644,8 @@ func (c *Reconciler) makeConditionCheckContainer(rprt *resources.ResolvedPipelin
 				Params:    rcc.PipelineTaskCondition.Params,
 				Resources: rcc.ToTaskResourceBindings(),
 			},
-			Timeout:      getTaskRunTimeout(pr),
-			NodeSelector: pr.Spec.NodeSelector,
-			Tolerations:  pr.Spec.Tolerations,
-			Affinity:     pr.Spec.Affinity,
+			Timeout:     getTaskRunTimeout(pr),
+			PodTemplate: pr.Spec.PodTemplate,
 		}}
 
 	cctr, err := c.PipelineClientSet.TektonV1alpha1().TaskRuns(pr.Namespace).Create(tr)

diff --git a/pkg/reconciler/taskrun/resources/pod.go b/pkg/reconciler/taskrun/resources/pod.go
@@ -331,8 +331,6 @@ func MakePod(taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient k
 		mergedPodContainers = append(mergedPodContainers, taskSpec.Sidecars...)
 	}
 
-	podTemplate := v1alpha1.CombinedPodTemplate(taskRun.Spec.PodTemplate, taskRun.Spec.NodeSelector, taskRun.Spec.Tolerations, taskRun.Spec.Affinity)
-
 	return &corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			// We execute the build's pod in the same namespace as where the build was
@@ -356,10 +354,10 @@ func MakePod(taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient k
 			Containers:         mergedPodContainers,
 			ServiceAccountName: taskRun.Spec.ServiceAccount,
 			Volumes:            volumes,
-			NodeSelector:       podTemplate.NodeSelector,
-			Tolerations:        podTemplate.Tolerations,
-			Affinity:           podTemplate.Affinity,
-			SecurityContext:    podTemplate.SecurityContext,
+			NodeSelector:       taskRun.Spec.PodTemplate.NodeSelector,
+			Tolerations:        taskRun.Spec.PodTemplate.Tolerations,
+			Affinity:           taskRun.Spec.PodTemplate.Affinity,
+			SecurityContext:    taskRun.Spec.PodTemplate.SecurityContext,
 		},
 	}, nil
 }

diff --git a/test/builder/pipeline.go b/test/builder/pipeline.go
@@ -380,21 +380,21 @@ func PipelineRunNilTimeout(prs *v1alpha1.PipelineRunSpec) {
 // PipelineRunNodeSelector sets the Node selector to the PipelineSpec.
 func PipelineRunNodeSelector(values map[string]string) PipelineRunSpecOp {
 	return func(prs *v1alpha1.PipelineRunSpec) {
-		prs.NodeSelector = values
+		prs.PodTemplate.NodeSelector = values
 	}
 }
 
 // PipelineRunTolerations sets the Node selector to the PipelineSpec.
 func PipelineRunTolerations(values []corev1.Toleration) PipelineRunSpecOp {
 	return func(prs *v1alpha1.PipelineRunSpec) {
-		prs.Tolerations = values
+		prs.PodTemplate.Tolerations = values
 	}
 }
 
 // PipelineRunAffinity sets the affinity to the PipelineSpec.
 func PipelineRunAffinity(affinity *corev1.Affinity) PipelineRunSpecOp {
 	return func(prs *v1alpha1.PipelineRunSpec) {
-		prs.Affinity = affinity
+		prs.PodTemplate.Affinity = affinity
 	}
 }
 

diff --git a/test/builder/task.go b/test/builder/task.go
@@ -368,21 +368,21 @@ func TaskRunNilTimeout(spec *v1alpha1.TaskRunSpec) {
 // TaskRunNodeSelector sets the NodeSelector to the PipelineSpec.
 func TaskRunNodeSelector(values map[string]string) TaskRunSpecOp {
 	return func(spec *v1alpha1.TaskRunSpec) {
-		spec.NodeSelector = values
+		spec.PodTemplate.NodeSelector = values
 	}
 }
 
 // TaskRunTolerations sets the Tolerations to the PipelineSpec.
 func TaskRunTolerations(values []corev1.Toleration) TaskRunSpecOp {
 	return func(spec *v1alpha1.TaskRunSpec) {
-		spec.Tolerations = values
+		spec.PodTemplate.Tolerations = values
 	}
 }
 
 // TaskRunAffinity sets the Affinity to the PipelineSpec.
 func TaskRunAffinity(affinity *corev1.Affinity) TaskRunSpecOp {
 	return func(spec *v1alpha1.TaskRunSpec) {
-		spec.Affinity = affinity
+		spec.PodTemplate.Affinity = affinity
 	}
 }