diff --git a/src/main/java/hudson/plugins/ec2/EC2AbstractSlave.java b/src/main/java/hudson/plugins/ec2/EC2AbstractSlave.java index 949124021..5fabf8d75 100644 --- a/src/main/java/hudson/plugins/ec2/EC2AbstractSlave.java +++ b/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,7 +523,11 @@ 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(); } @@ -534,7 +535,11 @@ void idleTimeout() { 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() { diff --git a/src/main/java/hudson/plugins/ec2/EC2Computer.java b/src/main/java/hudson/plugins/ec2/EC2Computer.java index 489b2f938..335f7cd6f 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Computer.java +++ b/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 { 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(".."); } diff --git a/src/main/java/hudson/plugins/ec2/EC2ComputerLauncher.java b/src/main/java/hudson/plugins/ec2/EC2ComputerLauncher.java index 4d95a7b01..bd16ec6d8 100644 --- a/src/main/java/hudson/plugins/ec2/EC2ComputerLauncher.java +++ b/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); + } } } } diff --git a/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java b/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java index d648d8d10..e0727bcfc 100644 --- a/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java +++ b/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(); diff --git a/src/main/java/hudson/plugins/ec2/EC2RetentionStrategy.java b/src/main/java/hudson/plugins/ec2/EC2RetentionStrategy.java index 356e7bb47..5a1500a96 100644 --- a/src/main/java/hudson/plugins/ec2/EC2RetentionStrategy.java +++ b/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."); diff --git a/src/main/java/hudson/plugins/ec2/EC2SlaveMonitor.java b/src/main/java/hudson/plugins/ec2/EC2SlaveMonitor.java index daa622a39..511546ca8 100644 --- a/src/main/java/hudson/plugins/ec2/EC2SlaveMonitor.java +++ b/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 && diff --git a/src/main/java/hudson/plugins/ec2/EC2SpotSlave.java b/src/main/java/hudson/plugins/ec2/EC2SpotSlave.java index 04c4b861b..59fe7ee49 100644 --- a/src/main/java/hudson/plugins/ec2/EC2SpotSlave.java +++ b/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(); diff --git a/src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java b/src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java index a044237f6..5532de763 100644 --- a/src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java +++ b/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>(), "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 diff --git a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java b/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java index 4e4ad5bb3..0018fb0ef 100644 --- a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java +++ b/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,7 +45,6 @@ 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; @@ -55,10 +52,6 @@ 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>(), "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>(), "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>(), "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)); + } } } diff --git a/src/test/java/hudson/plugins/ec2/ssh/verifiers/SshHostKeyVerificationStrategyTest.java b/src/test/java/hudson/plugins/ec2/ssh/verifiers/SshHostKeyVerificationStrategyTest.java index c1dce8437..6d74319f6 100644 --- a/src/test/java/hudson/plugins/ec2/ssh/verifiers/SshHostKeyVerificationStrategyTest.java +++ b/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>(), "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 diff --git a/src/test/java/hudson/plugins/ec2/util/EC2AgentFactoryMockImpl.java b/src/test/java/hudson/plugins/ec2/util/EC2AgentFactoryMockImpl.java index 1516574b9..2dde9d080 100644 --- a/src/test/java/hudson/plugins/ec2/util/EC2AgentFactoryMockImpl.java +++ b/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); } }