support fail fast for node removal

src/main/java/hudson/plugins/ec2/EC2AbstractSlave.java

@@ -28,10 +28,11 @@
 import hudson.model.Descriptor;
 import hudson.model.Descriptor.FormException;
 import hudson.model.Node;
-import hudson.model.Slave;
 import hudson.plugins.ec2.util.AmazonEC2Factory;
 import hudson.plugins.ec2.util.ResettableCountDownLatch;
+import hudson.slaves.AbstractCloudComputer;
 import hudson.slaves.NodeProperty;
+import hudson.slaves.AbstractCloudSlave;
 import hudson.slaves.ComputerLauncher;
 import hudson.slaves.RetentionStrategy;
 import hudson.util.ListBoxModel;
@@ -76,7 +77,7 @@
  * @author Kohsuke Kawaguchi
  */
 @SuppressWarnings("serial")
-public abstract class EC2AbstractSlave extends Slave {
+public abstract class EC2AbstractSlave extends AbstractCloudSlave {
     public static final Boolean DEFAULT_METADATA_SUPPORTED = Boolean.TRUE;
     public static final Boolean DEFAULT_METADATA_ENDPOINT_ENABLED = Boolean.TRUE;
     public static final Boolean DEFAULT_METADATA_TOKENS_REQUIRED = Boolean.TRUE;
@@ -442,7 +443,7 @@ public String getInstanceId() {
     }
 
     @Override
-    public Computer createComputer() {
+    public EC2Computer createComputer() {
         return new EC2Computer(this);
     }
 
@@ -463,10 +464,6 @@ public static Instance getInstance(String instanceId, EC2Cloud cloud) {
         }
         return i;
     }
-    /**
-     * Terminates the instance in EC2.
-     */
-    public abstract void terminate();
 
     void stop() {
         try {
@@ -526,15 +523,23 @@ public boolean isAcceptingTasks() {
     void idleTimeout() {
         LOGGER.info("EC2 instance idle time expired: " + getInstanceId());
         if (!stopOnTerminate) {
-            terminate();
+            try {
+                terminate();
+            } catch (InterruptedException | IOException e) {
+                LOGGER.log(Level.WARNING, "Failed to terminate EC2 instance: " + getInstanceId(), e);
+            }
         } else {
             stop();
         }
     }
 
     void launchTimeout(){
         LOGGER.info("EC2 instance failed to launch: " + getInstanceId());
-        terminate();
+        try {
+            terminate();
+        } catch (InterruptedException | IOException e) {
+            LOGGER.log(Level.WARNING, "Failed to terminate EC2 instance: " + getInstanceId(), e);
+        }
     }
 
     public long getLaunchTimeoutInMillis() {

src/main/java/hudson/plugins/ec2/EC2Computer.java

@@ -27,13 +27,13 @@
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import hudson.Util;
 import hudson.model.Node;
-import hudson.slaves.SlaveComputer;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 
+import hudson.slaves.AbstractCloudComputer;
 import org.kohsuke.stapler.HttpRedirect;
 import org.kohsuke.stapler.HttpResponse;
 import com.amazonaws.AmazonClientException;
@@ -43,7 +43,7 @@
 /**
  * @author Kohsuke Kawaguchi
  */
-public class EC2Computer extends SlaveComputer {
+public class EC2Computer extends AbstractCloudComputer<EC2AbstractSlave> {
 
     private static final Logger LOGGER = Logger.getLogger(EC2Computer.class.getName());
 
@@ -220,7 +220,11 @@ public HttpResponse doDoDelete() throws IOException {
         checkPermission(DELETE);
         EC2AbstractSlave node = getNode();
         if (node != null)
-            node.terminate();
+            try {
+                node.terminate();
+            } catch (InterruptedException | IOException e) {
+                LOGGER.log(Level.WARNING, "Failed to terminate EC2 instance: " + getInstanceId(), e);
+            }
         return new HttpRedirect("..");
     }
 

src/main/java/hudson/plugins/ec2/EC2ComputerLauncher.java

@@ -52,7 +52,11 @@ public void launch(SlaveComputer slaveComputer, TaskListener listener) {
                 LOGGER.log(Level.FINE, String.format("Terminating the ec2 agent %s due a problem launching or connecting to it", slaveComputer.getName()), e);
                 EC2AbstractSlave ec2AbstractSlave = (EC2AbstractSlave) slaveComputer.getNode();
                 if (ec2AbstractSlave != null) {
-                    ec2AbstractSlave.terminate();
+                    try {
+                        ec2AbstractSlave.terminate();
+                    } catch (InterruptedException | IOException e1) {
+                        LOGGER.log(Level.WARNING, "Failed to terminate EC2 instance: " + ec2AbstractSlave.getInstanceId(), e1);
+                    }
                 }
             }
         } catch (InterruptedException e) {
@@ -62,7 +66,11 @@ public void launch(SlaveComputer slaveComputer, TaskListener listener) {
                 LOGGER.log(Level.FINE, String.format("Terminating the ec2 agent %s due a problem launching or connecting to it", slaveComputer.getName()), e);
                 EC2AbstractSlave ec2AbstractSlave = (EC2AbstractSlave) slaveComputer.getNode();
                 if (ec2AbstractSlave != null) {
-                    ec2AbstractSlave.terminate();
+                    try {
+                        ec2AbstractSlave.terminate();
+                    } catch (InterruptedException | IOException e1) {
+                        LOGGER.log(Level.WARNING, "Failed to terminate EC2 instance: " + ec2AbstractSlave.getInstanceId(), e1);
+                    }
                 }
             }
         }

src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java

@@ -1,9 +1,11 @@
 package hudson.plugins.ec2;
 
 import hudson.Extension;
+import hudson.Functions;
 import hudson.model.Descriptor.FormException;
 import hudson.model.Computer;
 import hudson.model.Node;
+import hudson.model.TaskListener;
 import hudson.plugins.ec2.ssh.EC2UnixLauncher;
 import hudson.plugins.ec2.win.EC2WindowsLauncher;
 import hudson.plugins.ec2.ssh.EC2MacLauncher;
@@ -89,7 +91,8 @@ public EC2OndemandSlave(String instanceId) throws FormException, IOException {
     /**
      * Terminates the instance in EC2.
      */
-    public void terminate() {
+    @Override
+    protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
         if (terminateScheduled.getCount() == 0) {
             synchronized(terminateScheduled) {
                 if (terminateScheduled.getCount() == 0) {
@@ -109,7 +112,7 @@ public void terminate() {
                             Jenkins.get().removeNode(this);
                             LOGGER.info("Removed EC2 instance from jenkins controller: " + getInstanceId());
                         } catch (AmazonClientException | IOException e) {
-                            LOGGER.log(Level.WARNING, "Failed to terminate EC2 instance: " + getInstanceId(), e);
+                            Functions.printStackTrace(e, listener.error("Failed to terminate EC2 instance: " + getInstanceId()));
                         } finally {
                             synchronized(terminateScheduled) {
                                 terminateScheduled.countDown();

src/main/java/hudson/plugins/ec2/EC2RetentionStrategy.java

@@ -36,6 +36,7 @@
 import hudson.slaves.RetentionStrategy;
 import jenkins.model.Jenkins;
 
+import java.io.IOException;
 import java.time.Clock;
 import java.util.Objects;
 import java.util.concurrent.TimeUnit;
@@ -346,7 +347,11 @@ private void postJobAction(Executor executor) {
                 // At this point, if agent is in suspended state and has 1 last executer running, it is safe to terminate.
                 if (computer.countBusy() <= 1 && !computer.isAcceptingTasks()) {
                     LOGGER.info("Agent " + slaveNode.instanceId + " is terminated due to maxTotalUses (" + slaveNode.maxTotalUses + ")");
-                    slaveNode.terminate();
+                    try {
+                        slaveNode.terminate();
+                    } catch (InterruptedException | IOException e) {
+                        LOGGER.log(Level.WARNING, "Failed to terminate EC2 instance: " + slaveNode.getInstanceId(), e);
+                    }
                 } else {
                     if (slaveNode.maxTotalUses == 1) {
                         LOGGER.info("Agent " + slaveNode.instanceId + " is still in use by more than one (" + computer.countBusy() + ") executers.");

src/main/java/hudson/plugins/ec2/EC2SlaveMonitor.java

@@ -51,7 +51,11 @@ private void removeDeadNodes() {
                 try {
                     if (!ec2Slave.isAlive(true)) {
                         LOGGER.info("EC2 instance is dead: " + ec2Slave.getInstanceId());
-                        ec2Slave.terminate();
+                        try {
+                            ec2Slave.terminate();
+                        } catch (InterruptedException | IOException e) {
+                            LOGGER.log(Level.WARNING, "Failed to terminate EC2 instance: " + ec2Slave.getInstanceId(), e);
+                        }
                     }
                 } catch (AmazonClientException e) {
                     if (e instanceof AmazonEC2Exception &&

src/main/java/hudson/plugins/ec2/EC2SpotSlave.java

@@ -7,6 +7,7 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import hudson.Functions;
 import jenkins.model.Jenkins;
 import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.DataBoundConstructor;
@@ -22,6 +23,7 @@
 
 import hudson.Extension;
 import hudson.model.Computer;
+import hudson.model.TaskListener;
 import hudson.model.Descriptor.FormException;
 import hudson.plugins.ec2.ssh.EC2UnixLauncher;
 import hudson.plugins.ec2.win.EC2WindowsLauncher;
@@ -66,7 +68,7 @@ protected boolean isAlive(boolean force) {
      * Cancel the spot request for the instance. Terminate the instance if it is up. Remove the agent from Jenkins.
      */
     @Override
-    public void terminate() {
+    protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
         if (terminateScheduled.getCount() == 0) {
             synchronized(terminateScheduled) {
                 if (terminateScheduled.getCount() == 0) {
@@ -83,7 +85,7 @@ public void terminate() {
                                 LOGGER.info("Cancelled Spot request: " + spotInstanceRequestId);
                             } catch (AmazonClientException e) {
                                 // Spot request is no longer valid
-                                LOGGER.log(Level.WARNING, "Failed to cancel Spot request: " + spotInstanceRequestId, e);
+                                Functions.printStackTrace(e, listener.error("Failed to cancel Spot request: " + spotInstanceRequestId));
                             }
 
                             // Terminate the agent if it is running
@@ -100,7 +102,7 @@ public void terminate() {
                                         LOGGER.info("Terminated EC2 instance (terminated): " + instanceId);
                                     } catch (AmazonClientException e) {
                                         // Spot request is no longer valid
-                                        LOGGER.log(Level.WARNING, "Failed to terminate the Spot instance: " + instanceId, e);
+                                        Functions.printStackTrace(e, listener.error("Failed to terminate the Spot instance: " + instanceId));
                                     }
                                 }
                             }
@@ -114,7 +116,7 @@ public void terminate() {
                             try {
                                 Jenkins.get().removeNode(this);
                             } catch (IOException e) {
-                                LOGGER.log(Level.WARNING, "Failed to remove agent: " + name, e);
+                                Functions.printStackTrace(e, listener.error("Failed to remove agent"));
                             }
                             synchronized(terminateScheduled) {
                                 terminateScheduled.countDown();

src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java

@@ -7,6 +7,9 @@
 
 import hudson.slaves.NodeProperty;
 import hudson.model.Node;
+import hudson.model.TaskListener;
+
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 import org.junit.Rule;
@@ -29,9 +32,7 @@ public void testGetLaunchTimeoutInMillisShouldNotOverflow() throws Exception {
             DEFAULT_METADATA_ENDPOINT_ENABLED, DEFAULT_METADATA_TOKENS_REQUIRED, DEFAULT_METADATA_HOPS_LIMIT, DEFAULT_METADATA_SUPPORTED) {
 
             @Override
-            public void terminate() {
-                // To change body of implemented methods use File | Settings |
-                // File Templates.
+            protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
             }
 
             @Override
@@ -56,7 +57,7 @@ public void testMaxUsesBackwardCompat() throws Exception {
         r.jenkins.clouds.add(ac);
         EC2AbstractSlave slave = new EC2AbstractSlave("name", "", description, "fs", 1, null, "label", null, null, "init", "tmpDir", new ArrayList<NodeProperty<?>>(), "root", "jvm", false, "idle", null, cloudName, false, Integer.MAX_VALUE, new UnixData("remote", null, null, "22", null), ConnectionStrategy.PRIVATE_IP, 0)  {
             @Override
-            public void terminate() {
+            protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
             }
 
             @Override

src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java

@@ -8,6 +8,7 @@
 import hudson.model.Label;
 import hudson.model.Queue;
 import hudson.model.ResourceList;
+import hudson.model.TaskListener;
 import hudson.model.Queue.Executable;
 import hudson.model.Queue.Task;
 import hudson.model.queue.CauseOfBlockage;
@@ -18,20 +19,17 @@
 import hudson.plugins.ec2.util.SSHCredentialHelper;
 import hudson.security.ACL;
 import hudson.security.AccessControlled;
-import hudson.security.Permission;import hudson.model.Executor;
+import hudson.security.Permission;
 import hudson.slaves.NodeProperty;
 import hudson.slaves.OfflineCause;
 import jenkins.util.NonLocalizable;
 import jenkins.model.Jenkins;
-import net.sf.json.JSONObject;
 import org.acegisecurity.Authentication;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.JenkinsRule;
-import org.testcontainers.shaded.org.bouncycastle.jce.provider.BouncyCastleProvider;
 import org.jvnet.hudson.test.LoggerRule;
 
-import java.security.Security;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
@@ -47,18 +45,13 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.logging.Level;
 import java.util.stream.Collectors;
-import jenkins.util.NonLocalizable;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasItem;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-import org.junit.Rule;
-import org.junit.Test;
-import org.jvnet.hudson.test.JenkinsRule;
-import org.jvnet.hudson.test.LoggerRule;
 
 public class EC2RetentionStrategyTest {
 
@@ -210,7 +203,7 @@ private EC2Computer computerWithIdleTime(final int minutes, final int seconds) t
     private EC2Computer computerWithIdleTime(final int minutes, final int seconds, final Boolean isOffline, final Boolean isConnecting) throws Exception {
         final EC2AbstractSlave slave = new EC2AbstractSlave("name", "id", "description", "fs", 1, null, "label", null, null, "init", "tmpDir", new ArrayList<NodeProperty<?>>(), "remote", "jvm", false, "idle", null, "cloud", false, Integer.MAX_VALUE, null, ConnectionStrategy.PRIVATE_IP, -1) {
             @Override
-            public void terminate() {
+            protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
             }
 
             @Override
@@ -279,7 +272,7 @@ private EC2Computer computerWithUpTime(final int minutes, final int seconds, fin
         idleTimeoutCalled.set(false);
         final EC2AbstractSlave slave = new EC2AbstractSlave("name", "id", "description", "fs", 1, null, "label", null, null, "init", "tmpDir", new ArrayList<NodeProperty<?>>(), "remote", "jvm", false, "idle", null, "cloud", false, Integer.MAX_VALUE, null, ConnectionStrategy.PRIVATE_IP, -1) {
             @Override
-            public void terminate() {
+            protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
             }
 
             @Override
@@ -368,7 +361,7 @@ public void testOnUsageCountRetention() throws Exception {
     private EC2Computer computerWithUsageLimit(final int usageLimit) throws Exception {
         final EC2AbstractSlave slave = new EC2AbstractSlave("name", "id", "description", "fs", 1, null, "label", null, null, "init", "tmpDir", new ArrayList<NodeProperty<?>>(), "remote", "jvm", false, "idle", null, "cloud", false, Integer.MAX_VALUE, null, ConnectionStrategy.PRIVATE_IP, usageLimit) {
             @Override
-            public void terminate() {
+            protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
                 terminateCalled.set(true);
             }
 
@@ -764,6 +757,9 @@ public void testRetentionStopsAfterActiveRangeEnds() throws Exception {
     private static void checkRetentionStrategy(EC2RetentionStrategy rs, EC2Computer c) throws InterruptedException {
         rs.check(c);
         EC2AbstractSlave node = c.getNode();
-        assertTrue(node.terminateScheduled.await(10, TimeUnit.SECONDS));
+        // checks if node has been terminated within timeout. if node is null, it has already been terminated
+        if (node != null) {
+            assertTrue(node.terminateScheduled.await(10, TimeUnit.SECONDS));
+        }
     }
 }

src/test/java/hudson/plugins/ec2/ssh/verifiers/SshHostKeyVerificationStrategyTest.java

@@ -5,6 +5,7 @@
 import com.trilead.ssh2.Connection;
 import com.trilead.ssh2.ServerHostKeyVerifier;
 import hudson.model.Node;
+import hudson.model.TaskListener;
 import hudson.plugins.ec2.ConnectionStrategy;
 import hudson.plugins.ec2.EC2AbstractSlave;
 import hudson.plugins.ec2.EC2Computer;
@@ -339,7 +340,7 @@ public MockEC2Computer(EC2AbstractSlave slave) {
         private static MockEC2Computer createComputer(String suffix) throws Exception {
             final EC2AbstractSlave slave = new EC2AbstractSlave(COMPUTER_NAME + suffix, "id" + suffix, "description" + suffix, "fs", 1, null, "label", null, null, "init", "tmpDir", new ArrayList<NodeProperty<?>>(), "remote", "jvm", false, "idle", null, "cloud", false, Integer.MAX_VALUE, null, ConnectionStrategy.PRIVATE_IP, -1) {
                 @Override
-                public void terminate() {
+                protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
                 }
 
                 @Override

src/test/java/hudson/plugins/ec2/util/EC2AgentFactoryMockImpl.java

@@ -53,7 +53,7 @@ private MockEC2OndemandSlave(String name, String instanceId, String description,
         }
 
         @Override
-        public Computer createComputer() {
+        public EC2Computer createComputer() {
             return new MockEC2Computer(this);
         }
     }
@@ -67,7 +67,7 @@ private MockEC2SpotSlave(String name, String spotInstanceRequestId, String descr
         }
 
         @Override
-        public Computer createComputer() {
+        public EC2Computer createComputer() {
             return new MockEC2Computer(this);
         }
     }