From 81617a66f3ad81ef30986deae1cbee17cd334b3b Mon Sep 17 00:00:00 2001 From: Carroll Chiou Date: Mon, 28 Oct 2024 11:54:51 -0600 Subject: [PATCH 1/5] Use AbstractCloudSlave and AbstractCloudComputer --- .../hudson/plugins/ec2/EC2AbstractSlave.java | 23 +++++++++++-------- .../java/hudson/plugins/ec2/EC2Computer.java | 10 +++++--- .../plugins/ec2/EC2ComputerLauncher.java | 12 ++++++++-- .../hudson/plugins/ec2/EC2OndemandSlave.java | 5 +++- .../plugins/ec2/EC2RetentionStrategy.java | 7 +++++- .../hudson/plugins/ec2/EC2SlaveMonitor.java | 6 ++++- .../java/hudson/plugins/ec2/EC2SpotSlave.java | 6 ++++- .../plugins/ec2/EC2AbstractSlaveTest.java | 11 +++++---- .../plugins/ec2/EC2RetentionStrategyTest.java | 9 +++++--- .../SshHostKeyVerificationStrategyTest.java | 4 +++- .../ec2/util/EC2AgentFactoryMockImpl.java | 4 ++-- 11 files changed, 69 insertions(+), 28 deletions(-) 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..c31b615ae 100644 --- a/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java +++ b/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java @@ -4,6 +4,7 @@ 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 +90,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,6 +111,7 @@ public void terminate() { Jenkins.get().removeNode(this); LOGGER.info("Removed EC2 instance from jenkins controller: " + getInstanceId()); } catch (AmazonClientException | IOException e) { + listener.error("Failed to terminate EC2 instance: " + getInstanceId()); LOGGER.log(Level.WARNING, "Failed to terminate EC2 instance: " + getInstanceId(), e); } finally { synchronized(terminateScheduled) { 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..8c24bb382 100644 --- a/src/main/java/hudson/plugins/ec2/EC2SpotSlave.java +++ b/src/main/java/hudson/plugins/ec2/EC2SpotSlave.java @@ -22,6 +22,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 +67,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,6 +84,7 @@ public void terminate() { LOGGER.info("Cancelled Spot request: " + spotInstanceRequestId); } catch (AmazonClientException e) { // Spot request is no longer valid + listener.error("Failed to cancel Spot request: " + spotInstanceRequestId); LOGGER.log(Level.WARNING, "Failed to cancel Spot request: " + spotInstanceRequestId, e); } @@ -100,6 +102,7 @@ public void terminate() { LOGGER.info("Terminated EC2 instance (terminated): " + instanceId); } catch (AmazonClientException e) { // Spot request is no longer valid + listener.error("Failed to terminate the Spot instance: " + instanceId); LOGGER.log(Level.WARNING, "Failed to terminate the Spot instance: " + instanceId, e); } } @@ -114,6 +117,7 @@ public void terminate() { try { Jenkins.get().removeNode(this); } catch (IOException e) { + listener.error("Failed to remove agent"); LOGGER.log(Level.WARNING, "Failed to remove agent: " + name, e); } synchronized(terminateScheduled) { diff --git a/src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java b/src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java index a044237f6..0e61c4f6d 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,8 @@ 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 { + return; } @Override @@ -56,7 +58,8 @@ 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 { + return; } @Override diff --git a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java b/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java index 4e4ad5bb3..4ada7092e 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; @@ -210,7 +211,8 @@ 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 { + return; } @Override @@ -279,7 +281,8 @@ 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 { + return; } @Override @@ -368,7 +371,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); } 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..f2a127fac 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,8 @@ 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 { + return; } @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); } } From bbcfe078cdfadfc953ae019fa7c458216ca16869 Mon Sep 17 00:00:00 2001 From: Carroll Chiou Date: Tue, 29 Oct 2024 14:05:24 -0600 Subject: [PATCH 2/5] cleanup --- .../hudson/plugins/ec2/EC2RetentionStrategyTest.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java b/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java index 4ada7092e..0ea3f4da8 100644 --- a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java +++ b/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java @@ -19,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; @@ -48,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; @@ -56,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 { From 753e04ada93fab3c258d348449ecb44f5c3f7396 Mon Sep 17 00:00:00 2001 From: Carroll Chiou Date: Thu, 31 Oct 2024 11:14:11 -0600 Subject: [PATCH 3/5] address comments --- src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java | 3 +-- src/main/java/hudson/plugins/ec2/EC2SpotSlave.java | 9 +++------ .../java/hudson/plugins/ec2/EC2AbstractSlaveTest.java | 2 -- .../hudson/plugins/ec2/EC2RetentionStrategyTest.java | 2 -- .../verifiers/SshHostKeyVerificationStrategyTest.java | 1 - 5 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java b/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java index c31b615ae..4b3f168ab 100644 --- a/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java +++ b/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java @@ -111,8 +111,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted Jenkins.get().removeNode(this); LOGGER.info("Removed EC2 instance from jenkins controller: " + getInstanceId()); } catch (AmazonClientException | IOException e) { - listener.error("Failed to terminate EC2 instance: " + getInstanceId()); - LOGGER.log(Level.WARNING, "Failed to terminate EC2 instance: " + getInstanceId(), e); + e.printStackTrace(listener.error("Failed to terminate EC2 instance: " + getInstanceId())); } finally { synchronized(terminateScheduled) { terminateScheduled.countDown(); diff --git a/src/main/java/hudson/plugins/ec2/EC2SpotSlave.java b/src/main/java/hudson/plugins/ec2/EC2SpotSlave.java index 8c24bb382..116975bed 100644 --- a/src/main/java/hudson/plugins/ec2/EC2SpotSlave.java +++ b/src/main/java/hudson/plugins/ec2/EC2SpotSlave.java @@ -84,8 +84,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted LOGGER.info("Cancelled Spot request: " + spotInstanceRequestId); } catch (AmazonClientException e) { // Spot request is no longer valid - listener.error("Failed to cancel Spot request: " + spotInstanceRequestId); - LOGGER.log(Level.WARNING, "Failed to cancel Spot request: " + spotInstanceRequestId, e); + e.printStackTrace(listener.error("Failed to cancel Spot request: " + spotInstanceRequestId)); } // Terminate the agent if it is running @@ -102,8 +101,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted LOGGER.info("Terminated EC2 instance (terminated): " + instanceId); } catch (AmazonClientException e) { // Spot request is no longer valid - listener.error("Failed to terminate the Spot instance: " + instanceId); - LOGGER.log(Level.WARNING, "Failed to terminate the Spot instance: " + instanceId, e); + e.printStackTrace(listener.error("Failed to terminate the Spot instance: " + instanceId)); } } } @@ -117,8 +115,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted try { Jenkins.get().removeNode(this); } catch (IOException e) { - listener.error("Failed to remove agent"); - LOGGER.log(Level.WARNING, "Failed to remove agent: " + name, e); + e.printStackTrace(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 0e61c4f6d..5532de763 100644 --- a/src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java +++ b/src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java @@ -33,7 +33,6 @@ public void testGetLaunchTimeoutInMillisShouldNotOverflow() throws Exception { @Override protected void _terminate(TaskListener listener) throws IOException, InterruptedException { - return; } @Override @@ -59,7 +58,6 @@ public void testMaxUsesBackwardCompat() throws Exception { 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 protected void _terminate(TaskListener listener) throws IOException, InterruptedException { - return; } @Override diff --git a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java b/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java index 0ea3f4da8..7e67aa104 100644 --- a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java +++ b/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java @@ -204,7 +204,6 @@ private EC2Computer computerWithIdleTime(final int minutes, final int seconds, f 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 protected void _terminate(TaskListener listener) throws IOException, InterruptedException { - return; } @Override @@ -274,7 +273,6 @@ private EC2Computer computerWithUpTime(final int minutes, final int seconds, fin 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 protected void _terminate(TaskListener listener) throws IOException, InterruptedException { - return; } @Override 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 f2a127fac..6d74319f6 100644 --- a/src/test/java/hudson/plugins/ec2/ssh/verifiers/SshHostKeyVerificationStrategyTest.java +++ b/src/test/java/hudson/plugins/ec2/ssh/verifiers/SshHostKeyVerificationStrategyTest.java @@ -341,7 +341,6 @@ 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 protected void _terminate(TaskListener listener) throws IOException, InterruptedException { - return; } @Override From 8b95a323909b18a7bf4bba82fe92ad942520dfec Mon Sep 17 00:00:00 2001 From: Carroll Chiou Date: Fri, 1 Nov 2024 09:23:04 -0600 Subject: [PATCH 4/5] use Functions.printStackTrace --- src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java | 3 ++- src/main/java/hudson/plugins/ec2/EC2SpotSlave.java | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java b/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java index 4b3f168ab..e0727bcfc 100644 --- a/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java +++ b/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java @@ -1,6 +1,7 @@ package hudson.plugins.ec2; import hudson.Extension; +import hudson.Functions; import hudson.model.Descriptor.FormException; import hudson.model.Computer; import hudson.model.Node; @@ -111,7 +112,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted Jenkins.get().removeNode(this); LOGGER.info("Removed EC2 instance from jenkins controller: " + getInstanceId()); } catch (AmazonClientException | IOException e) { - e.printStackTrace(listener.error("Failed to terminate EC2 instance: " + getInstanceId())); + 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/EC2SpotSlave.java b/src/main/java/hudson/plugins/ec2/EC2SpotSlave.java index 116975bed..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; @@ -84,7 +85,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted LOGGER.info("Cancelled Spot request: " + spotInstanceRequestId); } catch (AmazonClientException e) { // Spot request is no longer valid - e.printStackTrace(listener.error("Failed to cancel Spot request: " + spotInstanceRequestId)); + Functions.printStackTrace(e, listener.error("Failed to cancel Spot request: " + spotInstanceRequestId)); } // Terminate the agent if it is running @@ -101,7 +102,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted LOGGER.info("Terminated EC2 instance (terminated): " + instanceId); } catch (AmazonClientException e) { // Spot request is no longer valid - e.printStackTrace(listener.error("Failed to terminate the Spot instance: " + instanceId)); + Functions.printStackTrace(e, listener.error("Failed to terminate the Spot instance: " + instanceId)); } } } @@ -115,7 +116,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted try { Jenkins.get().removeNode(this); } catch (IOException e) { - e.printStackTrace(listener.error("Failed to remove agent")); + Functions.printStackTrace(e, listener.error("Failed to remove agent")); } synchronized(terminateScheduled) { terminateScheduled.countDown(); From b032ed09fdf76e7e4a5374d335f7d545c9b0427a Mon Sep 17 00:00:00 2001 From: Carroll Chiou Date: Fri, 1 Nov 2024 13:58:12 -0600 Subject: [PATCH 5/5] update checkRetentionStrategy --- .../java/hudson/plugins/ec2/EC2RetentionStrategyTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java b/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java index 7e67aa104..0018fb0ef 100644 --- a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java +++ b/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java @@ -757,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)); + } } }