Possibility of setting priorityClassName in pod template configuration and via yamlMergeStrategy: merge with yaml section

[dadurex]

GitHub issue

#2813

TL;DR

This change adds support for configuring Kubernetes pod priorityClassName in Jenkins Kubernetes plugin pod templates, and ensures it is preserved when templates are merged (including YAML merge strategy).

What’s included

• Added priorityClassName to core pod template model (PodTemplate) with databinding support.
• Applied priorityClassName in pod creation (PodTemplateBuilder) so it is set on PodSpec.
• Extended template inheritance/merge logic (PodTemplateUtils) to correctly resolve and propagate priorityClassName.
• Added priorityClassName support in pipeline APIs:
  -- podTemplate step (PodTemplateStep + execution path)
  -- Declarative Kubernetes agent (KubernetesDeclarativeAgent)
• Added UI/config form fields and help text for priorityClassName in classic and pipeline configuration screens.
  Included snippet/argument map support so pipeline generation/roundtrip includes the new field.

Behavior

• If a child template specifies priorityClassName, it overrides the parent value.
• If not specified by child, parent value is retained.
• YAML merge behavior now correctly handles spec.priorityClassName when yamlMergeStrategy: merge is used.

Tests

• Top-level priorityClassName is applied to generated pod specs.
• YAML merge can override parent priorityClassName.
• Inheritance/combination behavior in PodTemplateUtils is correct for override and fallback cases.

Testing done

Tested cases:

• setting priorityClassName in yaml.spec (screenshot 1)- PASSED - prio set correctly
• setting priorityClassName in pod template field (screenshot 1) - PASSED - prio set correctly
• setting priorityClassName in GUI (screenshot 3 from GUI below)- PASSED - prio set correctly
• setting priorityClassName to nothing in GUI (screenshot 4) - PASSED - prio not set, default to 0

Casc template definitions:

              - name: test_1
                label: test_1
                inheritFrom: base
                yamlMergeStrategy: merge
                yaml: |-
                  spec:
                    priorityClassName: high-priority

              - name: test_2
                label: test_2
                inheritFrom: base
                priorityClassName: high-priority

screenshot 1)

screenshot 2)

screenshot 3)

screenshot 4)

Submitter checklist

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes
• Ensure you have provided tests that demonstrate the feature works or the issue is fixed

[dadurex]

Woho! CI passed!!!

[jglick]

Why would we need this? Anything but the most elementary configuration should be done via yaml, supporting anything offered by Kubernetes now or in the future.

[dadurex]

@jglick I described it in issue: #2813

Check below case:

If we want to have one base pod template which will contain all configuration regarding containers (as below label called base) we right now cannot set priorityClassName for child labels (below caled custom) using yaml with merge - i tested it and also i find prove in code that only some fields are really merged (nodeSelector is for example covered) - see here: https://github.com/jenkinsci/kubernetes-plugin/blob/4423.vb_59f230b_ce53/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java#L328-L342

Thats why i added mechanism to set:

• priorityClassName using field in same way as its possible right now for nodeSelector, volumes etc.
• added support for setting priorityClassName with yaml using yamlMergeStrategy: merge

Below example of config - with my change there will be option to set priorityClassName like in custom and even like in label custom_v2 - that's exactly the same was as its possible with nodeSelector right now

              - name: base
                label: base
                containers:
                - name: jnlp
                  image: some.repository/agent:1
                  ttyEnabled: true
                  alwaysPullImage: true
                  command: "/home/build/entrypoint.sh"
                  args: "/bin/bash -c \"jenkins-agent && cat\""
                imagePullSecrets:
                  - name: docker-registry

              - name: custom
                label: custom
                inheritFrom: base
                yamlMergeStrategy: merge
                yaml: |-
                  spec:
                    nodeSelector:
                      role: deployment
                    priorityClassName: high-priority

              - name: custom_v2
                label: custom_v2
                inheritFrom: base
                priorityClassName: ci-tool
                nodeSelector: role:deployment

[jglick]

i find prove in code that only some fields are really merged

YAML merge never worked very well. I do not recommend using it. Better (IMO) for a pipeline to use the podTemplate step and specify the complete Pod YAML it wants; if you want to avoid duplication between projects, use a library, Groovy """ templating, etc.

[dadurex]

The main reason for introducing this is to reduce configuration duplication. In my setup, labels are predefined in JCasC, and pipelines reference them using node('custom_label') {} - this way of configuring labels is widely used with JCasC, for example: https://github.com/jenkinsci/configuration-as-code-plugin/blob/master/demos/kubernetes/adv_config.yml#L27-L48 . Labels inherits from other base label which have all configuration regarding image, entrypoint, all envs and child labels only set reosurces-requests/limits and priority class. Without that change right now is required to duplicate whole yaml section in each child label as with yamlMergeStrategy: merge its not possible to set priority class with simple:

                inheritFrom: base
                yamlMergeStrategy: merge
                yaml: |-
                  spec:
                    priorityClassName: high-priority

So imo it is better to extend what is already possible to modify with yaml merge option - there is already option to set e.g. tolerations, init contiers - so why we should not cover also priorityClass? Using PriorityClass is often necessary when you need to refine how Pods are scheduled when you deal with huge queue of builds in jenkins.

This change also allows configuring priorityClass in the podTemplate (as below). The implementation closely follows the mechanism already used for nodeSelector - https://plugins.jenkins.io/kubernetes/#plugin-content-pod-template.

              - name: custom
                label: custom
                inheritFrom: base
                priorityClassName: high-priority
                nodeSelector: role:deployment

[dadurex]

YAML merge never worked very well. I do not recommend using it.

This is why i added that change to cover more cases. I do not understand why we don't want to improve something that is already here. If its not recommended then maybe lets deprecate merge option of yaml section.

[jglick]

labels are predefined in JCasC, and pipelines reference them

It is one usage mode of course, but podTemplate is just more flexible than global (label-selected) templates.

deprecate merge option of yaml section

Might be a good idea, though I am not sure how to communicate it clearly.

Fine if some maintainer (if there is one…) wants to merge this PR, but I have watched many of these PRs get filed and the number of configurable fields in pod templates grows and grows and it is never enough for what someone needs.

[dadurex]

I have watched many of these PRs get filed and the number of configurable fields in pod templates grows and grows and it is never enough for what someone needs.

I can shrink that PR to only fix the priorityClass setting in the yaml section (as below in example) - if its OK then i will update that PR tomorrow. To be honest, configuration through yaml block is much cleaner for me and i always prefer it than custom pod template fields.

                inheritFrom: base
                yamlMergeStrategy: merge
                yaml: |-
                  spec:
                    priorityClassName: high-priority

Fine if some maintainer (if there is one…)

Can we ask someone else then for their opinion - if this PR is fine at this state or i should cover only setting priorityClass in yaml section? (I'm new in that community and i don't know who is activity maintaining this plugin)