From 038a4d4d475bcb4dd754aeb6e350b4e5bd48be27 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Tue, 9 Jul 2019 17:01:59 +0200 Subject: [PATCH 1/5] [JENKINS-58405] Backward compatibility for yaml field merge 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. --- .../plugins/kubernetes/PodTemplate.java | 34 +++++++++++++++- .../kubernetes/PodTemplateBuilder.java | 3 +- .../plugins/kubernetes/PodTemplateUtils.java | 1 + .../kubernetes/pipeline/PodTemplateStep.java | 11 +++++ .../pipeline/PodTemplateStepExecution.java | 1 + .../plugins/kubernetes/pod/yaml/Merge.java | 38 ++++++++++++++++++ .../kubernetes/pod/yaml/Overrides.java | 40 +++++++++++++++++++ .../pod/yaml/YamlMergeStrategy.java | 16 ++++++++ .../kubernetes/PodTemplate/config.jelly | 2 + .../PodTemplate/help-yamlMergeStrategy.html | 6 +++ .../kubernetes/PodTemplateBuilderTest.java | 11 +++++ .../pipeline/KubernetesPipelineTest.java | 12 ++++++ .../kubernetes/pipeline/mergeYaml.groovy | 22 ++++++++++ .../kubernetes/pipeline/overrideYaml.groovy | 21 ++++++++++ 14 files changed, 215 insertions(+), 3 deletions(-) create mode 100644 src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java create mode 100644 src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java create mode 100644 src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java create mode 100644 src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-yamlMergeStrategy.html create mode 100644 src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/mergeYaml.groovy create mode 100644 src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/overrideYaml.groovy diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java index 994d7450c5..d7e60d276e 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java @@ -16,6 +16,7 @@ import org.apache.commons.lang.StringUtils; import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.PodRetention; +import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.YamlMergeStrategy; import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume; import org.csanchez.jenkins.plugins.kubernetes.volumes.workspace.WorkspaceVolume; import org.kohsuke.accmod.Restricted; @@ -129,6 +130,21 @@ public class PodTemplate extends AbstractDescribableImpl implements private List yamls = new ArrayList<>(); + public YamlMergeStrategy getYamlMergeStrategy() { + return yamlMergeStrategy; + } + + @DataBoundSetter + public void setYamlMergeStrategy(YamlMergeStrategy yamlMergeStrategy) { + this.yamlMergeStrategy = yamlMergeStrategy; + } + + private YamlMergeStrategy yamlMergeStrategy = YamlMergeStrategy.defaultStrategy(); + + public Pod getYamlsPod() { + return yamlMergeStrategy.merge(getYamls()); + } + private Boolean showRawYaml; @CheckForNull @@ -718,6 +734,10 @@ protected Object readResolve() { showRawYaml = Boolean.TRUE; } + if (yamlMergeStrategy == null) { + yamlMergeStrategy = YamlMergeStrategy.defaultStrategy(); + } + return this; } @@ -801,7 +821,8 @@ public List getEnvVarsDescriptors() { return DescriptorVisibilityFilter.apply(null, Jenkins.getInstance().getDescriptorList(TemplateEnvVar.class)); } - @SuppressWarnings("rawtypes") + @SuppressWarnings("unused") // Used by jelly + @Restricted(DoNotUse.class) // Used by jelly public Descriptor getDefaultPodRetention() { Jenkins jenkins = Jenkins.getInstanceOrNull(); if (jenkins == null) { @@ -810,6 +831,17 @@ public Descriptor getDefaultPodRetention() { return jenkins.getDescriptor(PodRetention.getPodTemplateDefault().getClass()); } + @SuppressWarnings("unused") // Used by jelly + @Restricted(DoNotUse.class) // Used by jelly + public Descriptor getDefaultYamlMergeStrategy() { + Jenkins jenkins = Jenkins.getInstanceOrNull(); + if (jenkins == null) { + return null; + } + return jenkins.getDescriptor(YamlMergeStrategy.defaultStrategy().getClass()); + } + + } @Override diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java index 23f8558441..b285ea8cef 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java @@ -198,8 +198,7 @@ public Pod build() { builder.withContainers(containers.values().toArray(new Container[containers.size()])); // merge with the yaml fragments - Pod yamlPods = combine(template.getYamls().stream().map(PodTemplateUtils::parseFromYaml).collect(Collectors.toList())); - Pod pod = combine(yamlPods, builder.endSpec().build()); + Pod pod = combine(template.getYamlsPod(), builder.endSpec().build()); // Apply defaults diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java index 6a992d67a2..ec4699d1ee 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java @@ -351,6 +351,7 @@ public static PodTemplate combine(PodTemplate parent, PodTemplate template) { podTemplate.setAnnotations(new ArrayList<>(podAnnotations)); podTemplate.setNodeProperties(toolLocationNodeProperties); podTemplate.setNodeUsageMode(nodeUsageMode); + podTemplate.setYamlMergeStrategy(template.getYamlMergeStrategy()); podTemplate.setInheritFrom(!Strings.isNullOrEmpty(template.getInheritFrom()) ? template.getInheritFrom() : parent.getInheritFrom()); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java index 9f85b2f1c2..a21192b8f1 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java @@ -13,6 +13,7 @@ import org.csanchez.jenkins.plugins.kubernetes.PodTemplate; import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.PodRetention; +import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.YamlMergeStrategy; import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume; import org.csanchez.jenkins.plugins.kubernetes.volumes.workspace.WorkspaceVolume; import org.jenkinsci.plugins.workflow.steps.Step; @@ -58,6 +59,7 @@ public class PodTemplateStep extends Step implements Serializable { private String workingDir = ContainerTemplate.DEFAULT_WORKING_DIR; private String yaml; + private YamlMergeStrategy yamlMergeStrategy = YamlMergeStrategy.defaultStrategy(); private PodRetention podRetention; @DataBoundConstructor @@ -122,6 +124,15 @@ public void setEnvVars(List envVars) { } } + public YamlMergeStrategy getYamlMergeStrategy() { + return yamlMergeStrategy; + } + + @DataBoundSetter + public void setYamlMergeStrategy(YamlMergeStrategy yamlMergeStrategy) { + this.yamlMergeStrategy = yamlMergeStrategy; + } + public List getVolumes() { return volumes; } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepExecution.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepExecution.java index d6c7dc252d..4a348ea4f2 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepExecution.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepExecution.java @@ -101,6 +101,7 @@ public boolean start() throws Exception { newTemplate.setNodeUsageMode(step.getNodeUsageMode()); newTemplate.setServiceAccount(step.getServiceAccount()); newTemplate.setAnnotations(step.getAnnotations()); + newTemplate.setYamlMergeStrategy(step.getYamlMergeStrategy()); if(run!=null) { newTemplate.getAnnotations().add(new PodAnnotation("buildUrl", ((KubernetesCloud)cloud).getJenkinsUrlOrDie()+run.getUrl())); } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java new file mode 100644 index 0000000000..8a6dcebcef --- /dev/null +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java @@ -0,0 +1,38 @@ +package org.csanchez.jenkins.plugins.kubernetes.pod.yaml; + +import hudson.Extension; +import hudson.model.Descriptor; +import io.fabric8.kubernetes.api.model.Pod; +import org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils; +import org.jenkinsci.Symbol; +import org.kohsuke.stapler.DataBoundConstructor; + +import javax.annotation.Nonnull; +import java.io.Serializable; +import java.util.List; +import java.util.stream.Collectors; + +import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.combine; + +public class Merge extends YamlMergeStrategy implements Serializable { + private static final long serialVersionUID = 6562610892063268131L; + + @DataBoundConstructor + public Merge() { + } + + @Override + public Pod merge(List yamls) { + return combine(yamls.stream().map(PodTemplateUtils::parseFromYaml).collect(Collectors.toList())); + } + + @Extension + @Symbol("merge") + public static class DescriptorImpl extends Descriptor { + @Nonnull + @Override + public String getDisplayName() { + return "Merge"; + } + } +} diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java new file mode 100644 index 0000000000..a64ec03ddb --- /dev/null +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java @@ -0,0 +1,40 @@ +package org.csanchez.jenkins.plugins.kubernetes.pod.yaml; + +import hudson.Extension; +import hudson.model.Descriptor; +import io.fabric8.kubernetes.api.model.Pod; +import org.jenkinsci.Symbol; +import org.kohsuke.stapler.DataBoundConstructor; + +import javax.annotation.Nonnull; +import java.io.Serializable; +import java.util.List; + +import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.parseFromYaml; + +public class Overrides extends YamlMergeStrategy implements Serializable { + private static final long serialVersionUID = 4510341864619338164L; + + @DataBoundConstructor + public Overrides() { + } + + @Override + public Pod merge(List yamls) { + if (yamls.isEmpty()) { + return null; + } else { + return parseFromYaml(yamls.get(yamls.size()-1)); + } + } + + @Extension + @Symbol("override") + public static class DescriptorImpl extends Descriptor { + @Nonnull + @Override + public String getDisplayName() { + return "Override"; + } + } +} diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java new file mode 100644 index 0000000000..edbef3fec7 --- /dev/null +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java @@ -0,0 +1,16 @@ +package org.csanchez.jenkins.plugins.kubernetes.pod.yaml; + +import hudson.ExtensionPoint; +import hudson.model.AbstractDescribableImpl; +import io.fabric8.kubernetes.api.model.Pod; + +import javax.annotation.Nonnull; +import java.util.List; + +public abstract class YamlMergeStrategy extends AbstractDescribableImpl implements ExtensionPoint { + public static YamlMergeStrategy defaultStrategy() { + return new Overrides(); + } + + public abstract Pod merge(@Nonnull List yamls); +} diff --git a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/config.jelly b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/config.jelly index 6562fc5871..35d27ba8e0 100644 --- a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/config.jelly +++ b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/config.jelly @@ -71,6 +71,8 @@ + + diff --git a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-yamlMergeStrategy.html b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-yamlMergeStrategy.html new file mode 100644 index 0000000000..24ad526833 --- /dev/null +++ b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-yamlMergeStrategy.html @@ -0,0 +1,6 @@ +Defines how the raw yaml field gets merged with yaml definitions from inherited pod templates. + +
    +
  • Override: The yaml definitions from inherited pod templates are completely overridden by the current yaml field.
    • +
  • Merge: The yaml definitions from inherited pod templates are merged with the current template yaml fragment.
    • +
\ No newline at end of file diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java index db0584e2d0..2115fcd636 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java @@ -20,6 +20,8 @@ import org.apache.commons.compress.utils.IOUtils; import org.csanchez.jenkins.plugins.kubernetes.model.KeyValueEnvVar; import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar; +import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.Merge; +import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.YamlMergeStrategy; import org.csanchez.jenkins.plugins.kubernetes.volumes.EmptyDirVolume; import org.csanchez.jenkins.plugins.kubernetes.volumes.HostPathVolume; import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume; @@ -331,6 +333,7 @@ public void yamlMergeContainers() throws Exception { " - cat\n" + " tty: true\n" ); + child.setYamlMergeStrategy(merge()); child.setInheritFrom("parent"); setupStubs(); PodTemplate result = combine(parent, child); @@ -372,6 +375,7 @@ public void yamlOverrideContainer() throws Exception { " tty: true\n" ); child.setInheritFrom("parent"); + child.setYamlMergeStrategy(merge()); setupStubs(); PodTemplate result = combine(parent, child); Pod pod = new PodTemplateBuilder(result).withSlave(slave).build(); @@ -382,6 +386,10 @@ public void yamlOverrideContainer() throws Exception { assertEquals("busybox2", container.get().getImage()); } + public YamlMergeStrategy merge() { + return new Merge(); + } + @Issue("JENKINS-58374") @Test public void yamlOverrideContainerEnvvar() throws Exception { @@ -396,6 +404,7 @@ public void yamlOverrideContainerEnvvar() throws Exception { " - name: VAR2\n" + " value: \"1\"\n"); PodTemplate child = new PodTemplate(); + child.setYamlMergeStrategy(new Merge()); child.setYaml("kind: Pod\n" + "spec:\n" + " containers:\n" + @@ -439,6 +448,7 @@ public void yamlOverrideVolume() throws Exception { " path: /host/data2\n" ); child.setInheritFrom("parent"); + child.setYamlMergeStrategy(merge()); setupStubs(); PodTemplate result = combine(parent, child); Pod pod = new PodTemplateBuilder(result).withSlave(slave).build(); @@ -474,6 +484,7 @@ public void yamlMergeVolumes() throws Exception { " path: /host/data2\n" ); child.setInheritFrom("parent"); + child.setYamlMergeStrategy(merge()); setupStubs(); PodTemplate result = combine(parent, child); Pod pod = new PodTemplateBuilder(result).withSlave(slave).build(); diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java index 58f9453aac..593e7f2ea2 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java @@ -345,4 +345,16 @@ public void runInPodWithRetention() throws Exception { r.assertBuildStatusSuccess(r.waitForCompletion(b)); assertTrue(deletePods(cloud.connect(), getLabels(this, name), true)); } + + @Test + @Issue("JENKINS-58405") + public void overrideYaml() throws Exception { + r.assertBuildStatusSuccess(r.waitForCompletion(b)); + } + + @Test + @Issue("JENKINS-58405") + public void mergeYaml() throws Exception { + r.assertBuildStatusSuccess(r.waitForCompletion(b)); + } } diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/mergeYaml.groovy b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/mergeYaml.groovy new file mode 100644 index 0000000000..c59893891c --- /dev/null +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/mergeYaml.groovy @@ -0,0 +1,22 @@ +podTemplate(label: 'mergeYaml-parent', yaml: """ +spec: + containers: + - name: jnlp + env: + - name: VAR1 + value: 1 +""") { + podTemplate(label: 'mergeYaml-child', + yamlMergeStrategy: merge(), + yaml: """ + containers: + - name: jnlp + env: + - name: VAR2 + value: 1 +""") { + node('mergeYaml-child'){ + sh '["$VAR1" == "$VAR2"]' + } + } +} \ No newline at end of file diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/overrideYaml.groovy b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/overrideYaml.groovy new file mode 100644 index 0000000000..e05dfde37b --- /dev/null +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/overrideYaml.groovy @@ -0,0 +1,21 @@ +podTemplate(label: 'mergeYaml-parent', yaml: """ +spec: + containers: + - name: jnlp + env: + - name: VAR1 + value: 1 +""") { + podTemplate(label: 'mergeYaml-child', + yaml: """ + containers: + - name: jnlp + env: + - name: VAR2 + value: 1 +""") { + node('mergeYaml-child'){ + sh '["$VAR1" != "$VAR2"]' + } + } +} \ No newline at end of file From 162e4041a255e1f4d8789f5b5ab15da64f970abe Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Tue, 9 Jul 2019 17:49:35 +0200 Subject: [PATCH 2/5] Simplify a bit --- .../jenkins/plugins/kubernetes/PodTemplate.java | 10 ++-------- .../jenkins/plugins/kubernetes/pod/yaml/Merge.java | 2 +- .../jenkins/plugins/kubernetes/pod/yaml/Overrides.java | 2 +- .../plugins/kubernetes/pod/yaml/YamlMergeStrategy.java | 3 ++- 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java index d7e60d276e..c329589467 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java @@ -833,15 +833,9 @@ public Descriptor getDefaultPodRetention() { @SuppressWarnings("unused") // Used by jelly @Restricted(DoNotUse.class) // Used by jelly - public Descriptor getDefaultYamlMergeStrategy() { - Jenkins jenkins = Jenkins.getInstanceOrNull(); - if (jenkins == null) { - return null; - } - return jenkins.getDescriptor(YamlMergeStrategy.defaultStrategy().getClass()); + public YamlMergeStrategy getDefaultYamlMergeStrategy() { + return YamlMergeStrategy.defaultStrategy(); } - - } @Override diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java index 8a6dcebcef..8372fb4e76 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java @@ -14,7 +14,7 @@ import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.combine; -public class Merge extends YamlMergeStrategy implements Serializable { +public class Merge extends YamlMergeStrategy { private static final long serialVersionUID = 6562610892063268131L; @DataBoundConstructor diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java index a64ec03ddb..2babddd2d6 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java @@ -12,7 +12,7 @@ import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.parseFromYaml; -public class Overrides extends YamlMergeStrategy implements Serializable { +public class Overrides extends YamlMergeStrategy { private static final long serialVersionUID = 4510341864619338164L; @DataBoundConstructor diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java index edbef3fec7..741a258b09 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/YamlMergeStrategy.java @@ -5,9 +5,10 @@ import io.fabric8.kubernetes.api.model.Pod; import javax.annotation.Nonnull; +import java.io.Serializable; import java.util.List; -public abstract class YamlMergeStrategy extends AbstractDescribableImpl implements ExtensionPoint { +public abstract class YamlMergeStrategy extends AbstractDescribableImpl implements ExtensionPoint, Serializable { public static YamlMergeStrategy defaultStrategy() { return new Overrides(); } From e01f122916f09230b9e7ab5cc6d55ac770abac56 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Wed, 10 Jul 2019 10:19:20 +0200 Subject: [PATCH 3/5] Put each descriptor description in its own file --- .../csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java | 1 - .../jenkins/plugins/kubernetes/pod/yaml/Overrides.java | 1 - .../kubernetes/PodTemplate/help-yamlMergeStrategy.html | 5 ----- .../jenkins/plugins/kubernetes/pod/yaml/Merge/config.jelly | 5 +++++ .../plugins/kubernetes/pod/yaml/Overrides/config.jelly | 5 +++++ 5 files changed, 10 insertions(+), 7 deletions(-) create mode 100644 src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge/config.jelly create mode 100644 src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides/config.jelly diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java index 8372fb4e76..0adca46978 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge.java @@ -8,7 +8,6 @@ import org.kohsuke.stapler.DataBoundConstructor; import javax.annotation.Nonnull; -import java.io.Serializable; import java.util.List; import java.util.stream.Collectors; diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java index 2babddd2d6..3157271b0f 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides.java @@ -7,7 +7,6 @@ import org.kohsuke.stapler.DataBoundConstructor; import javax.annotation.Nonnull; -import java.io.Serializable; import java.util.List; import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.parseFromYaml; diff --git a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-yamlMergeStrategy.html b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-yamlMergeStrategy.html index 24ad526833..3da76ecab4 100644 --- a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-yamlMergeStrategy.html +++ b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-yamlMergeStrategy.html @@ -1,6 +1 @@ Defines how the raw yaml field gets merged with yaml definitions from inherited pod templates. - -
    -
  • Override: The yaml definitions from inherited pod templates are completely overridden by the current yaml field.
    • -
  • Merge: The yaml definitions from inherited pod templates are merged with the current template yaml fragment.
    • -
\ No newline at end of file diff --git a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge/config.jelly b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge/config.jelly new file mode 100644 index 0000000000..451886f724 --- /dev/null +++ b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Merge/config.jelly @@ -0,0 +1,5 @@ + + + The yaml definitions from inherited pod templates are merged with the current template yaml fragment. + diff --git a/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides/config.jelly b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides/config.jelly new file mode 100644 index 0000000000..3dd1a4de4a --- /dev/null +++ b/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pod/yaml/Overrides/config.jelly @@ -0,0 +1,5 @@ + + + The yaml definitions from inherited pod templates are completely overridden by the current yaml field. + From 8846ee4da172eba7862f02c78a95fe5193f6756c Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Wed, 10 Jul 2019 10:21:22 +0200 Subject: [PATCH 4/5] merge() should be private and moved to bottom --- .../plugins/kubernetes/PodTemplateBuilderTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java index 2115fcd636..08db89359f 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java @@ -386,10 +386,6 @@ public void yamlOverrideContainer() throws Exception { assertEquals("busybox2", container.get().getImage()); } - public YamlMergeStrategy merge() { - return new Merge(); - } - @Issue("JENKINS-58374") @Test public void yamlOverrideContainerEnvvar() throws Exception { @@ -404,7 +400,7 @@ public void yamlOverrideContainerEnvvar() throws Exception { " - name: VAR2\n" + " value: \"1\"\n"); PodTemplate child = new PodTemplate(); - child.setYamlMergeStrategy(new Merge()); + child.setYamlMergeStrategy(merge()); child.setYaml("kind: Pod\n" + "spec:\n" + " containers:\n" + @@ -522,4 +518,8 @@ private Map toContainerMap(Pod pod) { private String loadYamlFile(String s) throws IOException { return new String(IOUtils.toByteArray(getClass().getResourceAsStream(s))); } + + private YamlMergeStrategy merge() { + return new Merge(); + } } From 7bf0dcae43084c5c8a6e6bb1564e2defd7e4fa33 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Wed, 10 Jul 2019 11:32:32 +0200 Subject: [PATCH 5/5] Update README with the new attribute --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 8d9adf6ff7..cfdd8b82de 100644 --- a/README.md +++ b/README.md @@ -141,6 +141,7 @@ Either way it provides access to the following fields: * **namespace** The namespace of the pod. * **label** The label of the pod. Set a unique value to avoid conflicts across builds * **yaml** [yaml representation of the Pod](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.10/#pod-v1-core), to allow setting any values not supported as fields +* **yamlMergeStrategy** `merge()` or `override()`. Controls whether the yaml definition overrides or is merged with the yaml definition inherited from pod templates declared with `inheritFrom`. Defaults to `override()`. * **containers** The container templates that are use to create the containers of the pod *(see below)*. * **serviceAccount** The service account of the pod. * **nodeSelector** The node selector of the pod.