Merge pull request #1137 from Vlatombe/invalid-image-gets-cancelled

Author: Vlatombe

src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java

@@ -48,6 +48,7 @@
 import java.util.stream.Collectors;
 import jenkins.metrics.api.Metrics;
 import org.apache.commons.lang.StringUtils;
+import org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper;
 import org.kohsuke.stapler.DataBoundConstructor;
 
 import static java.util.logging.Level.FINE;
@@ -94,6 +95,8 @@ public synchronized void launch(SlaveComputer computer, TaskListener listener) {
         if (!(computer instanceof KubernetesComputer)) {
             throw new IllegalArgumentException("This Launcher can be used only with KubernetesComputer");
         }
+        // Activate reaper if it never got activated.
+        Reaper.getInstance().maybeActivate();
         KubernetesComputer kubernetesComputer = (KubernetesComputer) computer;
         computer.setAcceptingTasks(false);
         KubernetesSlave node = kubernetesComputer.getNode();

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java

@@ -92,7 +92,13 @@ public static Reaper getInstance() {
 
     @Override
     public void onOnline(Computer c, TaskListener listener) throws IOException, InterruptedException {
-        if (c instanceof KubernetesComputer && activated.compareAndSet(false, true)) {
+        if (c instanceof KubernetesComputer) {
+            maybeActivate();
+        }
+    }
+
+    public void maybeActivate() {
+        if (activated.compareAndSet(false, true)) {
             activate();
         }
     }
@@ -107,6 +113,10 @@ private void activate() {
                 continue;
             }
             KubernetesSlave ks = (KubernetesSlave) n;
+            if (ks.getLauncher().isLaunchSupported()) {
+                // Being launched, don't touch it.
+                continue;
+            }
             String ns = ks.getNamespace();
             String name = ks.getPodName();
             try {
@@ -230,7 +240,7 @@ public void onEvent(@NonNull Action action, @NonNull KubernetesSlave node, @NonN
                     LOGGER.info(() -> ns + "/" + name + " Container " + c.getName() + " was just terminated, so removing the corresponding Jenkins agent");
                     runListener.getLogger().printf("%s/%s Container %s was terminated (Exit Code: %d, Reason: %s)%n", ns, name, c.getName(), t.getExitCode(), t.getReason());
                 });
-                node.terminate();
+                logLastLinesThenTerminateNode(node, pod, runListener);
             }
         }
     }
@@ -248,20 +258,24 @@ public void onEvent(@NonNull Action action, @NonNull KubernetesSlave node, @NonN
                 TaskListener runListener = node.getTemplate().getListener();
                 LOGGER.info(() -> ns + "/" + name + " Pod just failed. Removing the corresponding Jenkins agent. Reason: " + pod.getStatus().getReason() + ", Message: " + pod.getStatus().getMessage());
                 runListener.getLogger().printf("%s/%s Pod just failed (Reason: %s, Message: %s)%n", ns, name, pod.getStatus().getReason(), pod.getStatus().getMessage());
-                try {
-                    String lines = PodUtils.logLastLines(pod, node.getKubernetesCloud().connect());
-                    if (lines != null) {
-                        runListener.getLogger().print(lines);
-                    }
-                } catch (KubernetesAuthException e) {
-                    LOGGER.log(Level.FINE, e, () -> "Unable to get logs after pod failed event");
-                } finally {
-                    node.terminate();
-                }
+                logLastLinesThenTerminateNode(node, pod, runListener);
             }
         }
     }
 
+    private static void logLastLinesThenTerminateNode(KubernetesSlave node, Pod pod, TaskListener runListener) throws IOException, InterruptedException {
+        try {
+            String lines = PodUtils.logLastLines(pod, node.getKubernetesCloud().connect());
+            if (lines != null) {
+                runListener.getLogger().print(lines);
+            }
+        } catch (KubernetesAuthException e) {
+            LOGGER.log(Level.FINE, e, () -> "Unable to get logs after pod failed event");
+        } finally {
+            node.terminate();
+        }
+    }
+
     @Extension
     public static class TerminateAgentOnImagePullBackOff implements Listener {
 

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

@@ -701,6 +701,12 @@ public void invalidPodGetsCancelled() throws Exception {
         r.assertLogContains("Queue task was cancelled", b);
     }
 
+    @Test
+    public void invalidImageGetsCancelled() throws Exception {
+        r.assertBuildStatus(Result.ABORTED, r.waitForCompletion(b));
+        r.assertLogContains("Queue task was cancelled", b);
+    }
+
     @Issue("SECURITY-1646")
     @Test
     public void substituteEnv() throws Exception {

src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/invalidImageGetsCancelled.groovy

@@ -0,0 +1,10 @@
+podTemplate(yaml: '''
+spec:
+  containers:
+  - name: jnlp
+    image: some/invalid
+''') {
+  node(POD_LABEL) {
+    sh 'false'
+  }
+}