merge podTemplates instead of override

The podTemplate can be specified for any given pipelineTask at the pipelineRun.spec.taskRunSpecs. The podTemplate can also be specified for the entire pipeline at the pipelineRun.spec.podTemplate. Before this commit, the podTemplates at the pipelineRun level were ignored if it was specified at the taskRunSpecs. This results in the settings specified at the pipelineRun level not included along with the pipelineTask specific configuration. For example, there could be a common setting at the pipelineRun level to set the User and Group for all the pipelineTasks and a pipelineTask can have its own diskType. Without this commit, the taskRun runs with a podTemplate with the specified diskType and no user or group configurations. Signed-off-by: Priti Desai <pdesai@us.ibm.com>
tektoncd · Jun 23, 2023 · 7277870 · 7277870
1 parent cc9562c
commit 7277870
Show file tree

Hide file tree

Showing 5 changed files with 156 additions and 7 deletions.
diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md
@@ -896,7 +896,8 @@ spec:
 {{< /tab >}}
 {{< /tabs >}}
 
-If used with this `Pipeline`,  `build-task` will use the task specific `PodTemplate` (where `nodeSelector` has `disktype` equal to `ssd`).
+If used with this `Pipeline`,  `build-task` will use the task specific `PodTemplate` (where `nodeSelector` has `disktype` equal to `ssd`)
+along with `securityContext` from the `pipelineRun.spec.podTemplate`.
 `PipelineTaskRunSpec` may also contain `StepSpecs` and `SidecarSpecs`; see
 [Overriding `Task` `Steps` and `Sidecars`](./taskruns.md#overriding-task-steps-and-sidecars) for more information.
 

diff --git a/pkg/apis/pipeline/v1/pipelinerun_types.go b/pkg/apis/pipeline/v1/pipelinerun_types.go
@@ -555,9 +555,9 @@ func (pr *PipelineRun) GetTaskRunSpec(pipelineTaskName string) PipelineTaskRunSp
 	}
 	for _, task := range pr.Spec.TaskRunSpecs {
 		if task.PipelineTaskName == pipelineTaskName {
-			if task.PodTemplate != nil {
-				s.PodTemplate = task.PodTemplate
-			}
+			// merge podTemplates specified in pipelineRun.spec.taskRunSpecs[].podTemplate and pipelineRun.spec.podTemplate
+			// with taskRunSpecs taking higher precedence
+			s.PodTemplate = pod.MergePodTemplateWithDefault(task.PodTemplate, s.PodTemplate)
 			if task.ServiceAccountName != "" {
 				s.ServiceAccountName = task.ServiceAccountName
 			}

diff --git a/pkg/apis/pipeline/v1/pipelinerun_types_test.go b/pkg/apis/pipeline/v1/pipelinerun_types_test.go
@@ -401,3 +401,78 @@ func TestPipelineRunGetPodSpec(t *testing.T) {
 		}
 	}
 }
+
+func TestPipelineRun_GetTaskRunSpec(t *testing.T) {
+	user := int64(1000)
+	group := int64(2000)
+	fsGroup := int64(3000)
+	for _, tt := range []struct {
+		name                 string
+		pr                   *v1.PipelineRun
+		expectedPodTemplates map[string]*pod.PodTemplate
+	}{
+		{
+			name: "pipelineRun Spec podTemplate and taskRunSpec pipelineTask podTemplate",
+			pr: &v1.PipelineRun{
+				ObjectMeta: metav1.ObjectMeta{Name: "pr"},
+				Spec: v1.PipelineRunSpec{
+					TaskRunTemplate: v1.PipelineTaskRunTemplate{
+						PodTemplate: &pod.Template{
+							SecurityContext: &corev1.PodSecurityContext{
+								RunAsUser:  &user,
+								RunAsGroup: &group,
+								FSGroup:    &fsGroup,
+							},
+						},
+						ServiceAccountName: "defaultSA",
+					},
+					PipelineRef: &v1.PipelineRef{Name: "prs"},
+					TaskRunSpecs: []v1.PipelineTaskRunSpec{{
+						PipelineTaskName:   "task-1",
+						ServiceAccountName: "task-1-service-account",
+						PodTemplate: &pod.Template{
+							NodeSelector: map[string]string{
+								"diskType": "ssd",
+							},
+						},
+					}, {
+						PipelineTaskName:   "task-2",
+						ServiceAccountName: "task-2-service-account",
+						PodTemplate: &pod.Template{
+							SchedulerName: "task-2-schedule",
+						},
+					}},
+				},
+			},
+			expectedPodTemplates: map[string]*pod.PodTemplate{
+				"task-1": {
+					SecurityContext: &corev1.PodSecurityContext{
+						RunAsUser:  &user,
+						RunAsGroup: &group,
+						FSGroup:    &fsGroup,
+					},
+					NodeSelector: map[string]string{
+						"diskType": "ssd",
+					},
+				},
+				"task-2": {
+					SecurityContext: &corev1.PodSecurityContext{
+						RunAsUser:  &user,
+						RunAsGroup: &group,
+						FSGroup:    &fsGroup,
+					},
+					SchedulerName: "task-2-schedule",
+				},
+			},
+		},
+	} {
+		for taskName := range tt.expectedPodTemplates {
+			t.Run(tt.name, func(t *testing.T) {
+				s := tt.pr.GetTaskRunSpec(taskName)
+				if d := cmp.Diff(tt.expectedPodTemplates[taskName], s.PodTemplate); d != "" {
+					t.Error(diff.PrintWantGot(d))
+				}
+			})
+		}
+	}
+}
diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go
@@ -618,9 +618,9 @@ func (pr *PipelineRun) GetTaskRunSpec(pipelineTaskName string) PipelineTaskRunSp
 	}
 	for _, task := range pr.Spec.TaskRunSpecs {
 		if task.PipelineTaskName == pipelineTaskName {
-			if task.TaskPodTemplate != nil {
-				s.TaskPodTemplate = task.TaskPodTemplate
-			}
+			// merge podTemplates specified in pipelineRun.spec.taskRunSpecs[].podTemplate and pipelineRun.spec.podTemplate
+			// with taskRunSpecs taking higher precedence
+			s.TaskPodTemplate = pod.MergePodTemplateWithDefault(task.TaskPodTemplate, s.TaskPodTemplate)
 			if task.TaskServiceAccountName != "" {
 				s.TaskServiceAccountName = task.TaskServiceAccountName
 			}

diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types_test.go
@@ -662,3 +662,76 @@ func TestPipelineRunGetPodSpec(t *testing.T) {
 		}
 	}
 }
+
+func TestPipelineRun_GetTaskRunSpec(t *testing.T) {
+	user := int64(1000)
+	group := int64(2000)
+	fsGroup := int64(3000)
+	for _, tt := range []struct {
+		name                 string
+		pr                   *v1beta1.PipelineRun
+		expectedPodTemplates map[string]*pod.PodTemplate
+	}{
+		{
+			name: "pipelineRun Spec podTemplate and taskRunSpec pipelineTask podTemplate",
+			pr: &v1beta1.PipelineRun{
+				ObjectMeta: metav1.ObjectMeta{Name: "pr"},
+				Spec: v1beta1.PipelineRunSpec{
+					PodTemplate: &pod.Template{
+						SecurityContext: &corev1.PodSecurityContext{
+							RunAsUser:  &user,
+							RunAsGroup: &group,
+							FSGroup:    &fsGroup,
+						},
+					},
+					PipelineRef:        &v1beta1.PipelineRef{Name: "prs"},
+					ServiceAccountName: "defaultSA",
+					TaskRunSpecs: []v1beta1.PipelineTaskRunSpec{{
+						PipelineTaskName:       "task-1",
+						TaskServiceAccountName: "task-1-service-account",
+						TaskPodTemplate: &pod.Template{
+							NodeSelector: map[string]string{
+								"diskType": "ssd",
+							},
+						},
+					}, {
+						PipelineTaskName:       "task-2",
+						TaskServiceAccountName: "task-2-service-account",
+						TaskPodTemplate: &pod.Template{
+							SchedulerName: "task-2-schedule",
+						},
+					}},
+				},
+			},
+			expectedPodTemplates: map[string]*pod.PodTemplate{
+				"task-1": {
+					SecurityContext: &corev1.PodSecurityContext{
+						RunAsUser:  &user,
+						RunAsGroup: &group,
+						FSGroup:    &fsGroup,
+					},
+					NodeSelector: map[string]string{
+						"diskType": "ssd",
+					},
+				},
+				"task-2": {
+					SecurityContext: &corev1.PodSecurityContext{
+						RunAsUser:  &user,
+						RunAsGroup: &group,
+						FSGroup:    &fsGroup,
+					},
+					SchedulerName: "task-2-schedule",
+				},
+			},
+		},
+	} {
+		for taskName := range tt.expectedPodTemplates {
+			t.Run(tt.name, func(t *testing.T) {
+				s := tt.pr.GetTaskRunSpec(taskName)
+				if d := cmp.Diff(tt.expectedPodTemplates[taskName], s.TaskPodTemplate); d != "" {
+					t.Error(diff.PrintWantGot(d))
+				}
+			})
+		}
+	}
+}