[JENKINS-57828] Cleaning up the Snippet Generator output for PodTemplateStep #703

kerogers-cloudbees · 2020-02-07T19:43:00Z

Followed the instructions https://jenkins.io/doc/developer/plugin-development/pipeline-integration/ in the Handling default values section, except had to use Util.fixEmpty(String s) instead of Util.fixNull()
Added defaults for podRetention and workspaceVolume in PodTemplateStep.
Changed DynamicPVCWorkspaceVolume to be compliant with snippet generation, changed DataBoundConstructor to have no parameters, added DataBoundSetters for all properties.
Added tests classifier to workflow-cps test dependency to get the SnippitizerTest class.
Added unit test for PodTemplateStep.

…for PodTemplateStep. Followed the instructions https://jenkins.io/doc/developer/plugin-development/pipeline-integration/ in the Handling default values section, except had to use `Util.fixEmpty(String s)` instead of `Util.fixNull()` Changed the default value of `instanceCap` to 0 from MAX_INT. Otherwise requires a lot of logic to make it not be generated if set to the default of 0 from the config.jelly. Added defaults for podRetention and workspaceVolume in PodTemplateStep. Changed DynamicPVCWorkspaceVolume to be compliant with snippet generation, changed DataBoundConstructor to have no parameters, added DataBoundSetters for all properties. Added tests classifier to workflow-cps test dependency to get the SnippitizerTest class. Added unit test for PodTemplateStep.

… file.

jglick · 2020-02-07T20:04:49Z

Have some SpotBugs errors.

pom.xml

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java

src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepTest.java

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java

@issue

…/PodTemplateStepTest.java Change @issue argument. Co-Authored-By: Jesse Glick <jglick@cloudbees.com>

jglick

For List-typed fields, the natural default in most cases is an empty list.

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java

jglick · 2020-02-07T21:05:03Z

As of master, the unconfigured step gives

podTemplate(inheritFrom: '', instanceCap: 0, namespace: '', nodeSelector: '', podRetention: always(), serviceAccount: '', supplementalGroups: '', workspaceVolume: dynamicPVC(accessModes: 'ReadWriteOnce', requestsSize: '', storageClassName: ''), yaml: '') {
    // some block
}

which is too much, but neither containers nor volumes are mentioned here, so there is no need to touch these properties—only those which actually appear in the above list.

Revert changes to list properties, they are all initialized and should never be null. Change defaults to be the same as the PodTemplate defaults. Change config.jelly to use these defaults also.

….com:kerogers-cloudbees/kubernetes-plugin into podTemplate-snippet-jenkins-57828-cplt2-6206

jglick · 2020-02-10T20:03:53Z

#709 might solve these test failures.

Vlatombe · 2020-02-13T10:17:51Z

Rebuilding

stale

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java

…tanceCap of Integer.MAX_VALUE. Adding logic to the setter that sets instanceCap to Integer.MAX_VALUE if the Integer passed to it is null or less than equal to 0.

…Test.runInPodNestedExplicitInherit to pass. Having inheritFrom be null instead of an empty string was breaking something in the podTemplate construction.

...ources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runInPodNestedExplicitInherit.groovy

…equire escaping in either xml or java and seems very unlikely to ever be the name of an actual template.

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java

...main/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep/config.jelly

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java

...ources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runInPodNestedExplicitInherit.groovy

src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepTest.java

Vlatombe · 2020-03-06T10:08:35Z

@kerogers-cloudbees can you process latest reviews from Jessie?

…n. The behavior I intended for this string to trigger is actually the opposite of the default behavior, it is to create a pod template step that has inheritance set to an empty string so that `PodTemplateStepExecution` will not add the templates to the inheritance.

...ources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runInPodNestedExplicitInherit.groovy

…e default inheritance. Use the string "<default>" in the snippet generator to mean null in the generated script.

stale

src/main/webapp/help/inheritFrom.html

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java

...main/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep/config.jelly

….com:kerogers-cloudbees/kubernetes-plugin into podTemplate-snippet-jenkins-57828-cplt2-6206

…/PodTemplateStep.java Co-Authored-By: Jesse Glick <jglick@cloudbees.com>

…/PodTemplateStep.java recommended change from review Co-Authored-By: Jesse Glick <jglick@cloudbees.com>

recommended change from review Co-Authored-By: Vincent Latombe <vincent@latombe.net>

…form. Removed attributes `oneEach`, `hasHeader`, `addCaption`, and `deleteCaption` as they don't seem to work with a `dropdownDescriptorSelector`

Vlatombe

lgtm

jglick · 2020-03-13T15:30:03Z

Looks right now. Thanks for your perseverance!

kerogers-cloudbees added 3 commits February 6, 2020 15:52

[CPLT2-6206][JENKINS-57828] Accidentally left commented out code in a…

79e7062

… file.

Missed a brace that was in the comment.

62e6056

kerogers-cloudbees requested review from jglick and Vlatombe February 7, 2020 19:43

jglick changed the title ~~[CPLT2-6206][JENKINS-57828] Cleaning up the Snippet Generator output …~~ [CPLT2-6206][JENKINS-57828] Cleaning up the Snippet Generator output for PodTemplateStep Feb 7, 2020

jglick requested changes Feb 7, 2020

View reviewed changes

Update src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline…

f83ee33

…/PodTemplateStepTest.java Change @issue argument. Co-Authored-By: Jesse Glick <jglick@cloudbees.com>

jglick requested changes Feb 7, 2020

View reviewed changes

jglick previously requested changes Feb 7, 2020

View reviewed changes

kerogers-cloudbees added 4 commits February 7, 2020 16:41

[CPLT2-6206][JENKINS-57828] Updates from code review

13820a7

Revert changes to list properties, they are all initialized and should never be null. Change defaults to be the same as the PodTemplate defaults. Change config.jelly to use these defaults also.

Merge branch 'podTemplate-snippet-jenkins-57828-cplt2-6206' of github…

67884c3

….com:kerogers-cloudbees/kubernetes-plugin into podTemplate-snippet-jenkins-57828-cplt2-6206

Fix unit test to use corrected defaults.

511a7c1

add dependency with classifier for tests.

984d409

jglick self-requested a review February 10, 2020 20:03

Merge branch 'master' into podTemplate-snippet-jenkins-57828-cplt2-6206

4f57e22

Vlatombe closed this Feb 13, 2020

Vlatombe reopened this Feb 13, 2020

Vlatombe added 2 commits February 13, 2020 14:09

Merge branch 'master' into podTemplate-snippet-jenkins-57828-cplt2-6206

14886ef

Merge branch 'master' into podTemplate-snippet-jenkins-57828-cplt2-6206

924d0a0

Merge branch 'master' into podTemplate-snippet-jenkins-57828-cplt2-6206

0e8ea09

Vlatombe reviewed Feb 17, 2020

View reviewed changes

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java Outdated Show resolved Hide resolved

kerogers-cloudbees and others added 3 commits February 17, 2020 13:59

[CPLT2-6206] Changing PodTemplateStep to have a default value for ins…

934a27c

…tanceCap of Integer.MAX_VALUE. Adding logic to the setter that sets instanceCap to Integer.MAX_VALUE if the Integer passed to it is null or less than equal to 0.

Merge branch 'master' into podTemplate-snippet-jenkins-57828-cplt2-6206

a17439a

Fix merge

4b01002

[CPLT2-6206] special case for inheritFrom to allow KubernetesPipeline…

4504773

…Test.runInPodNestedExplicitInherit to pass. Having inheritFrom be null instead of an empty string was breaking something in the podTemplate construction.

jglick requested changes Feb 28, 2020

View reviewed changes

...ources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runInPodNestedExplicitInherit.groovy Outdated Show resolved Hide resolved

Vlatombe added the enhancement Improvements label Mar 3, 2020

[CPLT2-6206] Change the special string to ",default," as it doesn't r…

1662c1e

…equire escaping in either xml or java and seems very unlikely to ever be the name of an actual template.

jglick reviewed Mar 3, 2020

View reviewed changes

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java Outdated Show resolved Hide resolved

...main/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep/config.jelly Outdated Show resolved Hide resolved

jglick requested changes Mar 3, 2020

View reviewed changes

jglick reviewed Mar 3, 2020

View reviewed changes

src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepTest.java Show resolved Hide resolved

[CPLT2-6206] Explain behavior of inheritFrom in the help text.

febb56f

jglick previously requested changes Mar 6, 2020

View reviewed changes

...ources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runInPodNestedExplicitInherit.groovy Outdated Show resolved Hide resolved

[CPLT2-6206] restored empty string to be no inheritance and null to b…

7309343

…e default inheritance. Use the string "<default>" in the snippet generator to mean null in the generated script.

jglick changed the title ~~[CPLT2-6206][JENKINS-57828] Cleaning up the Snippet Generator output for PodTemplateStep~~ [JENKINS-57828] Cleaning up the Snippet Generator output for PodTemplateStep Mar 9, 2020

jglick self-requested a review March 9, 2020 14:53

Merge branch 'master' into podTemplate-snippet-jenkins-57828-cplt2-6206

b36413b

Vlatombe reviewed Mar 10, 2020

View reviewed changes

src/main/webapp/help/inheritFrom.html Outdated Show resolved Hide resolved

jglick reviewed Mar 10, 2020

View reviewed changes

kerogers-cloudbees and others added 8 commits March 11, 2020 05:16

Merge branch 'master' into podTemplate-snippet-jenkins-57828-cplt2-6206

60442ae

Merge branch 'podTemplate-snippet-jenkins-57828-cplt2-6206' of github…

cceb7ee

….com:kerogers-cloudbees/kubernetes-plugin into podTemplate-snippet-jenkins-57828-cplt2-6206

Update src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline…

4254f2c

…/PodTemplateStep.java Co-Authored-By: Jesse Glick <jglick@cloudbees.com>

Update src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline…

e346b9f

…/PodTemplateStep.java Co-Authored-By: Jesse Glick <jglick@cloudbees.com>

Update src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline…

4774681

…/PodTemplateStep.java recommended change from review Co-Authored-By: Jesse Glick <jglick@cloudbees.com>

Update src/main/webapp/help/inheritFrom.html

03debcc

recommended change from review Co-Authored-By: Vincent Latombe <vincent@latombe.net>

[CPLT2-6206] Clean up imports, fix a missing dot.

529e89b

[CPLT2-6206] Remove the checkbox that was hiding the WorkspaceVolume …

877a145

…form. Removed attributes `oneEach`, `hasHeader`, `addCaption`, and `deleteCaption` as they don't seem to work with a `dropdownDescriptorSelector`

Vlatombe approved these changes Mar 13, 2020

View reviewed changes

Vlatombe merged commit 55233bc into jenkinsci:master Mar 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-57828] Cleaning up the Snippet Generator output for PodTemplateStep #703

[JENKINS-57828] Cleaning up the Snippet Generator output for PodTemplateStep #703

kerogers-cloudbees commented Feb 7, 2020 •

edited by jglick

Loading

jglick commented Feb 7, 2020

jglick left a comment

jglick commented Feb 7, 2020

jglick commented Feb 10, 2020

Vlatombe commented Feb 13, 2020

Vlatombe commented Mar 6, 2020

Vlatombe left a comment

jglick commented Mar 13, 2020

[JENKINS-57828] Cleaning up the Snippet Generator output for PodTemplateStep #703

[JENKINS-57828] Cleaning up the Snippet Generator output for PodTemplateStep #703

Conversation

kerogers-cloudbees commented Feb 7, 2020 • edited by jglick Loading

jglick commented Feb 7, 2020

jglick left a comment

Choose a reason for hiding this comment

jglick commented Feb 7, 2020

jglick commented Feb 10, 2020

Vlatombe commented Feb 13, 2020

Vlatombe commented Mar 6, 2020

Vlatombe left a comment

Choose a reason for hiding this comment

jglick commented Mar 13, 2020

kerogers-cloudbees commented Feb 7, 2020 •

edited by jglick

Loading