Skip to content

Commit

Permalink
fix: ensure global podTemplate configuration is merged correctly
Browse files Browse the repository at this point in the history
fix #7551

Adjust the behavior of how global Pod templates are merged with TaskRun or PipelineRun.
The `env` and `volumes` fields are merged by the `name` value in the array elements.
  • Loading branch information
l-qing authored and tekton-robot committed Jan 16, 2024
1 parent 47848a0 commit 3d76902
Show file tree
Hide file tree
Showing 5 changed files with 225 additions and 13 deletions.
2 changes: 1 addition & 1 deletion docs/matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ data:
...
```

For more information, see [installation customizations](install.md#customizing-basic-execution-parameters).
For more information, see [installation customizations](./additional-configs.md#customizing-basic-execution-parameters).

## Parameters

Expand Down
10 changes: 5 additions & 5 deletions docs/podtemplates.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ configuration that Tekton can use as "boilerplate" for a Pod that runs your `Tas
You can specify a Pod template for `TaskRuns` and `PipelineRuns`. In the template, you can specify custom values for fields governing
the execution of individual `Tasks` or for all `Tasks` executed by a given `PipelineRun`.

You also have the option to define a global Pod template [in your Tekton config](./install.md#customizing-basic-execution-parameters) using the key `default-pod-template`.
However, this global template is going to be merged with any templates
you specify in your `TaskRuns` and `PipelineRuns`. Any field that is
present in both the global template and the `TaskRun`'s or
You also have the option to define a global Pod template [in your Tekton config](./additional-configs.md#customizing-basic-execution-parameters) using the key `default-pod-template`.
However, this global template is going to be merged with any templates you specify in your `TaskRuns` and `PipelineRuns`.<br>
Except for the `env` and `volumes` fields, other fields that exist in both the global template and the `TaskRun`'s or
`PipelineRun`'s template will be taken from the `TaskRun` or `PipelineRun`.
The `env` and `volumes` fields are merged by the `name` value in the array elements. If the item's `name` is the same, the item from `TaskRun` or `PipelineRun` will be used.

See the following for examples of specifying a Pod template:
- [Specifying a Pod template for a `TaskRun`](./taskruns.md#specifying-a-pod-template)
Expand All @@ -33,7 +33,7 @@ The supported fields are: `tolerations`, `nodeSelector`, and
`imagePullSecrets` (see the table below for more details).

Similarily to Pod templates, you have the option to define a global affinity
assistant Pod template [in your Tekton config](./install.md#customizing-basic-execution-parameters)
assistant Pod template [in your Tekton config](./additional-configs.md#customizing-basic-execution-parameters)
using the key `default-affinity-assistant-pod-template`. The merge strategy is
the same as the one described above.

Expand Down
2 changes: 1 addition & 1 deletion docs/workspaces.md
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ a `Task` requires but which are not provided by the `TaskRun` will be bound with

The configuration for the default `Workspace Binding` is added to the `config-defaults` `ConfigMap`, under
the `default-task-run-workspace-binding` key. For an example, see the [Customizing basic execution
parameters](./install.md#customizing-basic-execution-parameters) section of the install doc.
parameters](./additional-configs.md#customizing-basic-execution-parameters) section of the install doc.

**Note:** the default configuration is used for any _required_ `Workspace` declared by a `Task`. Optional
`Workspaces` are not populated with the default binding. This is because a `Task's` behaviour will typically
Expand Down
54 changes: 48 additions & 6 deletions pkg/apis/pipeline/pod/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,10 @@ func MergePodTemplateWithDefault(tpl, defaultTpl *PodTemplate) *PodTemplate {
return defaultTpl
default:
// Otherwise, merge fields
if tpl.Env == nil {
tpl.Env = defaultTpl.Env
}
if tpl.NodeSelector == nil {
tpl.NodeSelector = defaultTpl.NodeSelector
}
tpl.Env = mergeByName(defaultTpl.Env, tpl.Env)
if tpl.Tolerations == nil {
tpl.Tolerations = defaultTpl.Tolerations
}
Expand All @@ -187,9 +185,7 @@ func MergePodTemplateWithDefault(tpl, defaultTpl *PodTemplate) *PodTemplate {
if tpl.SecurityContext == nil {
tpl.SecurityContext = defaultTpl.SecurityContext
}
if tpl.Volumes == nil {
tpl.Volumes = defaultTpl.Volumes
}
tpl.Volumes = mergeByName(defaultTpl.Volumes, tpl.Volumes)
if tpl.RuntimeClassName == nil {
tpl.RuntimeClassName = defaultTpl.RuntimeClassName
}
Expand Down Expand Up @@ -254,3 +250,49 @@ func MergeAAPodTemplateWithDefault(tpl, defaultTpl *AAPodTemplate) *AAPodTemplat
return tpl
}
}

// 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 {
if len(overrides) == 0 {
return base
}

// create a map to store the exist names in the override slice
exists := make(map[string]struct{})
merged := make([]T, 0, len(base)+len(overrides))

// append the items in the override slice
for _, item := range overrides {
name := getName(item)
if name != "" { // name should not be empty, if empty, ignore
merged = append(merged, item)
exists[name] = struct{}{}
}
}

// append the items in the base slice if they have a different name
for _, item := range base {
name := getName(item)
if name != "" { // name should not be empty, if empty, ignore
if _, found := exists[name]; !found {
merged = append(merged, item)
}
}
}

return merged
}

// getName returns the name of the given item, or an empty string if the item
// is not a supported type.
func getName(item interface{}) string {
switch item := item.(type) {
case corev1.EnvVar:
return item.Name
case corev1.Volume:
return item.Name
default:
return ""
}
}
170 changes: 170 additions & 0 deletions pkg/apis/pipeline/pod/template_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
/*
Copyright 2024 The Tekton Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package pod

import (
"reflect"
"testing"

corev1 "k8s.io/api/core/v1"
)

func TestMergeByName(t *testing.T) {
type testCase struct {
name string
base []interface{}
overrides []interface{}
expected []interface{}
}

testCases := []testCase{
{
name: "empty overrides",
base: []interface{}{corev1.EnvVar{Name: "foo", Value: "bar"}},
overrides: []interface{}{},
expected: []interface{}{corev1.EnvVar{Name: "foo", Value: "bar"}},
},
{
name: "empty base",
base: []interface{}{},
overrides: []interface{}{corev1.EnvVar{Name: "foo", Value: "bar"}},
expected: []interface{}{corev1.EnvVar{Name: "foo", Value: "bar"}},
},
{
name: "same name",
base: []interface{}{corev1.EnvVar{Name: "foo", Value: "bar"}},
overrides: []interface{}{corev1.EnvVar{Name: "foo", Value: "baz"}},
expected: []interface{}{corev1.EnvVar{Name: "foo", Value: "baz"}},
},
{
name: "different name",
base: []interface{}{corev1.EnvVar{Name: "foo", Value: "bar"}},
overrides: []interface{}{corev1.EnvVar{Name: "bar", Value: "baz"}},
expected: []interface{}{corev1.EnvVar{Name: "bar", Value: "baz"}, corev1.EnvVar{Name: "foo", Value: "bar"}},
},
{
name: "different volume name",
base: []interface{}{corev1.Volume{Name: "foo", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}}},
overrides: []interface{}{corev1.Volume{Name: "bar", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}}},
expected: []interface{}{
corev1.Volume{Name: "bar", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}},
corev1.Volume{Name: "foo", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}},
},
},
{
name: "unsupported type",
base: []interface{}{corev1.EnvVar{Name: "foo", Value: "bar"}},
overrides: []interface{}{42},
expected: []interface{}{corev1.EnvVar{Name: "foo", Value: "bar"}},
},
{
name: "empty name",
base: []interface{}{corev1.EnvVar{Name: "", Value: "bar"}},
overrides: []interface{}{corev1.EnvVar{Name: "", Value: "bar"}},
expected: []interface{}{},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
result := mergeByName(tc.base, tc.overrides)
if !reflect.DeepEqual(result, tc.expected) {
t.Errorf("mergeByName(%v, %v) = %v, want %v", tc.base, tc.overrides, result, tc.expected)
}
})
}
}

func TestMergePodTemplateWithDefault(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: "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 := MergePodTemplateWithDefault(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)
}
})
}
}

0 comments on commit 3d76902

Please sign in to comment.