From 5b63e54f3d2913e2dd27593b72342b527000ec40 Mon Sep 17 00:00:00 2001 From: Piyush Garg Date: Sun, 22 Sep 2024 17:01:19 +0530 Subject: [PATCH] Add support for priorityClassName in affinityAssistantPodTemplate 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 --- docs/additional-configs.md | 2 +- docs/podtemplates.md | 28 ++++----- .../pod/affinity_assitant_template.go | 9 +++ pkg/apis/pipeline/pod/template.go | 12 ++-- pkg/apis/pipeline/pod/template_test.go | 57 ++++++++++++++++++- .../pipeline/pod/zz_generated.deepcopy.go | 5 ++ pkg/apis/pipeline/v1/openapi_generated.go | 7 +++ pkg/apis/pipeline/v1/swagger.json | 4 ++ .../pipeline/v1alpha1/openapi_generated.go | 7 +++ pkg/apis/pipeline/v1alpha1/swagger.json | 4 ++ .../pipeline/v1beta1/openapi_generated.go | 7 +++ pkg/apis/pipeline/v1beta1/swagger.json | 4 ++ .../pipelinerun/affinity_assistant.go | 14 +++-- .../pipelinerun/affinity_assistant_test.go | 6 ++ 14 files changed, 142 insertions(+), 24 deletions(-) diff --git a/docs/additional-configs.md b/docs/additional-configs.md index e71f09123e4..6b307bbd151 100644 --- a/docs/additional-configs.md +++ b/docs/additional-configs.md @@ -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 diff --git a/docs/podtemplates.md b/docs/podtemplates.md index 8f38abc9e12..53bb70ca6e3 100644 --- a/docs/podtemplates.md +++ b/docs/podtemplates.md @@ -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. @@ -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 diff --git a/pkg/apis/pipeline/pod/affinity_assitant_template.go b/pkg/apis/pipeline/pod/affinity_assitant_template.go index aba3f97b3a3..214ec6a8700 100644 --- a/pkg/apis/pipeline/pod/affinity_assitant_template.go +++ b/pkg/apis/pipeline/pod/affinity_assitant_template.go @@ -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. diff --git a/pkg/apis/pipeline/pod/template.go b/pkg/apis/pipeline/pod/template.go index 68d7e61af33..6bc37fecd7f 100644 --- a/pkg/apis/pipeline/pod/template.go +++ b/pkg/apis/pipeline/pod/template.go @@ -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, } } @@ -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 } diff --git a/pkg/apis/pipeline/pod/template_test.go b/pkg/apis/pipeline/pod/template_test.go index 64eaf704877..4e09e5068a3 100644 --- a/pkg/apis/pipeline/pod/template_test.go +++ b/pkg/apis/pipeline/pod/template_test.go @@ -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) } }) } diff --git a/pkg/apis/pipeline/pod/zz_generated.deepcopy.go b/pkg/apis/pipeline/pod/zz_generated.deepcopy.go index 90b2d243266..098b0fbeb13 100644 --- a/pkg/apis/pipeline/pod/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/pod/zz_generated.deepcopy.go @@ -52,6 +52,11 @@ func (in *AffinityAssistantTemplate) DeepCopyInto(out *AffinityAssistantTemplate *out = new(v1.PodSecurityContext) (*in).DeepCopyInto(*out) } + if in.PriorityClassName != nil { + in, out := &in.PriorityClassName, &out.PriorityClassName + *out = new(string) + **out = **in + } return } diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go index 7e801069b7b..d05c2178e31 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -168,6 +168,13 @@ func schema_pkg_apis_pipeline_pod_AffinityAssistantTemplate(ref common.Reference Ref: ref("k8s.io/api/core/v1.PodSecurityContext"), }, }, + "priorityClassName": { + SchemaProps: spec.SchemaProps{ + 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{"string"}, + Format: "", + }, + }, }, }, }, diff --git a/pkg/apis/pipeline/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json index 7c3ebefb8b6..73544e75d84 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -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" diff --git a/pkg/apis/pipeline/v1alpha1/openapi_generated.go b/pkg/apis/pipeline/v1alpha1/openapi_generated.go index 03d5a5967e1..cbcdc1ddf82 100644 --- a/pkg/apis/pipeline/v1alpha1/openapi_generated.go +++ b/pkg/apis/pipeline/v1alpha1/openapi_generated.go @@ -115,6 +115,13 @@ func schema_pkg_apis_pipeline_pod_AffinityAssistantTemplate(ref common.Reference Ref: ref("k8s.io/api/core/v1.PodSecurityContext"), }, }, + "priorityClassName": { + SchemaProps: spec.SchemaProps{ + 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{"string"}, + Format: "", + }, + }, }, }, }, diff --git a/pkg/apis/pipeline/v1alpha1/swagger.json b/pkg/apis/pipeline/v1alpha1/swagger.json index 5ab14febfc3..12ecda6393c 100644 --- a/pkg/apis/pipeline/v1alpha1/swagger.json +++ b/pkg/apis/pipeline/v1alpha1/swagger.json @@ -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" diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 126888e63d7..145e72b083a 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -195,6 +195,13 @@ func schema_pkg_apis_pipeline_pod_AffinityAssistantTemplate(ref common.Reference Ref: ref("k8s.io/api/core/v1.PodSecurityContext"), }, }, + "priorityClassName": { + SchemaProps: spec.SchemaProps{ + 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{"string"}, + Format: "", + }, + }, }, }, }, diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index d83792633ab..7e2d8d9d95c 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -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" diff --git a/pkg/reconciler/pipelinerun/affinity_assistant.go b/pkg/reconciler/pipelinerun/affinity_assistant.go index 27b9874effd..94aab0951ab 100644 --- a/pkg/reconciler/pipelinerun/affinity_assistant.go +++ b/pkg/reconciler/pipelinerun/affinity_assistant.go @@ -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, @@ -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, diff --git a/pkg/reconciler/pipelinerun/affinity_assistant_test.go b/pkg/reconciler/pipelinerun/affinity_assistant_test.go index 30c5c273578..35c22df73de 100644 --- a/pkg/reconciler/pipelinerun/affinity_assistant_test.go +++ b/pkg/reconciler/pipelinerun/affinity_assistant_test.go @@ -778,6 +778,7 @@ func TestDefaultPodTemplatesArePropagatedToAffinityAssistant(t *testing.T) { }, }, } + priorityClassName := "test-priority" defaultTpl := &pod.AffinityAssistantTemplate{ Tolerations: []corev1.Toleration{{ @@ -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) @@ -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) {