merge podTemplates more specific

pkg/apis/pipeline/pod/template.go

@@ -223,6 +223,62 @@ func MergePodTemplateWithDefault(tpl, defaultTpl *PodTemplate) *PodTemplate {
  }
 }
 
+// Similar to MergePodTemplateWithDefault, but more fine-grained. When the value
+// type is map or slice, we will merge their values first. If there is the same
+// value, the value from tpl will overwrite the value from defaultTpl.
+func ExpandedMergePodTemplateWithDefault(tpl, defaultTpl *PodTemplate) *PodTemplate {
+ switch {
+ case defaultTpl == nil:
+ // No configured default, just return the template
+ return tpl
+ case tpl == nil:
+ // No template, just return the default template
+ return defaultTpl
+ default:
+ // Otherwise, merge fields
+ if tpl.NodeSelector==nil{
+ tpl.NodeSelector = defaultTpl.NodeSelector
+ }else{
+ tpl.NodeSelector = mergeMaps(defaultTpl.NodeSelector, tpl.NodeSelector)
+ }
+
+ tpl.Env = mergeByName(defaultTpl.Env, tpl.Env)
+ tpl.Tolerations = mergeByName(defaultTpl.Tolerations, tpl.Tolerations)
+ if tpl.SecurityContext == nil {
+ tpl.SecurityContext = defaultTpl.SecurityContext
+ }
+ tpl.Volumes = mergeByName(defaultTpl.Volumes, tpl.Volumes)
+ if tpl.RuntimeClassName == nil {
+ tpl.RuntimeClassName = defaultTpl.RuntimeClassName
+ }
+ if tpl.AutomountServiceAccountToken == nil {
+ tpl.AutomountServiceAccountToken = defaultTpl.AutomountServiceAccountToken
+ }
+ if tpl.DNSPolicy == nil {
+ tpl.DNSPolicy = defaultTpl.DNSPolicy
+ }
+ if tpl.DNSConfig == nil {
+ tpl.DNSConfig = defaultTpl.DNSConfig
+ }
+ if tpl.EnableServiceLinks == nil {
+ tpl.EnableServiceLinks = defaultTpl.EnableServiceLinks
+ }
+ if tpl.PriorityClassName == nil {
+ tpl.PriorityClassName = defaultTpl.PriorityClassName
+ }
+ if tpl.SchedulerName == "" {
+ tpl.SchedulerName = defaultTpl.SchedulerName
+ }
+ tpl.ImagePullSecrets = mergeByName(defaultTpl.ImagePullSecrets, tpl.ImagePullSecrets)
+ tpl.HostAliases = mergeByName(defaultTpl.HostAliases, tpl.HostAliases)
+ if !tpl.HostNetwork && defaultTpl.HostNetwork {
+ tpl.HostNetwork = true
+ }
+ tpl.TopologySpreadConstraints = mergeByName(defaultTpl.TopologySpreadConstraints, tpl.TopologySpreadConstraints)
+ return tpl
+ }
+}
+
 // AAPodTemplate holds pod specific configuration for the affinity-assistant
 type AAPodTemplate = AffinityAssistantTemplate
 
@@ -251,6 +307,24 @@ func MergeAAPodTemplateWithDefault(tpl, defaultTpl *AAPodTemplate) *AAPodTemplat
  }
 }
 
+// mergeMaps takes two maps with the same key and value types
+// and merges them, giving priority to the items in the override map.
+func mergeMaps[K comparable, V any](base, overrides map[K]V) map[K]V {
+ result := make(map[K]V)
+
+ // Add all elements from base to the result
+ for key, value := range base {
+ result[key] = value
+ }
+
+ // Add all elements from overrides to the result, overwriting any existing keys
+ for key, value := range overrides {
+ result[key] = value
+ }
+
+ return result
+}
+
 // mergeByName merges two slices of items with names based on the getName
 // function, giving priority to the items in the override slice.
 func mergeByName[T any](base, overrides []T) []T {
@@ -292,6 +366,14 @@ func getName(item interface{}) string {
  return item.Name
  case corev1.Volume:
  return item.Name
+ case corev1.Toleration:
+ return item.Key
+ case corev1.TopologySpreadConstraint:
+ return item.TopologyKey
+ case corev1.HostAlias:
+ return item.IP
+ case corev1.LocalObjectReference:
+ return item.Name
  default:
  return ""
  }

pkg/apis/pipeline/pod/template_test.go

@@ -168,3 +168,107 @@ func TestMergePodTemplateWithDefault(t *testing.T) {
  })
  }
 }
+
+func TestExpandedMergePodTemplateWithDefault(t *testing.T) {
+ type testCase struct {
+ name string
+ tpl *PodTemplate
+ defaultTpl *PodTemplate
+ expected *PodTemplate
+ }
+
+ testCases := []testCase{
+ {
+ name: "defaultTpl is nil",
+ tpl: &PodTemplate{
+ NodeSelector: map[string]string{"foo": "bar"},
+ },
+ defaultTpl: nil,
+ expected: &PodTemplate{
+ NodeSelector: map[string]string{"foo": "bar"},
+ },
+ },
+ {
+ name: "tpl is nil",
+ tpl: nil,
+ defaultTpl: &PodTemplate{
+ NodeSelector: map[string]string{"foo": "bar"},
+ },
+ expected: &PodTemplate{
+ NodeSelector: map[string]string{"foo": "bar"},
+ },
+ },
+ {
+ name: "override default env",
+ tpl: &PodTemplate{
+ Env: []corev1.EnvVar{{Name: "foo", Value: "bar"}},
+ },
+ defaultTpl: &PodTemplate{
+ Env: []corev1.EnvVar{{Name: "foo", Value: "baz"}},
+ },
+ expected: &PodTemplate{
+ Env: []corev1.EnvVar{{Name: "foo", Value: "bar"}},
+ },
+ },
+ {
+ name: "merge envs",
+ tpl: &PodTemplate{
+ Env: []corev1.EnvVar{{Name: "foo", Value: "bar"}},
+ },
+ defaultTpl: &PodTemplate{
+ Env: []corev1.EnvVar{{Name: "bar", Value: "bar"}},
+ },
+ expected: &PodTemplate{
+ Env: []corev1.EnvVar{
+ {Name: "foo", Value: "bar"},
+ {Name: "bar", Value: "bar"},
+ },
+ },
+ },
+ {
+ name: "override default nodeselector",
+ tpl: &PodTemplate{
+ NodeSelector: map[string]string{"disktype": "ssd"},
+ },
+ defaultTpl: &PodTemplate{
+ NodeSelector: map[string]string{"disktype": "hdd"},
+ },
+ expected: &PodTemplate{
+ NodeSelector: map[string]string{"disktype": "ssd"},
+ },
+ },
+ {
+ name: "merge nodeselector",
+ tpl: &PodTemplate{
+ NodeSelector: map[string]string{"kubernetes.io/hostname": "node-1"},
+ },
+ defaultTpl: &PodTemplate{
+ NodeSelector: map[string]string{"kubernetes.io/os": "linux"},
+ },
+ expected: &PodTemplate{
+ NodeSelector: map[string]string{"kubernetes.io/hostname": "node-1", "kubernetes.io/os": "linux"},
+ },
+ },
+ {
+ name: "update host network",
+ tpl: &PodTemplate{
+ HostNetwork: false,
+ },
+ defaultTpl: &PodTemplate{
+ HostNetwork: true,
+ },
+ expected: &PodTemplate{
+ HostNetwork: true,
+ },
+ },
+ }
+
+ for _, tc := range testCases {
+ t.Run(tc.name, func(t *testing.T) {
+ result := ExpandedMergePodTemplateWithDefault(tc.tpl, tc.defaultTpl)
+ if !reflect.DeepEqual(result, tc.expected) {
+ t.Errorf("mergeByName(%v, %v) = %v, want %v", tc.tpl, tc.defaultTpl, result, tc.expected)
+ }
+ })
+ }
+}

pkg/apis/pipeline/v1/pipelinerun_types.go

@@ -660,7 +660,7 @@ func (pr *PipelineRun) GetTaskRunSpec(pipelineTaskName string) PipelineTaskRunSp
  if task.PipelineTaskName == pipelineTaskName {
  // 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)
+ s.PodTemplate = pod.ExpandedMergePodTemplateWithDefault(task.PodTemplate, s.PodTemplate)
  if task.ServiceAccountName != "" {
  s.ServiceAccountName = task.ServiceAccountName
  }

pkg/apis/pipeline/v1/pipelinerun_types_test.go

@@ -689,6 +689,116 @@ func TestPipelineRun_GetTaskRunSpec(t *testing.T) {
  SchedulerName: "task-2-schedule",
  },
  },
+ }, {
+ name: "verify whether tolerations are merged correctly",
+ pr: &v1.PipelineRun{
+ ObjectMeta: metav1.ObjectMeta{Name: "pr"},
+ Spec: v1.PipelineRunSpec{
+ TaskRunTemplate: v1.PipelineTaskRunTemplate{
+ PodTemplate: &pod.Template{
+ Tolerations: []corev1.Toleration{
+ {
+ Key: "arch",
+ Operator: corev1.TolerationOpEqual,
+ Effect: corev1.TaintEffectPreferNoSchedule,
+ Value: "arm64",
+ },
+ },
+ },
+ ServiceAccountName: "defaultSA",
+ },
+ PipelineRef: &v1.PipelineRef{Name: "prs"},
+ TaskRunSpecs: []v1.PipelineTaskRunSpec{{
+ PipelineTaskName: "different-tolerations",
+ ServiceAccountName: "different-tolerations-service-account",
+ PodTemplate: &pod.Template{
+ Tolerations: []corev1.Toleration{
+ {
+ Key: "pipeline-priority",
+ Operator: corev1.TolerationOpEqual,
+ Effect: corev1.TaintEffectNoSchedule,
+ Value: "low",
+ },
+ },
+ },
+ }, {
+ PipelineTaskName: "same-tolerations",
+ ServiceAccountName: "same-tolerations-service-account",
+ PodTemplate: &pod.Template{
+ Tolerations: []corev1.Toleration{
+ {
+ Key: "arch",
+ Operator: corev1.TolerationOpEqual,
+ Effect: corev1.TaintEffectPreferNoSchedule,
+ Value: "amd64",
+ },
+ },
+ },
+ }},
+ },
+ },
+ expectedPodTemplates: map[string]*pod.PodTemplate{
+ "different-tolerations": {
+ Tolerations: []corev1.Toleration{
+ {
+ Key: "pipeline-priority",
+ Operator: corev1.TolerationOpEqual,
+ Effect: corev1.TaintEffectNoSchedule,
+ Value: "low",
+ }, {
+ Key: "arch",
+ Operator: corev1.TolerationOpEqual,
+ Effect: corev1.TaintEffectPreferNoSchedule,
+ Value: "arm64",
+ },
+ },
+ },
+ "same-tolerations": {
+ Tolerations: []corev1.Toleration{
+ {
+ Key: "arch",
+ Operator: corev1.TolerationOpEqual,
+ Effect: corev1.TaintEffectPreferNoSchedule,
+ Value: "amd64",
+ },
+ },
+ },
+ },
+ }, {
+ name: "verify whether nodeselector are merged correctly",
+ pr: &v1.PipelineRun{
+ ObjectMeta: metav1.ObjectMeta{Name: "pr"},
+ Spec: v1.PipelineRunSpec{
+ TaskRunTemplate: v1.PipelineTaskRunTemplate{
+ PodTemplate: &pod.Template{
+ NodeSelector: map[string]string{"disktype": "ssd"},
+ },
+ ServiceAccountName: "defaultSA",
+ },
+ PipelineRef: &v1.PipelineRef{Name: "prs"},
+ TaskRunSpecs: []v1.PipelineTaskRunSpec{{
+ PipelineTaskName: "different-nodeselector",
+ ServiceAccountName: "different-nodeselector-service-account",
+ PodTemplate: &pod.Template{
+ NodeSelector: map[string]string{"disktype": "hdd"},
+ },
+ }, {
+ PipelineTaskName: "same-nodeselector",
+ ServiceAccountName: "same-nodeselector-service-account",
+ PodTemplate: &pod.Template{
+ NodeSelector: map[string]string{"kubernetes.io/os": "linux"},
+ },
+ }},
+ },
+ },
+ expectedPodTemplates: map[string]*pod.PodTemplate{
+ "different-nodeselector": {
+ NodeSelector: map[string]string{"disktype": "hdd"},
+ },
+ "same-nodeselector": {
+ NodeSelector: map[string]string{"disktype": "ssd", "kubernetes.io/os": "linux"},
+ },
+ },
  },
  } {
  for taskName := range tt.expectedPodTemplates {

pkg/apis/pipeline/v1beta1/pipelinerun_types.go

@@ -624,7 +624,7 @@ func (pr *PipelineRun) GetTaskRunSpec(pipelineTaskName string) PipelineTaskRunSp
  if task.PipelineTaskName == pipelineTaskName {
  // 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)
+ s.TaskPodTemplate = pod.ExpandedMergePodTemplateWithDefault(task.TaskPodTemplate, s.TaskPodTemplate)
  if task.TaskServiceAccountName != "" {
  s.TaskServiceAccountName = task.TaskServiceAccountName
  }