Merge pull request #1304 from jglick/lazy-pod

jenkinsci · Feb 1, 2023 · a982397 · a982397
2 parents 7ff395e + cd43a3c
commit a982397
Show file tree

Hide file tree

Showing 8 changed files with 42 additions and 23 deletions.
diff --git a/Jenkinsfile b/Jenkinsfile
@@ -42,12 +42,14 @@ stage('Tests') {
         }
     }
     branches['jdk11'] = {
-        node('maven-11') {
-            timeout(60) {
-                checkout scm
-                sh 'mvn -B -ntp -Dset.changelist -Dmaven.test.failure.ignore clean install'
-                infra.prepareToPublishIncrementals()
-                junit 'target/surefire-reports/*.xml'
+        retry(count: 3, conditions: [kubernetesAgent(handleNonKubernetes: true), nonresumable()]) {
+            node('maven-11') {
+                timeout(60) {
+                    checkout scm
+                    sh 'mvn -B -ntp -Dset.changelist -Dmaven.test.failure.ignore clean install'
+                    infra.prepareToPublishIncrementals()
+                    junit 'target/surefire-reports/*.xml'
+                }
             }
         }
     }

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java
@@ -310,8 +310,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted
         // Prior to termination, determine if we should delete the slave pod based on
         // the slave pod's current state and the pod retention policy.
         // Healthy slave pods should still have a JNLP agent running at this point.
-        Pod pod = client.pods().inNamespace(getNamespace()).withName(name).get();
-        boolean deletePod = getPodRetention(cloud).shouldDeletePod(cloud, pod);
+        boolean deletePod = getPodRetention(cloud).shouldDeletePod(cloud, () -> client.pods().inNamespace(getNamespace()).withName(name).get());
 
         Computer computer = toComputer();
         if (computer == null) {
@@ -367,6 +366,7 @@ private void deleteSlavePod(TaskListener listener, KubernetesClient client) thro
                     e.getMessage());
             LOGGER.log(Level.WARNING, msg, e);
             listener.error(msg);
+            // TODO should perhaps retry later, in case API server is just overloaded currently
             return;
         }
 

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Always.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Always.java
@@ -8,6 +8,7 @@
 
 import hudson.Extension;
 import io.fabric8.kubernetes.api.model.Pod;
+import java.util.function.Supplier;
 
 public class Always extends PodRetention implements Serializable {
 
@@ -19,7 +20,7 @@ public Always() {
     }
 
     @Override
-    public boolean shouldDeletePod(KubernetesCloud cloud, Pod pod) {
+    public boolean shouldDeletePod(KubernetesCloud cloud, Supplier<Pod> pod) {
         return false;
     }
 

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Default.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Default.java
@@ -10,6 +10,7 @@
 import hudson.model.Descriptor;
 import hudson.model.DescriptorVisibilityFilter;
 import io.fabric8.kubernetes.api.model.Pod;
+import java.util.function.Supplier;
 
 public class Default extends PodRetention implements Serializable {
 
@@ -21,7 +22,7 @@ public Default() {
     }
 
     @Override
-    public boolean shouldDeletePod(KubernetesCloud cloud, Pod pod) {
+    public boolean shouldDeletePod(KubernetesCloud cloud, Supplier<Pod> pod) {
         PodRetention parent = cloud.getPodRetention();
         if (!(parent instanceof Default)) {
             return parent.shouldDeletePod(cloud, pod);

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Never.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Never.java
@@ -8,6 +8,7 @@
 
 import hudson.Extension;
 import io.fabric8.kubernetes.api.model.Pod;
+import java.util.function.Supplier;
 
 public class Never extends PodRetention implements Serializable {
 
@@ -19,7 +20,7 @@ public Never() {
     }
 
     @Override
-    public boolean shouldDeletePod(KubernetesCloud cloud, Pod pod) {
+    public boolean shouldDeletePod(KubernetesCloud cloud, Supplier<Pod> pod) {
         return true;
     }
 

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/OnFailure.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/OnFailure.java
@@ -8,18 +8,29 @@
 
 import hudson.Extension;
 import io.fabric8.kubernetes.api.model.Pod;
+import java.util.function.Supplier;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 public class OnFailure extends PodRetention implements Serializable {
 
     private static final long serialVersionUID = 6424267627207206819L;
 
+    private static final Logger LOGGER = Logger.getLogger(OnFailure.class.getName());
+
     @DataBoundConstructor
     public OnFailure() {
 
     }
 
     @Override
-    public boolean shouldDeletePod(KubernetesCloud cloud, Pod pod) {
+    public boolean shouldDeletePod(KubernetesCloud cloud, Supplier<Pod> podS) {
+        Pod pod = null;
+        try {
+            pod = podS.get();
+        } catch (RuntimeException x) {
+            LOGGER.log(Level.WARNING, null, x);
+        }
         if (pod == null || pod.getStatus() == null) {
             return false;
         }

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/PodRetention.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/PodRetention.java
@@ -5,6 +5,7 @@
 import hudson.ExtensionPoint;
 import hudson.model.AbstractDescribableImpl;
 import io.fabric8.kubernetes.api.model.Pod;
+import java.util.function.Supplier;
 
 /**
  * <code>PodRetention</code> instances determine if the Kubernetes pod running a Jenkins agent
@@ -41,7 +42,7 @@ public static PodRetention getPodTemplateDefault() {
      * 
      * @return <code>true</code> if the agent pod should be deleted.
      */
-    public abstract boolean shouldDeletePod(KubernetesCloud cloud, Pod pod);
+    public abstract boolean shouldDeletePod(KubernetesCloud cloud, Supplier<Pod> pod);
 
     @Override
     public String toString() {

diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/PodRetentionTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/PodRetentionTest.java
@@ -9,11 +9,13 @@
 import io.fabric8.kubernetes.api.model.Pod;
 import io.fabric8.kubernetes.api.model.PodStatus;
 import io.fabric8.kubernetes.api.model.PodStatusBuilder;
+import java.util.function.Supplier;
 
 public class PodRetentionTest {
 
     private KubernetesCloud cloud;
     private Pod pod;
+    private Supplier<Pod> podS = () -> pod;
 
     @Before
     public void setUp() {
@@ -24,39 +26,39 @@ public void setUp() {
     @Test
     public void testAlwaysPodRetention() {
         PodRetention subject = new Always();
-        assertFalse(subject.shouldDeletePod(cloud, pod));
+        assertFalse(subject.shouldDeletePod(cloud, podS));
     }
 
     @Test
     public void testNeverPodRetention() {
         PodRetention subject = new Never();
-        assertTrue(subject.shouldDeletePod(cloud, pod));
+        assertTrue(subject.shouldDeletePod(cloud, podS));
     }
 
     @Test
     public void testDefaultPodRetention() {
         PodRetention subject = new Default();
         cloud.setPodRetention(new Always());
-        assertFalse(subject.shouldDeletePod(cloud, pod));
+        assertFalse(subject.shouldDeletePod(cloud, podS));
         cloud.setPodRetention(new Never());
-        assertTrue(subject.shouldDeletePod(cloud, pod));
+        assertTrue(subject.shouldDeletePod(cloud, podS));
         cloud.setPodRetention(new Default());
-        assertTrue(subject.shouldDeletePod(cloud, pod));
+        assertTrue(subject.shouldDeletePod(cloud, podS));
         cloud.setPodRetention(null);
-        assertTrue(subject.shouldDeletePod(cloud, pod));
+        assertTrue(subject.shouldDeletePod(cloud, podS));
     }
 
     @Test
     public void testOnFailurePodRetention() {
         PodRetention subject = new OnFailure();
         pod.setStatus(buildStatus("Failed"));
-        assertFalse(subject.shouldDeletePod(cloud, pod));
+        assertFalse(subject.shouldDeletePod(cloud, podS));
         pod.setStatus(buildStatus("Unknown"));
-        assertFalse(subject.shouldDeletePod(cloud, pod));
+        assertFalse(subject.shouldDeletePod(cloud, podS));
         pod.setStatus(buildStatus("Running"));
-        assertTrue(subject.shouldDeletePod(cloud, pod));
+        assertTrue(subject.shouldDeletePod(cloud, podS));
         pod.setStatus(buildStatus("Pending"));
-        assertTrue(subject.shouldDeletePod(cloud, pod));
+        assertTrue(subject.shouldDeletePod(cloud, podS));
     }
 
     private PodStatus buildStatus(String phase) {