Merge pull request #382 from Vlatombe/JENKINS-50196

[JENKINS-50196] Fix nested Pod Templates support
jenkinsci · Oct 10, 2018 · 5fbdd1a · 5fbdd1a
2 parents ef177b3 + b96bd14
commit 5fbdd1a
Show file tree

Hide file tree

Showing 8 changed files with 130 additions and 20 deletions.
diff --git a/...in/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java b/...in/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java
@@ -40,6 +40,7 @@
  * @since 1.1.1
  *
  */
+@Deprecated
 public abstract class AbstractInvisibleRunAction2 extends InvisibleAction implements RunAction2 {
 
     private static final Logger LOGGER = Logger.getLogger(AbstractInvisibleRunAction2.class.getName());

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java
@@ -10,6 +10,10 @@
 import hudson.model.Run;
 import jenkins.model.RunAction2;
 
+/**
+ * Use <pre>getContext().get(PodTemplateContext.class)</pre> instead.
+ */
+@Deprecated
 public class NamespaceAction extends AbstractInvisibleRunAction2 implements RunAction2 {
 
     private static final Logger LOGGER = Logger.getLogger(NamespaceAction.class.getName());

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java
@@ -11,6 +11,10 @@
 import hudson.model.Run;
 import jenkins.model.RunAction2;
 
+/**
+ * Use <pre>getContext().get(PodTemplateContext.class)</pre> instead.
+ */
+@Deprecated
 public class PodTemplateAction extends AbstractInvisibleRunAction2 implements RunAction2 {
 
     private static final Logger LOGGER = Logger.getLogger(PodTemplateAction.class.getName());

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateContext.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateContext.java
@@ -0,0 +1,26 @@
+package org.csanchez.jenkins.plugins.kubernetes.pipeline;
+
+import java.io.Serializable;
+
+/**
+ * Context object for PodTemplate during pipeline execution
+ */
+public class PodTemplateContext implements Serializable {
+    private static final long serialVersionUID = 3065143885759619305L;
+
+    private final String namespace;
+    private final String name;
+
+    public PodTemplateContext(String namespace, String name) {
+        this.namespace = namespace;
+        this.name = name;
+    }
+
+    public String getNamespace() {
+        return namespace;
+    }
+
+    public String getName() {
+        return name;
+    }
+}
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
@@ -69,15 +69,14 @@ public boolean start() throws Exception {
             checkAccess(run, kubernetesCloud);
         }
 
-        PodTemplateAction podTemplateAction = run.getAction(PodTemplateAction.class);
-        NamespaceAction namespaceAction = run.getAction(NamespaceAction.class);
-        String parentTemplates = podTemplateAction != null ? podTemplateAction.getParentTemplates() : null;
+        PodTemplateContext podTemplateContext = getContext().get(PodTemplateContext.class);
+        String parentTemplates = podTemplateContext != null ? podTemplateContext.getName() : null;
 
         //Let's generate a random name based on the user specified to make sure that we don't have
         //issues with concurrent builds, or messing with pre-existing configuration
         String randString = RandomStringUtils.random(5, "bcdfghjklmnpqrstvwxz0123456789");
         String name = String.format(NAME_FORMAT, step.getName(), randString);
-        String namespace = checkNamespace(kubernetesCloud, namespaceAction);
+        String namespace = checkNamespace(kubernetesCloud, podTemplateContext);
 
         newTemplate = new PodTemplate();
         newTemplate.setName(name);
@@ -120,18 +119,11 @@ public boolean start() throws Exception {
         }
 
         kubernetesCloud.addDynamicTemplate(newTemplate);
-        getContext().newBodyInvoker().withContext(step).withCallback(new PodTemplateCallback(newTemplate)).start();
+        getContext().newBodyInvoker().withContexts(step, new PodTemplateContext(namespace, name)).withCallback(new PodTemplateCallback(newTemplate)).start();
 
-        PodTemplateAction.push(run, name);
-        NamespaceAction.push(run, namespace);
         return false;
     }
 
-    @Override
-    public void stop(Throwable cause) throws Exception {
-        new PodTemplateAction(getContext().get(Run.class)).pop();
-    }
-
     /**
      * Check if the current Job is permitted to use the cloud.
      * 
@@ -152,12 +144,12 @@ private void checkAccess(Run<?, ?> run, KubernetesCloud kubernetesCloud) throws
         }
     }
 
-    private String checkNamespace(KubernetesCloud kubernetesCloud, @CheckForNull NamespaceAction namespaceAction) {
+    private String checkNamespace(KubernetesCloud kubernetesCloud, @CheckForNull PodTemplateContext podTemplateContext) {
         String namespace = null;
         if (!Strings.isNullOrEmpty(step.getNamespace())) {
             namespace = step.getNamespace();
-        } else if ((namespaceAction != null) && (!Strings.isNullOrEmpty(namespaceAction.getNamespace()))) {
-            namespace = namespaceAction.getNamespace();
+        } else if (podTemplateContext != null && !Strings.isNullOrEmpty(podTemplateContext.getNamespace())) {
+            namespace = podTemplateContext.getNamespace();
         } else {
             namespace = kubernetesCloud.getNamespace();
         }

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
@@ -37,9 +37,11 @@
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
+import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import org.junit.rules.TestName;
 import org.jvnet.hudson.test.JenkinsRuleNonLocalhost;
 
 import hudson.model.Result;
@@ -58,6 +60,9 @@ public class KubernetesPipelineTest extends AbstractKubernetesPipelineTest {
     @Rule
     public TemporaryFolder tmp = new TemporaryFolder();
 
+    @Rule
+    public TestName name = new TestName();
+
     @Test
     public void runInPod() throws Exception {
         deletePods(cloud.connect(), getLabels(cloud, this), false);
@@ -99,6 +104,44 @@ public void runInPod() throws Exception {
                 deletePods(cloud.connect(), getLabels(cloud, this), true));
     }
 
+    @Test
+    public void runIn2Pods() throws Exception {
+        deletePods(cloud.connect(), getLabels(cloud, this), false);
+
+        WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
+        p.setDefinition(new CpsFlowDefinition(loadPipelineScript(name.getMethodName() + ".groovy"), true));
+        WorkflowRun b = p.scheduleBuild2(0).waitForStart();
+        assertNotNull(b);
+        SemaphoreStep.waitForStart("podTemplate1/1", b);
+        PodTemplate template1 = podTemplateWithLabel("mypod", cloud.getAllTemplates());
+        SemaphoreStep.success("podTemplate1/1", null);
+        assertEquals(Integer.MAX_VALUE, template1.getInstanceCap());
+        assertThat(template1.getLabelsMap(), hasEntry("jenkins/mypod", "true"));
+        SemaphoreStep.waitForStart("pod1/1", b);
+        Map<String, String> labels1 = getLabels(cloud, this);
+        labels1.put("jenkins/mypod", "true");
+        PodList pods = cloud.connect().pods().withLabels(labels1).list();
+        assertTrue(!pods.getItems().isEmpty());
+        SemaphoreStep.success("pod1/1", null);
+
+        SemaphoreStep.waitForStart("podTemplate2/1", b);
+        PodTemplate template2 = podTemplateWithLabel("mypod2", cloud.getAllTemplates());
+        SemaphoreStep.success("podTemplate2/1", null);
+        assertEquals(Integer.MAX_VALUE, template2.getInstanceCap());
+        assertThat(template2.getLabelsMap(), hasEntry("jenkins/mypod2", "true"));
+        assertNull("mypod2 should not inherit from anything", template2.getInheritFrom());
+        SemaphoreStep.waitForStart("pod2/1", b);
+        Map<String, String> labels2 = getLabels(cloud, this);
+        labels1.put("jenkins/mypod2", "true");
+        PodList pods2 = cloud.connect().pods().withLabels(labels2).list();
+        assertTrue(!pods2.getItems().isEmpty());
+        SemaphoreStep.success("pod2/1", null);
+        r.assertBuildStatusSuccess(r.waitForCompletion(b));
+        r.assertLogContains("script file contents: ", b);
+        assertFalse("There are pods leftover after test execution, see previous logs",
+                deletePods(cloud.connect(), getLabels(cloud, this), true));
+    }
+
     private PodTemplate podTemplateWithLabel(String label, List<PodTemplate> templates) {
         return templates != null ? templates.stream().filter(t -> label.equals(t.getLabel())).findFirst().orElse(null)
                 : null;
@@ -301,8 +344,9 @@ public void runDirContext() throws Exception {
     }
 
     @Test
-    public void runWithOverriddenNamespace() throws Exception {
+    public void runWithCloudOverriddenNamespace() throws Exception {
         String overriddenNamespace = testingNamespace + "-overridden-namespace";
+        cloud.setNamespace(overriddenNamespace);
         KubernetesClient client = cloud.connect();
         // Run in our own testing namespace
         if (client.namespaces().withName(overriddenNamespace).get() == null) {
@@ -311,11 +355,10 @@ public void runWithOverriddenNamespace() throws Exception {
         }
 
         WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "overriddenNamespace");
-        p.setDefinition(new CpsFlowDefinition(loadPipelineScript("runWithOverriddenNamespace.groovy"), true));
+        p.setDefinition(new CpsFlowDefinition(loadPipelineScript(name.getMethodName()+".groovy"), true));
 
         WorkflowRun b = p.scheduleBuild2(0).waitForStart();
         assertNotNull(b);
-        NamespaceAction.push(b, overriddenNamespace);
 
         r.assertBuildStatusSuccess(r.waitForCompletion(b));
         r.assertLogContains(overriddenNamespace, b);
@@ -328,6 +371,7 @@ public void runWithOverriddenNamespace() throws Exception {
     public void runWithStepOverriddenNamespace() throws Exception {
         String overriddenNamespace = testingNamespace + "-overridden-namespace";
         String stepNamespace = testingNamespace + "-overridden-namespace2";
+        cloud.setNamespace(overriddenNamespace);
         KubernetesClient client = cloud.connect();
         // Run in our own testing namespace
         if (client.namespaces().withName(stepNamespace).get() == null) {
@@ -336,12 +380,11 @@ public void runWithStepOverriddenNamespace() throws Exception {
         }
 
         WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "stepOverriddenNamespace");
-        p.setDefinition(new CpsFlowDefinition(loadPipelineScript("runWithStepOverriddenNamespace.groovy")
+        p.setDefinition(new CpsFlowDefinition(loadPipelineScript(name.getMethodName()+".groovy")
                 .replace("OVERRIDDEN_NAMESPACE", stepNamespace), true));
 
         WorkflowRun b = p.scheduleBuild2(0).waitForStart();
         assertNotNull(b);
-        NamespaceAction.push(b, overriddenNamespace);
 
         r.assertBuildStatusSuccess(r.waitForCompletion(b));
         r.assertLogContains(stepNamespace, b);

diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runIn2Pods.groovy b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runIn2Pods.groovy
@@ -0,0 +1,40 @@
+podTemplate(label: 'mypod', containers: [
+        containerTemplate(name: 'busybox', image: 'busybox', ttyEnabled: true, command: '/bin/cat'),
+    ]) {
+    semaphore 'podTemplate1'
+    node ('mypod') {
+        semaphore 'pod1'
+        stage('Run') {
+            container('busybox') {
+              sh """
+            ## durable-task plugin generates a script.sh file.
+            ##
+            echo "script file: \$(find ../../.. -iname script.sh))"
+            echo "script file contents: \$(find ../../.. -iname script.sh -exec cat {} \\;)"
+            test -n "\$(cat \$(find ../../.. -iname script.sh))"
+              """
+            }
+        }
+    }
+}
+
+podTemplate(label: 'mypod2', containers: [
+        containerTemplate(name: 'busybox2', image: 'busybox', ttyEnabled: true, command: '/bin/cat'),
+]) {
+    semaphore 'podTemplate2'
+    node ('mypod2') {
+        semaphore 'pod2'
+        stage('Run') {
+            container('busybox2') {
+
+                sh """
+            ## durable-task plugin generates a script.sh file.
+            ##
+            echo "script file: \$(find ../../.. -iname script.sh))"
+            echo "script file contents: \$(find ../../.. -iname script.sh -exec cat {} \\;)"
+            test -n "\$(cat \$(find ../../.. -iname script.sh))"
+          """
+            }
+        }
+    }
+}
diff --git a/...ipeline/runWithOverriddenNamespace.groovy → ...ne/runWithCloudOverriddenNamespace.groovy b/...ipeline/runWithOverriddenNamespace.groovy → ...ne/runWithCloudOverriddenNamespace.groovy