Skip to content

Commit

Permalink
Add support for priorityClassName in affinityAssistantPodTemplate
Browse files Browse the repository at this point in the history
This will add the support for specifying priorityClassName
for affinity assistant pods. It will be possible to specify
default value of priorityClassName in defaultAffinityAssistant
pod template. Also the priorityClassName value specified in
pipelinerun/taskrun will overwrite the default value of
priorityClassName and same value will be used for both
affinity assistant pods and tasrun pods.

This will help to specify same priorityClassName for
affinity assistant pods and task run pods, so all get
scheduled with same priority

Part of #7779

/kind feature
  • Loading branch information
piyush-garg authored and tekton-robot committed Oct 8, 2024
1 parent 819bab1 commit 5b63e54
Show file tree
Hide file tree
Showing 14 changed files with 142 additions and 24 deletions.
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

0 comments on commit 5b63e54

Please sign in to comment.