-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[JENKINS-58405] Backward compatibility for yaml field merge #538
Conversation
Added a yamlMergeStrategy field to PodTemplate/PodTemplateStep in order to control how yaml field is handled across inheritance of pod templates. Defaults to "override" in order to be backward compatible.
src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java
Outdated
Show resolved
Hide resolved
src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java
Outdated
Show resolved
Hide resolved
src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java
Outdated
Show resolved
Hide resolved
Defines how the raw yaml field gets merged with yaml definitions from inherited pod templates. | ||
|
||
<ul> | ||
<li>Override: The yaml definitions from inherited pod templates are completely overridden by the current yaml field.</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put this documentation in help.html
for each option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still recommend this ^^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum... does it work with dropdownDescriptorSelector
? The only place where I've seen this pattern being used was with descriptorRadioList
. I could probably pull the description for each implementation in its own config page though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure. Try config.jelly
plus f:description
. It is possible f:dropdownDescriptorSelector
does not yet honor help.html
, I do not recall.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is what I ended up doing in e01f122
src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/mergeYaml.groovy
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this going to be forward-ported to master
also?
src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java
Show resolved
Hide resolved
src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java
Show resolved
Hide resolved
Defines how the raw yaml field gets merged with yaml definitions from inherited pod templates. | ||
|
||
<ul> | ||
<li>Override: The yaml definitions from inherited pod templates are completely overridden by the current yaml field.</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still recommend this ^^^
[JENKINS-58405] Backward compatibility for yaml field merge
Amends #449 right? |
Added a yamlMergeStrategy field to PodTemplate/PodTemplateStep in order
to control how yaml field is handled across inheritance of pod
templates. Defaults to "override" in order to be backward compatible.