From f802fb27dd14f9208a0177ca5d11deb6ffadb39f Mon Sep 17 00:00:00 2001 From: Carroll Chiou Date: Mon, 2 Dec 2024 10:36:55 -0700 Subject: [PATCH] Use AbstractCloudSlave and AbstractCloudComputer --- .../hudson/plugins/ec2/EC2AbstractSlave.java | 22 +++++++++++-------- .../java/hudson/plugins/ec2/EC2Computer.java | 10 ++++++--- .../plugins/ec2/EC2ComputerLauncher.java | 18 +++++++++++++-- .../hudson/plugins/ec2/EC2OndemandSlave.java | 7 ++++-- .../plugins/ec2/EC2RetentionStrategy.java | 7 +++++- .../hudson/plugins/ec2/EC2SlaveMonitor.java | 7 +++++- .../java/hudson/plugins/ec2/EC2SpotSlave.java | 16 ++++++++------ .../plugins/ec2/EC2AbstractSlaveTest.java | 9 ++++---- .../plugins/ec2/EC2RetentionStrategyTest.java | 15 ++++++++----- .../SshHostKeyVerificationStrategyTest.java | 3 ++- .../ec2/util/EC2AgentFactoryMockImpl.java | 5 ++--- 11 files changed, 79 insertions(+), 40 deletions(-) diff --git a/src/main/java/hudson/plugins/ec2/EC2AbstractSlave.java b/src/main/java/hudson/plugins/ec2/EC2AbstractSlave.java index e70319f68..4396921c6 100644 --- a/src/main/java/hudson/plugins/ec2/EC2AbstractSlave.java +++ b/src/main/java/hudson/plugins/ec2/EC2AbstractSlave.java @@ -42,9 +42,9 @@ 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.AbstractCloudSlave; import hudson.slaves.ComputerLauncher; import hudson.slaves.NodeProperty; import hudson.slaves.RetentionStrategy; @@ -72,7 +72,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; @@ -664,7 +664,7 @@ public String getInstanceId() { } @Override - public Computer createComputer() { + public EC2Computer createComputer() { return new EC2Computer(this); } @@ -685,10 +685,6 @@ public static Instance getInstance(String instanceId, EC2Cloud cloud) { } return i; } - /** - * Terminates the instance in EC2. - */ - public abstract void terminate(); void stop() { try { @@ -748,7 +744,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(); } @@ -756,7 +756,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 03e7b2494..caca24058 100644 --- a/src/main/java/hudson/plugins/ec2/EC2Computer.java +++ b/src/main/java/hudson/plugins/ec2/EC2Computer.java @@ -29,7 +29,7 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import hudson.Util; import hudson.model.Node; -import hudson.slaves.SlaveComputer; +import hudson.slaves.AbstractCloudComputer; import java.io.IOException; import java.util.Collections; import java.util.logging.Level; @@ -41,7 +41,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 b268ed941..2ae56acf1 100644 --- a/src/main/java/hudson/plugins/ec2/EC2ComputerLauncher.java +++ b/src/main/java/hudson/plugins/ec2/EC2ComputerLauncher.java @@ -55,7 +55,14 @@ public void launch(SlaveComputer slaveComputer, TaskListener listener) { 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) { @@ -70,7 +77,14 @@ public void launch(SlaveComputer slaveComputer, TaskListener listener) { 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 fe759f14a..0f0a5a927 100644 --- a/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java +++ b/src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java @@ -4,9 +4,11 @@ import com.amazonaws.services.ec2.AmazonEC2; import com.amazonaws.services.ec2.model.*; import hudson.Extension; +import hudson.Functions; import hudson.model.Computer; import hudson.model.Descriptor.FormException; import hudson.model.Node; +import hudson.model.TaskListener; import hudson.plugins.ec2.ssh.EC2MacLauncher; import hudson.plugins.ec2.ssh.EC2UnixLauncher; import hudson.plugins.ec2.win.EC2WindowsLauncher; @@ -435,7 +437,7 @@ public EC2OndemandSlave(String instanceId) throws FormException, IOException { * Terminates the instance in EC2. */ @Override - public void terminate() { + protected void _terminate(TaskListener listener) throws IOException, InterruptedException { if (terminateScheduled.getCount() == 0) { synchronized (terminateScheduled) { if (terminateScheduled.getCount() == 0) { @@ -456,7 +458,8 @@ 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 21b4df61d..5636ca245 100644 --- a/src/main/java/hudson/plugins/ec2/EC2RetentionStrategy.java +++ b/src/main/java/hudson/plugins/ec2/EC2RetentionStrategy.java @@ -32,6 +32,7 @@ import hudson.model.Queue; import hudson.plugins.ec2.util.MinimumInstanceChecker; import hudson.slaves.RetentionStrategy; +import java.io.IOException; import java.time.Clock; import java.util.Objects; import java.util.concurrent.TimeUnit; @@ -363,7 +364,11 @@ private void postJobAction(Executor executor) { 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 (" diff --git a/src/main/java/hudson/plugins/ec2/EC2SlaveMonitor.java b/src/main/java/hudson/plugins/ec2/EC2SlaveMonitor.java index 5209c89b6..ddd46530d 100644 --- a/src/main/java/hudson/plugins/ec2/EC2SlaveMonitor.java +++ b/src/main/java/hudson/plugins/ec2/EC2SlaveMonitor.java @@ -48,7 +48,12 @@ 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 2f1372b1f..cc18323f9 100644 --- a/src/main/java/hudson/plugins/ec2/EC2SpotSlave.java +++ b/src/main/java/hudson/plugins/ec2/EC2SpotSlave.java @@ -10,8 +10,10 @@ import com.amazonaws.services.ec2.model.TerminateInstancesRequest; import edu.umd.cs.findbugs.annotations.CheckForNull; import hudson.Extension; +import hudson.Functions; import hudson.model.Computer; import hudson.model.Descriptor.FormException; +import hudson.model.TaskListener; import hudson.plugins.ec2.ssh.EC2UnixLauncher; import hudson.plugins.ec2.win.EC2WindowsLauncher; import hudson.slaves.NodeProperty; @@ -178,7 +180,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) { @@ -196,7 +198,8 @@ 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 @@ -214,15 +217,14 @@ 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)); } } } } catch (Exception e) { - LOGGER.log(Level.WARNING, "Failed to remove agent: ", e); + Functions.printStackTrace(e, listener.error("Failed to remove agent")); } finally { // Remove the instance even if deletion failed, otherwise it will hang around forever in // the nodes page. One way for this to occur is that an instance was terminated diff --git a/src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java b/src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java index a52d997e2..84f5084af 100644 --- a/src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java +++ b/src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java @@ -8,7 +8,9 @@ import com.amazonaws.services.ec2.model.InstanceType; import hudson.model.Node; +import hudson.model.TaskListener; import hudson.slaves.NodeProperty; +import java.io.IOException; import java.util.ArrayList; import java.util.List; import org.junit.Rule; @@ -56,10 +58,7 @@ public void testGetLaunchTimeoutInMillisShouldNotOverflow() throws Exception { 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 public String getEc2Type() { @@ -150,7 +149,7 @@ public void testMaxUsesBackwardCompat() throws Exception { ConnectionStrategy.PRIVATE_IP, 0) { @Override - public void terminate() {} + protected void _terminate(TaskListener listener) throws IOException, InterruptedException {} @Override public String getEc2Type() { diff --git a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java b/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java index 4ce202d0a..a266ec7e4 100644 --- a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java +++ b/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java @@ -18,6 +18,7 @@ import hudson.model.Queue.Executable; import hudson.model.Queue.Task; import hudson.model.ResourceList; +import hudson.model.TaskListener; import hudson.model.queue.CauseOfBlockage; import hudson.plugins.ec2.util.AmazonEC2FactoryMockImpl; import hudson.plugins.ec2.util.MinimumInstanceChecker; @@ -29,6 +30,7 @@ import hudson.security.Permission; import hudson.slaves.NodeProperty; import hudson.slaves.OfflineCause; +import java.io.IOException; import java.time.Clock; import java.time.Duration; import java.time.Instant; @@ -244,7 +246,7 @@ private EC2Computer computerWithIdleTime( ConnectionStrategy.PRIVATE_IP, -1) { @Override - public void terminate() {} + protected void _terminate(TaskListener listener) throws IOException, InterruptedException {} @Override public String getEc2Type() { @@ -369,7 +371,7 @@ private EC2Computer computerWithUpTime( ConnectionStrategy.PRIVATE_IP, -1) { @Override - public void terminate() {} + protected void _terminate(TaskListener listener) throws IOException, InterruptedException {} @Override public String getEc2Type() { @@ -515,9 +517,7 @@ private EC2Computer computerWithUsageLimit(final int usageLimit) throws Exceptio ConnectionStrategy.PRIVATE_IP, usageLimit) { @Override - public void terminate() { - terminateCalled.set(true); - } + protected void _terminate(TaskListener listener) throws IOException, InterruptedException {} @Override public String getEc2Type() { @@ -1148,6 +1148,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 e9974f54f..9c109a68c 100644 --- a/src/test/java/hudson/plugins/ec2/ssh/verifiers/SshHostKeyVerificationStrategyTest.java +++ b/src/test/java/hudson/plugins/ec2/ssh/verifiers/SshHostKeyVerificationStrategyTest.java @@ -9,6 +9,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; @@ -378,7 +379,7 @@ private static MockEC2Computer createComputer(String suffix) throws Exception { ConnectionStrategy.PRIVATE_IP, -1) { @Override - public void terminate() {} + protected void _terminate(TaskListener listener) throws IOException, InterruptedException {} @Override public String getEc2Type() { diff --git a/src/test/java/hudson/plugins/ec2/util/EC2AgentFactoryMockImpl.java b/src/test/java/hudson/plugins/ec2/util/EC2AgentFactoryMockImpl.java index 67aa26b84..7d1339768 100644 --- a/src/test/java/hudson/plugins/ec2/util/EC2AgentFactoryMockImpl.java +++ b/src/test/java/hudson/plugins/ec2/util/EC2AgentFactoryMockImpl.java @@ -2,7 +2,6 @@ import com.amazonaws.AmazonClientException; import hudson.Extension; -import hudson.model.Computer; import hudson.model.Descriptor; import hudson.plugins.ec2.*; import hudson.plugins.ec2.Tenancy; @@ -182,7 +181,7 @@ private MockEC2OndemandSlave( } @Override - public Computer createComputer() { + public EC2Computer createComputer() { return new MockEC2Computer(this); } } @@ -236,7 +235,7 @@ private MockEC2SpotSlave( } @Override - public Computer createComputer() { + public EC2Computer createComputer() { return new MockEC2Computer(this); } }