From 5bb204e789b5120cc91a9b165be5591e112022fb Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Thu, 12 Sep 2019 09:51:58 +0200 Subject: [PATCH] =?UTF-8?q?Remove=20deprecated=20podSpec=20field=20in=20fa?= =?UTF-8?q?vor=20of=20podTemplate=20=F0=9F=9B=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../pipeline/v1alpha1/pipelinerun_types.go | 13 -- pkg/apis/pipeline/v1alpha1/pod.go | 14 -- pkg/apis/pipeline/v1alpha1/pod_test.go | 122 ------------------ pkg/apis/pipeline/v1alpha1/taskrun_types.go | 13 -- .../v1alpha1/zz_generated.deepcopy.go | 38 ------ pkg/reconciler/pipelinerun/pipelinerun.go | 9 +- pkg/reconciler/taskrun/resources/pod.go | 10 +- test/builder/pipeline.go | 6 +- test/builder/task.go | 6 +- 9 files changed, 13 insertions(+), 218 deletions(-) diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go index 857e098a7a0..b6cef00be9d 100644 --- 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 index f348a788e4e..c4e61458ac3 100644 --- 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 index 1a3c61d6e59..7bcc4bdd58b 100644 --- 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 index 61c6bbb5535..ae60cfd2049 100644 --- 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 index 125ef8f04d8..08aafcd1c78 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -969,25 +969,6 @@ func (in *PipelineRunSpec) DeepCopyInto(out *PipelineRunSpec) { **out = **in } in.PodTemplate.DeepCopyInto(&out.PodTemplate) - if in.NodeSelector != nil { - in, out := &in.NodeSelector, &out.NodeSelector - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } - if in.Tolerations != nil { - in, out := &in.Tolerations, &out.Tolerations - *out = make([]v1.Toleration, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } - if in.Affinity != nil { - in, out := &in.Affinity, &out.Affinity - *out = new(v1.Affinity) - (*in).DeepCopyInto(*out) - } return } @@ -1741,25 +1722,6 @@ func (in *TaskRunSpec) DeepCopyInto(out *TaskRunSpec) { **out = **in } in.PodTemplate.DeepCopyInto(&out.PodTemplate) - if in.NodeSelector != nil { - in, out := &in.NodeSelector, &out.NodeSelector - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } - if in.Tolerations != nil { - in, out := &in.Tolerations, &out.Tolerations - *out = make([]v1.Toleration, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } - if in.Affinity != nil { - in, out := &in.Affinity, &out.Affinity - *out = new(v1.Affinity) - (*in).DeepCopyInto(*out) - } return } diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index ff4e9e5ef78..ca7d64390c2 100644 --- 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 index 978be2aafd9..189af68616e 100644 --- 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 index 9a3750409eb..82dc4c7068b 100644 --- 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 index d150215afd1..e7227ca5f7d 100644 --- 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 } }