allow re-use of existing pods for restarts

- container step - containerLog step Includes: - update to parent pom to get latest jenkins-test-harness - update to durable-task plugin v1.16 [FIXES JENKINS-47476]
jenkinsci · Nov 13, 2017 · 3151b5a · 3151b5a
1 parent 4303ec3
commit 3151b5a
Show file tree

Hide file tree

Showing 15 changed files with 415 additions and 67 deletions.
diff --git a/Jenkinsfile-kubernetes b/Jenkinsfile-kubernetes
@@ -35,10 +35,15 @@ timestamps {
         }
       }
       stage('Test') {
-        container('maven') {
-            sh 'mvn -B test'
+        try {
+            container('maven') {
+                sh 'mvn -B test'
+            }
+        } catch (e) {
+            throw e
+        } finally {
+            junit '**/target/surefire-reports/**/*.xml'
         }
-        junit '**/target/surefire-reports/**/*.xml'
       }
     }
   }

diff --git a/pom.xml b/pom.xml
@@ -3,7 +3,7 @@
   <parent>
     <groupId>org.jenkins-ci.plugins</groupId>
     <artifactId>plugin</artifactId>
-    <version>2.32</version>
+    <version>2.37</version>
   </parent>
 
   <groupId>org.csanchez.jenkins.plugins</groupId>
@@ -38,12 +38,12 @@
     <jenkins.version>2.32.1</jenkins.version>
 
     <kubernetes-client.version>2.6.1</kubernetes-client.version>
-    <slf4j.version>1.7.13</slf4j.version>
+    <slf4j.version>1.7.25</slf4j.version>
 
     <!-- jenkins plugins versions -->
     <jenkins-basic-steps.version>2.3</jenkins-basic-steps.version>
     <jenkins-credentials.version>2.1.7</jenkins-credentials.version>
-    <jenkins-durable-task.version>1.13</jenkins-durable-task.version>
+    <jenkins-durable-task.version>1.16-20171108.031054-1</jenkins-durable-task.version>
     <jenkins-durable-task-step.version>2.11</jenkins-durable-task-step.version>
     <jenkins-structs.version>1.6</jenkins-structs.version>
     <jenkins-workflow-cps.version>2.29</jenkins-workflow-cps.version>

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java
@@ -120,6 +120,12 @@ public void launch(SlaveComputer computer, TaskListener listener) {
         if (slave == null) {
             throw new IllegalStateException("Node has been removed, cannot launch " + computer.getName());
         }
+        if (launched) {
+            LOGGER.info("Agent: " + slave.getNodeName() + " has already been launched. Activating...");
+            computer.setAcceptingTasks(true);
+            return;
+        }
+
         KubernetesCloud cloud = slave.getKubernetesCloud();
         final PodTemplate unwrappedTemplate = slave.getTemplate();
         try {
@@ -229,6 +235,12 @@ public void launch(SlaveComputer computer, TaskListener listener) {
             throw Throwables.propagate(ex);
         }
         launched = true;
+        try {
+            // We need to persist the "launched" setting...
+            slave.save();
+        } catch (IOException e) {
+            LOGGER.warning("Could not save() agent: " + e.getMessage());
+        }
     }
 
     private Pod getPodTemplate(KubernetesSlave slave, PodTemplate template) {

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java
@@ -74,10 +74,10 @@ public class ContainerExecDecorator extends LauncherDecorator implements Seriali
     private static final String JENKINS_HOME = "JENKINS_HOME=";
     private static final Logger LOGGER = Logger.getLogger(ContainerExecDecorator.class.getName());
 
-    private final transient KubernetesClient client;
+    private transient KubernetesClient client;
 
     @SuppressFBWarnings(value = "SE_TRANSIENT_FIELD_NOT_RESTORED", justification = "not needed on deserialization")
-    private final transient List<Closeable> closables = new ArrayList<>();
+    private transient List<Closeable> closables = new ArrayList<>();
     private final String podName;
     private final String namespace;
     private final String containerName;
@@ -239,6 +239,9 @@ public void onClose(int i, String s) {
                     this.setupEnvironmentVariable(envVars, watch);
                     doExec(watch, printStream, commands);
                     ContainerExecProc proc = new ContainerExecProc(watch, alive, finished, exitCodeOutputStream::getExitCode);
+                    if (closables == null) {
+                        closables = new ArrayList<>();
+                    }
                     closables.add(proc);
                     return proc;
                 }  catch (InterruptedException ie) {
@@ -313,6 +316,8 @@ private void waitUntilContainerIsReady() throws IOException {
 
     @Override
     public void close() throws IOException {
+        if (closables == null) return;
+
         for (Closeable closable : closables) {
             try {
                 closable.close();
@@ -378,6 +383,10 @@ private static void closeWatch(ExecWatch watch) {
         }
     }
 
+    public void setKubernetesClient(KubernetesClient client) {
+        this.client = client;
+    }
+
     /**
      * Keeps the last bytes of the output stream to parse the exit code
      */

diff --git a/...main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java b/...main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java
@@ -16,6 +16,8 @@
 
 package org.csanchez.jenkins.plugins.kubernetes.pipeline;
 
+import com.google.inject.Inject;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.model.TaskListener;
 import hudson.util.LogTaskListener;
 import io.fabric8.kubernetes.client.KubernetesClient;
@@ -33,9 +35,24 @@ public class ContainerLogStepExecution extends SynchronousNonBlockingStepExecuti
     private static final long serialVersionUID = 5588861066775717487L;
     private static final transient Logger LOGGER = Logger.getLogger(ContainerLogStepExecution.class.getName());
 
-    private final ContainerLogStep step;
+    @Inject(optional=true)
+    @SuppressFBWarnings(value = "SE_TRANSIENT_FIELD_NOT_RESTORED", justification = "not needed on deserialization")
+    private transient final ContainerLogStep step;
+
     private transient KubernetesClient client;
 
+    @Override
+    // TODO Revisit for JENKINS-40161
+    public void onResume() {
+        super.onResume();
+        LOGGER.log(Level.FINE, "ContainerLogStepExecution onResume");
+        try {
+            KubernetesNodeContext nodeContext = new KubernetesNodeContext(getContext());
+            client = nodeContext.connectToCloud();
+        } catch (Exception e) {
+            ContainerLogStepExecution.this.getContext().onFailure(e);
+        }
+    }
     ContainerLogStepExecution(ContainerLogStep step, StepContext context) {
         super(context);
         this.step = step;

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java
@@ -4,13 +4,14 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import com.google.inject.Inject;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
 import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
 import org.jenkinsci.plugins.workflow.steps.BodyInvoker;
 import org.jenkinsci.plugins.workflow.steps.EnvironmentExpander;
 import org.jenkinsci.plugins.workflow.steps.StepContext;
 
-import hudson.EnvVars;
 import hudson.LauncherDecorator;
 import io.fabric8.kubernetes.client.KubernetesClient;
 
@@ -24,10 +25,26 @@ public class ContainerStepExecution extends AbstractStepExecutionImpl {
 
     private static final transient Logger LOGGER = Logger.getLogger(ContainerStepExecution.class.getName());
 
-    private final ContainerStep step;
+    @Inject(optional=true)
+    @SuppressFBWarnings(value = "SE_TRANSIENT_FIELD_NOT_RESTORED", justification = "not needed on deserialization")
+    private final transient ContainerStep step;
 
     private transient KubernetesClient client;
-    private transient ContainerExecDecorator decorator;
+    private ContainerExecDecorator decorator;
+
+    @Override
+    // TODO Revisit for JENKINS-40161
+    public void onResume() {
+        super.onResume();
+        LOGGER.log(Level.FINE, "ContainerStepExecution onResume");
+        try {
+            KubernetesNodeContext nodeContext = new KubernetesNodeContext(getContext());
+            client = nodeContext.connectToCloud();
+            decorator.setKubernetesClient(client);
+        } catch (Exception e) {
+            ContainerStepExecution.this.getContext().onFailure(e);
+        }
+    }
 
     ContainerStepExecution(ContainerStep step, StepContext context) {
         super(context);

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
@@ -5,12 +5,15 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import com.google.inject.Inject;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.commons.lang.RandomStringUtils;
 import org.apache.commons.lang.StringUtils;
 import org.csanchez.jenkins.plugins.kubernetes.KubernetesCloud;
 import org.csanchez.jenkins.plugins.kubernetes.PodImagePullSecret;
 import org.csanchez.jenkins.plugins.kubernetes.PodTemplate;
 import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
+import org.jenkinsci.plugins.workflow.steps.BodyExecution;
 import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
 import org.jenkinsci.plugins.workflow.steps.StepContext;
 
@@ -30,7 +33,12 @@ public class PodTemplateStepExecution extends AbstractStepExecutionImpl {
 
     private static final transient String NAME_FORMAT = "%s-%s";
 
-    private final PodTemplateStep step;
+    @Inject(optional=true)
+    @SuppressFBWarnings(value = "SE_TRANSIENT_FIELD_NOT_RESTORED", justification = "not needed on deserialization")
+    private final transient PodTemplateStep step;
+    private String cloudName;
+
+    private PodTemplate newTemplate = null;
 
     PodTemplateStepExecution(PodTemplateStep step, StepContext context) {
         super(context);
@@ -40,7 +48,8 @@ public class PodTemplateStepExecution extends AbstractStepExecutionImpl {
     @Override
     public boolean start() throws Exception {
 
-        Cloud cloud = Jenkins.getInstance().getCloud(step.getCloud());
+        cloudName = step.getCloud();
+        Cloud cloud = Jenkins.getInstance().getCloud(cloudName);
         if (cloud == null) {
             throw new AbortException(String.format("Cloud does not exist: %s", step.getCloud()));
         }
@@ -59,7 +68,7 @@ public boolean start() throws Exception {
         String name = String.format(NAME_FORMAT, step.getName(), randString);
         String namespace = checkNamespace(kubernetesCloud, namespaceAction);
 
-        PodTemplate newTemplate = new PodTemplate();
+        newTemplate = new PodTemplate();
         newTemplate.setName(name);
         newTemplate.setNamespace(namespace);
         newTemplate.setInheritFrom(!Strings.isNullOrEmpty( podTemplateAction.getParentTemplates()) ? podTemplateAction.getParentTemplates() : step.getInheritFrom());
@@ -124,7 +133,7 @@ private PodTemplateCallback(PodTemplate podTemplate) {
          * Remove the template after step is done
          */
         protected void finished(StepContext context) throws Exception {
-            Cloud cloud = Jenkins.getInstance().getCloud(step.getCloud());
+            Cloud cloud = Jenkins.getInstance().getCloud(cloudName);
             if (cloud == null) {
                 LOGGER.log(Level.WARNING, "Cloud {0} no longer exists, cannot delete pod template {1}",
                         new Object[] { step.getCloud(), podTemplate.getName() });

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,18 +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.runners.model.Statement;
 import org.jvnet.hudson.test.JenkinsRuleNonLocalhost;
-import org.jvnet.hudson.test.RestartableJenkinsRule;
 
-import hudson.model.Node;
-import hudson.slaves.DumbSlave;
-import hudson.slaves.NodeProperty;
-import hudson.slaves.RetentionStrategy;
 import io.fabric8.kubernetes.api.model.NamespaceBuilder;
 import io.fabric8.kubernetes.client.KubernetesClient;
 
@@ -59,8 +52,6 @@ public class KubernetesPipelineTest extends AbstractKubernetesPipelineTest {
 
     private static final Logger LOGGER = Logger.getLogger(KubernetesPipelineTest.class.getName());
 
-    @Rule
-    public RestartableJenkinsRule story = new RestartableJenkinsRule();
     @Rule
     public TemporaryFolder tmp = new TemporaryFolder();
 
@@ -82,7 +73,7 @@ public void runInPod() throws Exception {
         PodTemplate template = templates.get(0);
         assertEquals(Integer.MAX_VALUE, template.getInstanceCap());
         r.assertBuildStatusSuccess(r.waitForCompletion(b));
-        r.assertLogContains("PID file contents: ", b);
+        r.assertLogContains("script file contents: ", b);
         assertFalse("There are pods leftover after test execution, see previous logs",
                 deletePods(cloud.connect(), KubernetesCloud.DEFAULT_POD_LABELS, true));
     }
@@ -259,39 +250,6 @@ public void runInPodWithLivenessProbe() throws Exception {
         r.assertLogContains("Still alive", b);
     }
 
-    // @Test
-    public void runInPodWithRestart() throws Exception {
-        story.addStep(new Statement() {
-            @Override
-            public void evaluate() throws Throwable {
-                story.j.jenkins.clouds.add(new KubernetesCloud("test"));
-
-                story.j.jenkins.addNode(new DumbSlave("slave", "dummy", tmp.newFolder("remoteFS").getPath(), "1",
-                        Node.Mode.NORMAL, "", story.j.createComputerLauncher(null), RetentionStrategy.NOOP,
-                        Collections.<NodeProperty<?>>emptyList())); // TODO JENKINS-26398 clumsy
-                WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p");
-                p.setDefinition(new CpsFlowDefinition(loadPipelineScript("runInPodWithRestart.groovy")
-                        , true));
-                WorkflowRun b = p.scheduleBuild2(0).waitForStart();
-                SemaphoreStep.waitForStart("withDisplayAfterRestart/1", b);
-            }
-        });
-        story.addStep(new Statement() {
-            @SuppressWarnings("SleepWhileInLoop")
-            @Override
-            public void evaluate() throws Throwable {
-                SemaphoreStep.success("withDisplayAfterRestart/1", null);
-                WorkflowJob p = story.j.jenkins.getItemByFullName("p", WorkflowJob.class);
-                assertNotNull(p);
-                WorkflowRun b = p.getBuildByNumber(1);
-                assertNotNull(b);
-                story.j.assertBuildStatusSuccess(story.j.waitForCompletion(b));
-                story.j.assertLogContains("DISPLAY=:", b);
-                r.assertLogContains("xxx", b);
-            }
-        });
-    }
-
     @Test
     public void runWithActiveDeadlineSeconds() throws Exception {
         WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "Deadline");