[JENKINS-72112] Allow merge strategy inheritance in PodTemplates (#1513)

Co-authored-by: Vincent Latombe <[email protected]>
jenkinsci · Mar 15, 2024 · a1647c2 · a1647c2
1 parent 5da9435
commit a1647c2
Show file tree

Hide file tree

Showing 10 changed files with 118 additions and 9 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
@@ -195,17 +195,23 @@ public YamlMergeStrategy getYamlMergeStrategy() {
         return yamlMergeStrategy;
     }
 
+    public YamlMergeStrategy getResolvedYamlMergeStrategy() {
+        return getYamlMergeStrategy() != null ? getYamlMergeStrategy() : YamlMergeStrategy.defaultStrategy();
+    }
+
     @DataBoundSetter
     public void setYamlMergeStrategy(YamlMergeStrategy yamlMergeStrategy) {
         this.yamlMergeStrategy = yamlMergeStrategy;
     }
 
-    private YamlMergeStrategy yamlMergeStrategy = YamlMergeStrategy.defaultStrategy();
+    private YamlMergeStrategy yamlMergeStrategy;
 
     public Pod getYamlsPod() {
-        return yamlMergeStrategy.merge(getYamls());
+        return getResolvedYamlMergeStrategy().merge(getYamls());
     }
 
+    private Boolean inheritYamlMergeStrategy;
+
     private Boolean showRawYaml;
 
     /**
@@ -950,9 +956,6 @@ protected Object readResolve() {
             yamls = null;
         }
 
-        if (yamlMergeStrategy == null) {
-            yamlMergeStrategy = YamlMergeStrategy.defaultStrategy();
-        }
         if (id == null) {
             // Use the label and a digest of the current object representation to get the same value every restart if
             // the object isn't saved.
@@ -986,6 +989,15 @@ public String getDescriptionForLogging() {
                 "Agent specification [%s] (%s): %n%s", getName(), getLabel(), getContainersDescriptionForLogging());
     }
 
+    public boolean isInheritYamlMergeStrategy() {
+        return inheritYamlMergeStrategy != null ? inheritYamlMergeStrategy.booleanValue() : false;
+    }
+
+    @DataBoundSetter
+    public void setInheritYamlMergeStrategy(boolean inheritYamlMergeStrategy) {
+        this.inheritYamlMergeStrategy = Boolean.valueOf(inheritYamlMergeStrategy);
+    }
+
     boolean isShowRawYamlSet() {
         return showRawYaml != null;
     }

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java
@@ -510,7 +510,11 @@ public static PodTemplate combine(PodTemplate parent, PodTemplate template) {
         podTemplate.setAnnotations(new ArrayList<>(podAnnotations));
         podTemplate.setNodeProperties(nodeProperties);
         podTemplate.setNodeUsageMode(nodeUsageMode);
-        podTemplate.setYamlMergeStrategy(template.getYamlMergeStrategy());
+        podTemplate.setYamlMergeStrategy(
+                template.getYamlMergeStrategy() == null && parent.isInheritYamlMergeStrategy()
+                        ? parent.getYamlMergeStrategy()
+                        : template.getYamlMergeStrategy());
+        podTemplate.setInheritYamlMergeStrategy(parent.isInheritYamlMergeStrategy());
         podTemplate.setInheritFrom(
                 !isNullOrEmpty(template.getInheritFrom()) ? template.getInheritFrom() : parent.getInheritFrom());
 

diff --git a/...ain/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java b/...ain/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java
@@ -91,6 +91,8 @@ public class KubernetesDeclarativeAgent extends RetryableDeclarativeAgent<Kubern
 
     private YamlMergeStrategy yamlMergeStrategy;
 
+    private Boolean inheritYamlMergeStrategy;
+
     @CheckForNull
     private WorkspaceVolume workspaceVolume;
 
@@ -316,6 +318,15 @@ public void setYamlMergeStrategy(YamlMergeStrategy yamlMergeStrategy) {
         this.yamlMergeStrategy = yamlMergeStrategy;
     }
 
+    public boolean isInheritYamlMergeStrategy() {
+        return inheritYamlMergeStrategy != null ? inheritYamlMergeStrategy.booleanValue() : false;
+    }
+
+    @DataBoundSetter
+    public void setInheritYamlMergeStrategy(boolean inheritYamlMergeStrategy) {
+        this.inheritYamlMergeStrategy = Boolean.valueOf(inheritYamlMergeStrategy);
+    }
+
     public WorkspaceVolume getWorkspaceVolume() {
         return workspaceVolume == null ? PodTemplateStep.DescriptorImpl.defaultWorkspaceVolume : this.workspaceVolume;
     }
@@ -367,6 +378,9 @@ public Map<String, Object> getAsArgs() {
         if (yamlMergeStrategy != null) {
             argMap.put("yamlMergeStrategy", yamlMergeStrategy);
         }
+        if (inheritYamlMergeStrategy != null) {
+            argMap.put("inheritYamlMergeStrategy", inheritYamlMergeStrategy);
+        }
         if (workspaceVolume != null) {
             argMap.put("workspaceVolume", workspaceVolume);
         }

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
@@ -86,7 +86,9 @@ public class PodTemplateStep extends Step implements Serializable {
     @CheckForNull
     private String yaml;
 
-    private YamlMergeStrategy yamlMergeStrategy = YamlMergeStrategy.defaultStrategy();
+    private YamlMergeStrategy yamlMergeStrategy;
+
+    private Boolean inheritYamlMergeStrategy;
 
     @CheckForNull
     private PodRetention podRetention;
@@ -361,6 +363,15 @@ public void setPodRetention(@CheckForNull PodRetention podRetention) {
                 (podRetention == null || podRetention.equals(DescriptorImpl.defaultPodRetention)) ? null : podRetention;
     }
 
+    public boolean isInheritYamlMergeStrategy() {
+        return inheritYamlMergeStrategy != null ? inheritYamlMergeStrategy.booleanValue() : false;
+    }
+
+    @DataBoundSetter
+    public void setInheritYamlMergeStrategy(boolean inheritYamlMergeStrategy) {
+        this.inheritYamlMergeStrategy = Boolean.valueOf(inheritYamlMergeStrategy);
+    }
+
     boolean isShowRawYamlSet() {
         return showRawYaml != null;
     }

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
@@ -118,6 +118,7 @@ public boolean start() throws Exception {
         newTemplate.setListener(listener);
         newTemplate.setYamlMergeStrategy(step.getYamlMergeStrategy());
         if (run != null) {
+            newTemplate.setInheritYamlMergeStrategy(step.isInheritYamlMergeStrategy());
             String url = cloud.getJenkinsUrlOrNull();
             if (url != null) {
                 newTemplate.getAnnotations().add(new PodAnnotation("buildUrl", url + run.getUrl()));

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
@@ -89,6 +89,10 @@ THE SOFTWARE.
 
   <f:dropdownDescriptorSelector title="${%Yaml merge strategy}" field="yamlMergeStrategy" default="${descriptor.defaultYamlMergeStrategy}" />
 
+    <f:entry field="inheritYamlMergeStrategy" title="${%Inherit yaml merge strategy}" >
+      <f:checkbox/>
+    </f:entry>
+
   <f:entry field="showRawYaml" title="${%Show raw yaml in console}" >
     <f:checkbox default="true"/>
   </f:entry>

diff --git a/...es/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-inheritYamlMergeStrategy.html b/...es/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-inheritYamlMergeStrategy.html
@@ -0,0 +1 @@
+Controls whether the defined yaml merge strategy will be inherited if another defined pod template is configured to inherit from the current one.
diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java
@@ -40,6 +40,7 @@
 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.Overrides;
 import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.YamlMergeStrategy;
 import org.csanchez.jenkins.plugins.kubernetes.volumes.ConfigMapVolume;
 import org.csanchez.jenkins.plugins.kubernetes.volumes.EmptyDirVolume;
@@ -584,6 +585,62 @@ public void testInheritsFromWithYaml(boolean directConnection) throws Exception
         validateContainers(pod, slave, directConnection);
     }
 
+    @Test
+    public void inheritYamlMergeStrategy() throws Exception {
+        PodTemplate parent = new PodTemplate();
+        parent.setYaml("apiVersion: v1\n" + "kind: Pod\n"
+                + "spec:\n"
+                + "  tolerations:\n"
+                + "  - key: \"reservedFor\"\n"
+                + "    operator: Exists\n"
+                + "    effect: NoSchedule");
+
+        PodTemplate child = new PodTemplate();
+        child.setYaml("spec:\n");
+        child.setInheritFrom("parent");
+        setupStubs();
+
+        PodTemplate result;
+        Pod pod;
+
+        // Default behavior (backward compatible)
+        parent.setYamlMergeStrategy(merge());
+        parent.setInheritYamlMergeStrategy(false);
+        result = combine(parent, child);
+        pod = new PodTemplateBuilder(result, slave).build();
+        assertThat(pod.getSpec().getTolerations(), hasSize(0));
+
+        // Inherit merge strategy with merge
+        parent.setYamlMergeStrategy(merge());
+        parent.setInheritYamlMergeStrategy(true);
+        result = combine(parent, child);
+        pod = new PodTemplateBuilder(result, slave).build();
+        assertThat(pod.getSpec().getTolerations(), hasSize(1));
+
+        // Inherit merge strategy with override
+        parent.setYamlMergeStrategy(overrides());
+        parent.setInheritYamlMergeStrategy(true);
+        result = combine(parent, child);
+        pod = new PodTemplateBuilder(result, slave).build();
+        assertThat(pod.getSpec().getTolerations(), hasSize(0));
+
+        // Override merge strategy with overrides
+        parent.setYamlMergeStrategy(merge());
+        parent.setInheritYamlMergeStrategy(true);
+        child.setYamlMergeStrategy(overrides());
+        result = combine(parent, child);
+        pod = new PodTemplateBuilder(result, slave).build();
+        assertThat(pod.getSpec().getTolerations(), hasSize(0));
+
+        // Override overrides strategy with merge
+        parent.setYamlMergeStrategy(overrides());
+        parent.setInheritYamlMergeStrategy(true);
+        child.setYamlMergeStrategy(merge());
+        result = combine(parent, child);
+        pod = new PodTemplateBuilder(result, slave).build();
+        assertThat(pod.getSpec().getTolerations(), hasSize(1));
+    }
+
     @Test
     public void yamlMergeContainers() throws Exception {
         PodTemplate parent = new PodTemplate();
@@ -935,6 +992,10 @@ private String loadYamlFile(String s) throws IOException {
         return new String(IOUtils.toByteArray(getClass().getResourceAsStream(s)));
     }
 
+    private YamlMergeStrategy overrides() {
+        return new Overrides();
+    }
+
     private YamlMergeStrategy merge() {
         return new Merge();
     }

diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/casc/CasCTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/casc/CasCTest.java
@@ -6,6 +6,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import io.jenkins.plugins.casc.misc.RoundTripAbstractTest;
@@ -43,7 +44,8 @@ protected void assertConfiguredAsExpected(RestartableJenkinsRule r, String confi
         assertEquals(123, podTemplate.getSlaveConnectTimeout());
         assertEquals(5, podTemplate.getIdleMinutes());
         assertEquals(66, podTemplate.getActiveDeadlineSeconds());
-        assertThat(podTemplate.getYamlMergeStrategy(), isA(Overrides.class));
+        assertNull(podTemplate.getYamlMergeStrategy());
+        assertThat(podTemplate.getResolvedYamlMergeStrategy(), isA(Overrides.class));
         podTemplate = templates.get(1);
         assertFalse(podTemplate.isHostNetwork());
         assertEquals("dynamic-pvc", podTemplate.getLabel());

diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/casc/configuration-as-code.yaml b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/casc/configuration-as-code.yaml
@@ -9,7 +9,6 @@ jenkins:
           - hostNetwork: false
             label: "java"
             name: "default-java"
-            yamlMergeStrategy: "override"
             instanceCap: 10
             slaveConnectTimeout: 123
             idleMinutes: 5
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Controls whether the defined yaml merge strategy will be inherited if another defined pod template is configured to inherit from the current one.