Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for priorityClassName in affinityAssistantPodTemplate #8286

Merged
merged 1 commit into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/additional-configs.md
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ The example below customizes the following:
- the default service account from `default` to `tekton`.
- the default timeout from 60 minutes to 20 minutes.
- the default `app.kubernetes.io/managed-by` label is applied to all Pods created to execute `TaskRuns`.
- the default Pod template to include a node selector to select the node where the Pod will be scheduled by default. A list of supported fields is available [here](https://github.com/tektoncd/pipeline/blob/main/docs/podtemplates.md#supported-fields).
- the default Pod template to include a node selector to select the node where the Pod will be scheduled by default. A list of supported fields is available [here](./podtemplates.md#supported-fields).
For more information, see [`PodTemplate` in `TaskRuns`](./taskruns.md#specifying-a-pod-template) or [`PodTemplate` in `PipelineRuns`](./pipelineruns.md#specifying-a-pod-template).
- the default `Workspace` configuration can be set for any `Workspaces` that a Task declares but that a TaskRun does not explicitly provide.
- the default maximum combinations of `Parameters` in a `Matrix` that can be used to fan out a `PipelineTask`. For
Expand Down
28 changes: 14 additions & 14 deletions docs/podtemplates.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,6 @@ See the following for examples of specifying a Pod template:
- [Specifying a Pod template for a `TaskRun`](./taskruns.md#specifying-a-pod-template)
- [Specifying a Pod template for a `PipelineRun`](./pipelineruns.md#specifying-a-pod-template)

## Affinity Assistant Pod templates

The Pod templates specified in the `TaskRuns` and `PipelineRuns `also apply to
the [affinity assistant Pods](#./workspaces.md#specifying-workspace-order-in-a-pipeline-and-affinity-assistants)
that are created when using Workspaces, but only on select fields.

The supported fields are: `tolerations`, `nodeSelector`, `securityContext` 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](./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.

## Supported fields

Pod templates support fields listed in the table below.
Expand Down Expand Up @@ -156,6 +142,20 @@ roleRef:
apiGroup: rbac.authorization.k8s.io
```

# Affinity Assistant Pod templates

The Pod templates specified in the `TaskRuns` and `PipelineRuns `also apply to
the [affinity assistant Pods](#./workspaces.md#specifying-workspace-order-in-a-pipeline-and-affinity-assistants)
that are created when using Workspaces, but only on selected fields.

The supported fields for affinity assistant pods are: `tolerations`, `nodeSelector`, `securityContext`,
`priorityClassName` and `imagePullSecrets` (see the table above for more details about the fields).

Similarly to global Pod Template, you have the option to define a global affinity
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 for the supported fields.

---

Except as otherwise noted, the content of this page is licensed under the
Expand Down
9 changes: 9 additions & 0 deletions pkg/apis/pipeline/pod/affinity_assitant_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ type AffinityAssistantTemplate struct {
// SecurityContext sets the security context for the pod
// +optional
SecurityContext *corev1.PodSecurityContext `json:"securityContext,omitempty"`

// If specified, indicates the pod's priority. "system-node-critical" and
// "system-cluster-critical" are two special keywords which indicate the
// highest priorities with the former being the highest priority. Any other
// name must be defined by creating a PriorityClass object with that name.
// If not specified, the pod priority will be default or zero if there is no
// default.
// +optional
PriorityClassName *string `json:"priorityClassName,omitempty"`
}

// Equals checks if this Template is identical to the given Template.
Expand Down
12 changes: 8 additions & 4 deletions pkg/apis/pipeline/pod/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,11 @@ func (tpl *Template) ToAffinityAssistantTemplate() *AffinityAssistantTemplate {
}

return &AffinityAssistantTemplate{
NodeSelector: tpl.NodeSelector,
Tolerations: tpl.Tolerations,
ImagePullSecrets: tpl.ImagePullSecrets,
SecurityContext: tpl.SecurityContext,
NodeSelector: tpl.NodeSelector,
Tolerations: tpl.Tolerations,
ImagePullSecrets: tpl.ImagePullSecrets,
SecurityContext: tpl.SecurityContext,
PriorityClassName: tpl.PriorityClassName,
}
}

Expand Down Expand Up @@ -251,6 +252,9 @@ func MergeAAPodTemplateWithDefault(tpl, defaultTpl *AAPodTemplate) *AAPodTemplat
if tpl.SecurityContext == nil {
tpl.SecurityContext = defaultTpl.SecurityContext
}
if tpl.PriorityClassName == nil {
tpl.PriorityClassName = defaultTpl.PriorityClassName
}

return tpl
}
Expand Down
57 changes: 56 additions & 1 deletion pkg/apis/pipeline/pod/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,62 @@ func TestMergePodTemplateWithDefault(t *testing.T) {
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)
t.Errorf("mergePodTemplateWithDefault%v, %v) = %v, want %v", tc.tpl, tc.defaultTpl, result, tc.expected)
}
})
}
}

func TestMergeAAPodTemplateWithDefault(t *testing.T) {
priority1 := "low-priority"
priority2 := "high-priority"
type testCase struct {
name string
tpl *AAPodTemplate
defaultTpl *AAPodTemplate
expected *AAPodTemplate
}

testCases := []testCase{
{
name: "defaultTpl is nil",
tpl: &AAPodTemplate{
NodeSelector: map[string]string{"foo": "bar"},
},
defaultTpl: nil,
expected: &AAPodTemplate{
NodeSelector: map[string]string{"foo": "bar"},
},
},
{
name: "tpl is nil",
tpl: nil,
defaultTpl: &AAPodTemplate{
NodeSelector: map[string]string{"foo": "bar"},
},
expected: &AAPodTemplate{
NodeSelector: map[string]string{"foo": "bar"},
},
},
{
name: "override default priorityClassName",
tpl: &AAPodTemplate{
PriorityClassName: &priority2,
},
defaultTpl: &AAPodTemplate{
PriorityClassName: &priority1,
},
expected: &AAPodTemplate{
PriorityClassName: &priority2,
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
result := MergeAAPodTemplateWithDefault(tc.tpl, tc.defaultTpl)
if !reflect.DeepEqual(result, tc.expected) {
t.Errorf("mergeAAPodTemplateWithDefault(%v, %v) = %v, want %v", tc.tpl, tc.defaultTpl, result, tc.expected)
}
})
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/pipeline/pod/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/apis/pipeline/v1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
"default": ""
}
},
"priorityClassName": {
"description": "If specified, indicates the pod's priority. \"system-node-critical\" and \"system-cluster-critical\" are two special keywords which indicate the highest priorities with the former being the highest priority. Any other name must be defined by creating a PriorityClass object with that name. If not specified, the pod priority will be default or zero if there is no default.",
"type": "string"
},
"securityContext": {
"description": "SecurityContext sets the security context for the pod",
"$ref": "#/definitions/v1.PodSecurityContext"
Expand Down
7 changes: 7 additions & 0 deletions pkg/apis/pipeline/v1alpha1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1alpha1/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
"default": ""
}
},
"priorityClassName": {
"description": "If specified, indicates the pod's priority. \"system-node-critical\" and \"system-cluster-critical\" are two special keywords which indicate the highest priorities with the former being the highest priority. Any other name must be defined by creating a PriorityClass object with that name. If not specified, the pod priority will be default or zero if there is no default.",
"type": "string"
},
"securityContext": {
"description": "SecurityContext sets the security context for the pod",
"$ref": "#/definitions/v1.PodSecurityContext"
Expand Down
7 changes: 7 additions & 0 deletions pkg/apis/pipeline/v1beta1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1beta1/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
"default": ""
}
},
"priorityClassName": {
"description": "If specified, indicates the pod's priority. \"system-node-critical\" and \"system-cluster-critical\" are two special keywords which indicate the highest priorities with the former being the highest priority. Any other name must be defined by creating a PriorityClass object with that name. If not specified, the pod priority will be default or zero if there is no default.",
"type": "string"
},
"securityContext": {
"description": "SecurityContext sets the security context for the pod",
"$ref": "#/definitions/v1.PodSecurityContext"
Expand Down
14 changes: 10 additions & 4 deletions pkg/reconciler/pipelinerun/affinity_assistant.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,11 @@ func affinityAssistantStatefulSet(aaBehavior aa.AffinityAssistantBehavior, name
}
}

var priorityClassName string
if tpl.PriorityClassName != nil {
priorityClassName = *tpl.PriorityClassName
}

containers := []corev1.Container{{
Name: "affinity-assistant",
Image: containerConfig.Image,
Expand Down Expand Up @@ -375,10 +380,11 @@ func affinityAssistantStatefulSet(aaBehavior aa.AffinityAssistantBehavior, name
Spec: corev1.PodSpec{
Containers: containers,

Tolerations: tpl.Tolerations,
NodeSelector: tpl.NodeSelector,
ImagePullSecrets: tpl.ImagePullSecrets,
SecurityContext: tpl.SecurityContext,
Tolerations: tpl.Tolerations,
NodeSelector: tpl.NodeSelector,
ImagePullSecrets: tpl.ImagePullSecrets,
SecurityContext: tpl.SecurityContext,
PriorityClassName: priorityClassName,

Affinity: getAssistantAffinityMergedWithPodTemplateAffinity(pr, aaBehavior),
Volumes: volumes,
Expand Down
6 changes: 6 additions & 0 deletions pkg/reconciler/pipelinerun/affinity_assistant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,7 @@ func TestDefaultPodTemplatesArePropagatedToAffinityAssistant(t *testing.T) {
},
},
}
priorityClassName := "test-priority"

defaultTpl := &pod.AffinityAssistantTemplate{
Tolerations: []corev1.Toleration{{
Expand All @@ -796,6 +797,7 @@ func TestDefaultPodTemplatesArePropagatedToAffinityAssistant(t *testing.T) {
RunAsNonRoot: ptr.Bool(true),
SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault},
},
PriorityClassName: &priorityClassName,
}

stsWithOverridenTemplateFields := affinityAssistantStatefulSet(aa.AffinityAssistantPerWorkspace, "test-assistant", prWithCustomPodTemplate, []corev1.PersistentVolumeClaim{}, []string{}, containerConfigWithoutSecurityContext, defaultTpl)
Expand All @@ -815,6 +817,10 @@ func TestDefaultPodTemplatesArePropagatedToAffinityAssistant(t *testing.T) {
if stsWithOverridenTemplateFields.Spec.Template.Spec.SecurityContext == nil {
t.Errorf("expected SecurityContext in the StatefulSet")
}

if stsWithOverridenTemplateFields.Spec.Template.Spec.PriorityClassName == "" {
t.Errorf("expected PriorityClassName in the StatefulSet")
}
}

func TestMergedPodTemplatesArePropagatedToAffinityAssistant(t *testing.T) {
Expand Down