From 9450fa842f51719017d23fe30cdb78b47c3754c0 Mon Sep 17 00:00:00 2001 From: Prathibha Datta Kumar Date: Fri, 23 Jun 2023 10:48:50 -0500 Subject: [PATCH] Fix unnecessary decrement of maxTotalUses and add clarity to logs around terminations triggered by plugin (#375) * rename EC2TerminationCause to EC2ExecutorInterruptionCause for clarity * -add and track EC2AgentTerminationReason when plugin triggers termination -rename force to overrideOtherSettings in scheduleToTerminate for clarity https://github.com/jenkinsci/ec2-fleet-plugin/issues/363 * [fix] remove unnecessary decrement which could lead to -1 (has special meaning) [fix] remove misleading log and redundant set https://github.com/jenkinsci/ec2-fleet-plugin/issues/363 * update and add tests * rename overrideOtherSettings to ignoreMinConstraints --- .../ec2fleet/AbstractEC2FleetCloud.java | 2 +- .../ec2fleet/EC2AgentTerminationReason.java | 35 +++ ...java => EC2ExecutorInterruptionCause.java} | 6 +- .../EC2FleetAutoResubmitComputerLauncher.java | 2 +- .../jenkins/ec2fleet/EC2FleetCloud.java | 45 ++-- .../jenkins/ec2fleet/EC2FleetLabelCloud.java | 33 +-- .../ec2fleet/EC2FleetNodeComputer.java | 2 +- .../ec2fleet/EC2RetentionStrategy.java | 27 ++- .../EC2FleetCloud/help-maxTotalUses.html | 1 + ...FleetAutoResubmitComputerLauncherTest.java | 5 +- .../jenkins/ec2fleet/EC2FleetCloudTest.java | 48 ++-- .../ec2fleet/EC2FleetCloudWithMeter.java | 4 +- .../EC2MaxTotalUsesRetentionStrategyTest.java | 8 +- .../EC2RetentionStrategyIntegrationTest.java | 5 + .../ec2fleet/EC2RetentionStrategyTest.java | 217 ++++++++++++------ 15 files changed, 290 insertions(+), 150 deletions(-) create mode 100644 src/main/java/com/amazon/jenkins/ec2fleet/EC2AgentTerminationReason.java rename src/main/java/com/amazon/jenkins/ec2fleet/{EC2TerminationCause.java => EC2ExecutorInterruptionCause.java} (76%) diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/AbstractEC2FleetCloud.java b/src/main/java/com/amazon/jenkins/ec2fleet/AbstractEC2FleetCloud.java index 72298bf4..a9b9c779 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/AbstractEC2FleetCloud.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/AbstractEC2FleetCloud.java @@ -16,7 +16,7 @@ protected AbstractEC2FleetCloud(String name) { public abstract boolean hasExcessCapacity(); - public abstract boolean scheduleToTerminate(String instanceId, boolean force); + public abstract boolean scheduleToTerminate(String instanceId, boolean ignoreMinConstraints, EC2AgentTerminationReason reason); public abstract String getOldId(); diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2AgentTerminationReason.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2AgentTerminationReason.java new file mode 100644 index 00000000..aaf9fc61 --- /dev/null +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2AgentTerminationReason.java @@ -0,0 +1,35 @@ +package com.amazon.jenkins.ec2fleet; + +/** + * Enum to represent the reason for termination of an EC2 instance by the plugin. + */ +public enum EC2AgentTerminationReason { + IDLE_FOR_TOO_LONG("Agent idle for too long"), + MAX_TOTAL_USES_EXHAUSTED("MaxTotalUses exhausted for agent"), + EXCESS_CAPACITY("Excess capacity for fleet"), + AGENT_DELETED("Agent deleted"); + + private final String description; + + EC2AgentTerminationReason(String description) { + this.description = description; + } + + public String getDescription() { + return description; + } + + public static EC2AgentTerminationReason fromDescription(String desc) { + for (EC2AgentTerminationReason reason: values()) { + if(reason.description.equalsIgnoreCase(desc)) { + return reason; + } + } + return null; + } + + @Override + public String toString() { + return this.description; + } +} diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2TerminationCause.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2ExecutorInterruptionCause.java similarity index 76% rename from src/main/java/com/amazon/jenkins/ec2fleet/EC2TerminationCause.java rename to src/main/java/com/amazon/jenkins/ec2fleet/EC2ExecutorInterruptionCause.java index 93de3e72..d184d3b4 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/EC2TerminationCause.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2ExecutorInterruptionCause.java @@ -4,13 +4,13 @@ import javax.annotation.Nonnull; -public class EC2TerminationCause extends CauseOfInterruption { +public class EC2ExecutorInterruptionCause extends CauseOfInterruption { @Nonnull private final String nodeName; @SuppressWarnings("WeakerAccess") - public EC2TerminationCause(@Nonnull String nodeName) { + public EC2ExecutorInterruptionCause(@Nonnull String nodeName) { this.nodeName = nodeName; } @@ -22,7 +22,7 @@ public String getShortDescription() { @Override public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; - EC2TerminationCause that = (EC2TerminationCause) o; + EC2ExecutorInterruptionCause that = (EC2ExecutorInterruptionCause) o; return nodeName.equals(that.nodeName); } diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncher.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncher.java index ed211650..a707feb5 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncher.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncher.java @@ -94,7 +94,7 @@ public void afterDisconnect(final SlaveComputer computer, final TaskListener lis for (Executor executor : executors) { final Queue.Executable executable = executor.getCurrentExecutable(); if (executable != null) { - executor.interrupt(Result.ABORTED, new EC2TerminationCause(computer.getDisplayName())); + executor.interrupt(Result.ABORTED, new EC2ExecutorInterruptionCause(computer.getDisplayName())); final SubTask subTask = executable.getParent(); final Queue.Task task = subTask.getOwnerTask(); diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java index 69d16b58..2c518bca 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java @@ -149,7 +149,7 @@ public class EC2FleetCloud extends AbstractEC2FleetCloud { /** * {@link EC2FleetCloud#update()} updating this field, this is one thread * related to {@link CloudNanny}. At the same time {@link EC2RetentionStrategy} - * call {@link EC2FleetCloud#scheduleToTerminate(String, boolean)} to terminate instance when it is free + * call {@link EC2FleetCloud#scheduleToTerminate(String, boolean, EC2AgentTerminationReason)} to terminate instance when it is free * and uses this field to know the current capacity. *

* It could be situation that stats is outdated and plugin will make wrong decision, @@ -160,7 +160,7 @@ public class EC2FleetCloud extends AbstractEC2FleetCloud { private transient int toAdd; - private transient Set instanceIdsToTerminate; + private transient Map instanceIdsToTerminate; private transient Set plannedNodesCache; @@ -363,7 +363,7 @@ synchronized void setPlannedNodeScheduledFutures(final ArrayList getInstanceIdsToTerminate() { + synchronized Map getInstanceIdsToTerminate() { return instanceIdsToTerminate; } @@ -490,7 +490,7 @@ public FleetStateStats update() { } } final int currentToAdd; - final Set currentInstanceIdsToTerminate; + final Map currentInstanceIdsToTerminate; synchronized (this) { if(minSpareSize > 0) { // Check spare instances by considering FleetStateStats#getNumDesired so we account for newer instances which are in progress @@ -502,14 +502,14 @@ public FleetStateStats update() { } } currentToAdd = toAdd; - currentInstanceIdsToTerminate = new HashSet<>(instanceIdsToTerminate); + currentInstanceIdsToTerminate = new HashMap<>(instanceIdsToTerminate); } currentState = updateByState(currentToAdd, currentInstanceIdsToTerminate, currentState); // lock and update state of plugin, so terminate or provision could work with new state of world synchronized (this) { - instanceIdsToTerminate.removeAll(currentInstanceIdsToTerminate); + instanceIdsToTerminate.keySet().removeAll(currentInstanceIdsToTerminate.keySet()); // toAdd only grows outside of this method, so we can subtract toAdd = toAdd - currentToAdd; fine("setting stats"); @@ -551,7 +551,7 @@ public boolean removePlannedNodeScheduledFutures(final int numToRemove) { } private FleetStateStats updateByState( - final int currentToAdd, final Set currentInstanceIdsToTerminate, final FleetStateStats currentState) { + final int currentToAdd, final Map currentInstanceIdsToTerminate, final FleetStateStats currentState) { final Jenkins jenkins = Jenkins.get(); final AmazonEC2 ec2 = Registry.getEc2Api().connect(getAwsCredentialsId(), region, endpoint); @@ -577,20 +577,21 @@ private FleetStateStats updateByState( Queue.withLock(new Runnable() { @Override public void run() { - for (final String instanceId : currentInstanceIdsToTerminate) { + info("Removing Jenkins nodes before terminating corresponding EC2 instances"); + for (final String instanceId : currentInstanceIdsToTerminate.keySet()) { final Node node = jenkins.getNode(instanceId); if (node != null) { try { jenkins.removeNode(node); } catch (IOException e) { - warning("Failed to remove node '%s' from Jenkins before termination", instanceId); + warning("Failed to remove node '%s' from Jenkins before termination.", instanceId); } } } } }); - info("Terminating nodes: %s", currentInstanceIdsToTerminate); - Registry.getEc2Api().terminateInstances(ec2, currentInstanceIdsToTerminate); + Registry.getEc2Api().terminateInstances(ec2, currentInstanceIdsToTerminate.keySet()); + info("Terminated instances: %s", currentInstanceIdsToTerminate); } fine("Fleet instances: %s", updatedState.getInstances()); @@ -600,7 +601,7 @@ public void run() { final Map described = Registry.getEc2Api().describeInstances(ec2, fleetInstances); // Sometimes described includes just deleted instances - described.keySet().removeAll(currentInstanceIdsToTerminate); + described.keySet().removeAll(currentInstanceIdsToTerminate.keySet()); fine("Described instances: %s", described.keySet()); // Fleet takes a while to display terminated instances. Update stats with current view of active instance count @@ -693,8 +694,8 @@ public void run() { /** * Schedules Jenkins Node and EC2 instance for termination. - * If force is false and target capacity falls below minSize OR minSpareSize thresholds, then reject termination. - * Else if force is true, schedule instance for termination even if it breaches minSize OR minSpareSize + * If ignoreMinConstraints is false and target capacity falls below minSize OR minSpareSize thresholds, then reject termination. + * Else if ignoreMinConstraints is true, schedule instance for termination even if it breaches minSize OR minSpareSize *

* Real termination will happens in {@link EC2FleetCloud#update()} which is periodically called by * {@link CloudNanny}. So there could be some lag between the decision to terminate the node @@ -705,16 +706,18 @@ public void run() { * to AWS EC2 API which takes some time and block cloud class. * * @param instanceId node name or instance ID - * @param force terminate instance even if it breaches min size constraint + * @param ignoreMinConstraints terminate instance even if it breaches min size constraint + * @param reason reason for termination * @return true if node scheduled for termination, otherwise false */ - public synchronized boolean scheduleToTerminate(final String instanceId, final boolean force) { + public synchronized boolean scheduleToTerminate(final String instanceId, final boolean ignoreMinConstraints, + final EC2AgentTerminationReason reason) { if (stats == null) { info("First update not done, skipping termination scheduling for '%s'", instanceId); return false; } - // We can't remove instances beyond minSize or minSpareSize unless force true - if(!force) { + // We can't remove instances beyond minSize or minSpareSize unless ignoreMinConstraints true + if(!ignoreMinConstraints) { if (minSize > 0 && stats.getNumActive() - instanceIdsToTerminate.size() <= minSize) { info("Not scheduling instance '%s' for termination because we need a minimum of %s instance(s) running", instanceId, minSize); fine("cloud: %s, instanceIdsToTerminate: %s", this, instanceIdsToTerminate); @@ -729,8 +732,8 @@ public synchronized boolean scheduleToTerminate(final String instanceId, final b } } } - info("Scheduling instance '%s' for termination on cloud %s with force: %b", instanceId, this, force); - instanceIdsToTerminate.add(instanceId); + info("Scheduling instance '%s' for termination on cloud %s because of reason: %s", instanceId, this, reason); + instanceIdsToTerminate.put(instanceId, reason); fine("InstanceIdsToTerminate: %s", instanceIdsToTerminate); return true; } @@ -763,7 +766,7 @@ private void init() { id = new LazyUuid(); plannedNodesCache = new HashSet<>(); - instanceIdsToTerminate = new HashSet<>(); + instanceIdsToTerminate = new HashMap<>(); plannedNodeScheduledFutures = new ArrayList<>(); } diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud.java index 9ed21580..b11e611f 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud.java @@ -354,13 +354,13 @@ private static class State { int toAdd; final Set plannedNodes; final Set plannedNodesToRemove; - final Set instanceIdsToTerminate; + final Map instanceIdsToTerminate; public State(String fleetId) { this.fleetId = fleetId; this.plannedNodes = new HashSet<>(); this.plannedNodesToRemove = new HashSet<>(); - this.instanceIdsToTerminate = new HashSet<>(); + this.instanceIdsToTerminate = new HashMap<>(); } public State(State state) { @@ -370,7 +370,7 @@ public State(State state) { this.targetCapacity = state.targetCapacity; this.toAdd = state.toAdd; this.plannedNodesToRemove = new HashSet<>(state.plannedNodesToRemove); - this.instanceIdsToTerminate = new HashSet<>(state.instanceIdsToTerminate); + this.instanceIdsToTerminate = new HashMap<>(state.instanceIdsToTerminate); } } @@ -405,7 +405,7 @@ public void update() { final State state = states.get(entry.getKey()); state.stats = entry.getValue().stats; - state.instanceIdsToTerminate.removeAll(entry.getValue().instanceIdsToTerminate); + state.instanceIdsToTerminate.keySet().removeAll(entry.getValue().instanceIdsToTerminate.keySet()); // toAdd only grow outside of this method, so we can subtract state.toAdd = state.toAdd - entry.getValue().toAdd; // remove released planned nodes @@ -438,9 +438,9 @@ private void updateByState(final Map states) { } } - final List instanceIdsToRemove = new ArrayList<>(); + final Map instanceIdsToRemove = new HashMap<>(); for (State state : states.values()) { - instanceIdsToRemove.addAll(state.instanceIdsToTerminate); + instanceIdsToRemove.putAll(state.instanceIdsToTerminate); } if (instanceIdsToRemove.size() > 0) { @@ -450,13 +450,14 @@ private void updateByState(final Map states) { Queue.withLock(new Runnable() { @Override public void run() { - for (final String instanceId : instanceIdsToRemove) { + info("Removing Jenkins nodes before terminating corresponding EC2 instances"); + for (final String instanceId : instanceIdsToRemove.keySet()) { final Node node = jenkins.getNode(instanceId); if (node != null) { try { jenkins.removeNode(node); } catch (IOException e) { - warning("unable remove node %s from Jenkins, skip, just terminate EC2 instance", instanceId); + warning("unable to remove node %s from Jenkins, skip, just terminate EC2 instance", instanceId); } } } @@ -464,7 +465,7 @@ public void run() { }); info("Delete terminating nodes from Jenkins %s", instanceIdsToRemove); - Registry.getEc2Api().terminateInstances(ec2, instanceIdsToRemove); + Registry.getEc2Api().terminateInstances(ec2, instanceIdsToRemove.keySet()); info("Instances %s were terminated with result", instanceIdsToRemove); } @@ -562,7 +563,9 @@ public void run() { } } - public synchronized boolean scheduleToTerminate(final String instanceId, boolean force) { + @Override + public synchronized boolean scheduleToTerminate(final String instanceId, final boolean ignoreMinConstraints, + final EC2AgentTerminationReason terminationReason) { info("Attempting to terminate instance: %s", instanceId); final Node node = Jenkins.get().getNode(instanceId); @@ -574,19 +577,19 @@ public synchronized boolean scheduleToTerminate(final String instanceId, boolean return false; } - // We can't remove instances beyond minSize unless force is true + // We can't remove instances beyond minSize unless ignoreMinConstraints is true final EC2FleetLabelParameters parameters = new EC2FleetLabelParameters(node.getLabelString()); final int minSize = parameters.getIntOrDefault("minSize", this.minSize); - if (!force && (minSize > 0 && state.stats.getNumDesired() - state.instanceIdsToTerminate.size() <= minSize)) { + if (!ignoreMinConstraints && (minSize > 0 && state.stats.getNumDesired() - state.instanceIdsToTerminate.size() <= minSize)) { info("Not terminating %s because we need a minimum of %s instances running.", instanceId, minSize); return false; } - info("Scheduling instance '%s' for termination on cloud %s with force: %b", instanceId, this, force); - state.instanceIdsToTerminate.add(instanceId); + info("Scheduling instance '%s' for termination on cloud %s because of reason: %s", instanceId, this, terminationReason); + state.instanceIdsToTerminate.put(instanceId, terminationReason); return true; } - // sync as we are using modifyable state + // sync as we are using modifiable state @Override public synchronized boolean canProvision(final Cloud.CloudState cloudState) { final Label label = cloudState.getLabel(); diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetNodeComputer.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetNodeComputer.java index 344acb61..0c1a49b5 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetNodeComputer.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetNodeComputer.java @@ -81,7 +81,7 @@ public HttpResponse doDoDelete() throws IOException { final String instanceId = node.getNodeName(); final AbstractEC2FleetCloud cloud = node.getCloud(); if (cloud != null && StringUtils.isNotBlank(instanceId)) { - cloud.scheduleToTerminate(instanceId, false); + cloud.scheduleToTerminate(instanceId, false, EC2AgentTerminationReason.AGENT_DELETED); } } return super.doDoDelete(); diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2RetentionStrategy.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2RetentionStrategy.java index fc555b43..587bfcd1 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/EC2RetentionStrategy.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2RetentionStrategy.java @@ -53,7 +53,16 @@ public long check(final SlaveComputer computer) { boolean justTerminated = false; fc.setAcceptingTasks(false); try { - if(fc.isIdle() && (cloud.hasExcessCapacity() || isIdleForTooLong(cloud, fc))) { + if(fc.isIdle()) { + final EC2AgentTerminationReason reason; + if (isIdleForTooLong(cloud, fc)) { + reason = EC2AgentTerminationReason.IDLE_FOR_TOO_LONG; + } else if (cloud.hasExcessCapacity()) { + reason = EC2AgentTerminationReason.EXCESS_CAPACITY; + } else { + return 0; + } + // Find instance ID Node compNode = fc.getNode(); if (compNode == null) { @@ -61,11 +70,11 @@ public long check(final SlaveComputer computer) { } final String instanceId = compNode.getNodeName(); - if (cloud.scheduleToTerminate(instanceId, false)) { + if (cloud.scheduleToTerminate(instanceId, false, reason)) { // Instance successfully scheduled for termination, so no longer accept tasks (i.e. suspended) shouldAcceptTasks = false; - LOGGER.fine(String.format("Suspended node %s after scheduling instance for termination.", - compNode.getDisplayName(), instanceId)); + LOGGER.fine(String.format("Suspended node %s after scheduling instance for termination, reason: %s.", + compNode.getDisplayName(), instanceId, reason)); justTerminated = true; } } @@ -117,7 +126,6 @@ public void taskAccepted(Executor executor, Queue.Task task) { } else if (maxTotalUses <= 1) { LOGGER.info("maxTotalUses drained - suspending agent after current build " + computer.getName()); computer.setAcceptingTasks(false); - ec2FleetNode.setMaxTotalUses(ec2FleetNode.getMaxTotalUses() - 1); } else { ec2FleetNode.setMaxTotalUses(ec2FleetNode.getMaxTotalUses() - 1); LOGGER.info("Agent " + computer.getName() + " has " + ec2FleetNode.getMaxTotalUses() + " builds left"); @@ -144,13 +152,8 @@ private void postJobAction(Executor executor) { final AbstractEC2FleetCloud cloud = ec2FleetNode.getCloud(); if (computer.countBusy() <= 1 && !computer.isAcceptingTasks()) { LOGGER.info("Calling scheduleToTerminate for node " + ec2FleetNode.getNodeName() + " due to maxTotalUses (" + ec2FleetNode.getMaxTotalUses() + ")"); - computer.setAcceptingTasks(false); - // Schedule instance for termination even if it breaches min size constraint - cloud.scheduleToTerminate(ec2FleetNode.getNodeName(), true); - } else { - if (ec2FleetNode.getMaxTotalUses() == 1) { - LOGGER.info("Agent " + ec2FleetNode.getNodeName() + " is still in use by more than one (" + computer.countBusy() + ") executors."); - } + // Schedule instance for termination even if it breaches minSize and minSpareSize constraints + cloud.scheduleToTerminate(ec2FleetNode.getNodeName(), true, EC2AgentTerminationReason.MAX_TOTAL_USES_EXHAUSTED); } } } diff --git a/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/help-maxTotalUses.html b/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/help-maxTotalUses.html index 83102f23..f5b3dabb 100644 --- a/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/help-maxTotalUses.html +++ b/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/help-maxTotalUses.html @@ -1,6 +1,7 @@

Set a maximum total uses allowed for instances. After running 'maximum total uses' amount of jobs, the instance would be terminated. + This setting overrides minSize and minSpareSize, if set. Use '-1' for unlimited uses.

diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncherTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncherTest.java index 713cf7a8..bc3250cf 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncherTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncherTest.java @@ -9,7 +9,6 @@ import hudson.model.TaskListener; import hudson.model.queue.SubTask; import hudson.slaves.ComputerLauncher; -import hudson.slaves.OfflineCause; import jenkins.model.Jenkins; import org.junit.Before; import org.junit.Test; @@ -163,8 +162,8 @@ public void taskCompleted_should_resubmit_task_for_all_executors() { public void taskCompleted_should_abort_executors_during_resubmit() { new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher) .afterDisconnect(computer, taskListener); - verify(executor1).interrupt(Result.ABORTED, new EC2TerminationCause("i-12")); - verify(executor2).interrupt(Result.ABORTED, new EC2TerminationCause("i-12")); + verify(executor1).interrupt(Result.ABORTED, new EC2ExecutorInterruptionCause("i-12")); + verify(executor2).interrupt(Result.ABORTED, new EC2ExecutorInterruptionCause("i-12")); } @Test diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudTest.java index 0b4beec1..17dfda48 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudTest.java @@ -427,7 +427,7 @@ public void scheduleToTerminate_shouldNotRemoveIfStatsNotUpdated() { 10, false); // when - boolean r = fleetCloud.scheduleToTerminate("z", false); + boolean r = fleetCloud.scheduleToTerminate("z", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); // then assertFalse(r); @@ -452,7 +452,7 @@ public void scheduleToTerminate_notRemoveIfBelowMin() { Collections.emptySet(), Collections.emptyMap())); // when - boolean r = fleetCloud.scheduleToTerminate("z", false); + boolean r = fleetCloud.scheduleToTerminate("z", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); // then assertFalse(r); @@ -477,7 +477,7 @@ public void scheduleToTerminate_notRemoveIfEqualMin() { Collections.singleton("z"), Collections.emptyMap())); // when - boolean r = fleetCloud.scheduleToTerminate("z", false); + boolean r = fleetCloud.scheduleToTerminate("z", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); // then assertFalse(r); @@ -503,7 +503,7 @@ public void scheduleToTerminate_notRemoveIfEqualMinSpare() { Collections.singleton("z"), Collections.emptyMap())); // when - boolean r = fleetCloud.scheduleToTerminate("z", false); + boolean r = fleetCloud.scheduleToTerminate("z", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); // then assertFalse(r); @@ -528,11 +528,11 @@ public void scheduleToTerminate_remove() { new HashSet<>(Arrays.asList("z", "z1")), Collections.emptyMap())); // when - boolean r = fleetCloud.scheduleToTerminate("z", false); + boolean r = fleetCloud.scheduleToTerminate("z", false, EC2AgentTerminationReason.MAX_TOTAL_USES_EXHAUSTED); // then assertTrue(r); - assertEquals(Collections.singleton("z"), fleetCloud.getInstanceIdsToTerminate()); + assertEquals(Collections.singletonMap("z", EC2AgentTerminationReason.MAX_TOTAL_USES_EXHAUSTED), fleetCloud.getInstanceIdsToTerminate()); } @Test @@ -554,13 +554,16 @@ public void scheduleToTerminate_upToZeroNodes() { new HashSet<>(Arrays.asList("z-1", "z-2")), Collections.emptyMap())); // when - boolean r1 = fleetCloud.scheduleToTerminate("z-1", false); - boolean r2 = fleetCloud.scheduleToTerminate("z-2", false); + boolean r1 = fleetCloud.scheduleToTerminate("z-1", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); + boolean r2 = fleetCloud.scheduleToTerminate("z-2", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); // then assertTrue(r1); assertTrue(r2); - assertEquals(new HashSet<>(Arrays.asList("z-1", "z-2")), fleetCloud.getInstanceIdsToTerminate()); + assertEquals(new HashMap(){{ + put("z-1", EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); + put("z-2", EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); + }}, fleetCloud.getInstanceIdsToTerminate()); } @Test @@ -582,15 +585,18 @@ public void scheduleToTerminate_removeNoMoreMinIfCalledMultipleBeforeUpdate() { new HashSet<>(Arrays.asList("z1", "z2", "z3")), Collections.emptyMap())); // when - boolean r1 = fleetCloud.scheduleToTerminate("z1", false); - boolean r2 = fleetCloud.scheduleToTerminate("z2", false); - boolean r3 = fleetCloud.scheduleToTerminate("z3", false); + boolean r1 = fleetCloud.scheduleToTerminate("z1", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); + boolean r2 = fleetCloud.scheduleToTerminate("z2", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); + boolean r3 = fleetCloud.scheduleToTerminate("z3", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); // then assertTrue(r1); assertTrue(r2); assertFalse(r3); - assertEquals(new HashSet<>(Arrays.asList("z1", "z2")), fleetCloud.getInstanceIdsToTerminate()); + assertEquals(new HashMap(){{ + put("z1", EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); + put("z2", EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); + }}, fleetCloud.getInstanceIdsToTerminate()); } @Test @@ -663,7 +669,7 @@ public void update_shouldResetTerminateAndProvision() { fleetCloud.setStats(currentState); fleetCloud.provision(new Cloud.CloudState(null, 0), 2); - fleetCloud.scheduleToTerminate("i-1", false); + fleetCloud.scheduleToTerminate("i-1", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); // when fleetCloud.update(); @@ -693,7 +699,7 @@ public void update_shouldNotIncreaseMoreThenMax() { Collections.emptySet(), Collections.emptyMap())); for (int i = 0; i < 10; i++) fleetCloud.provision(new Cloud.CloudState(null, 0), 1); - for (int i = 0; i < 10; i++) fleetCloud.scheduleToTerminate("i-" + i, false); + for (int i = 0; i < 10; i++) fleetCloud.scheduleToTerminate("i-" + i, false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); for (int i = 0; i < 10; i++) fleetCloud.provision(new Cloud.CloudState(null, 0), 1); // when @@ -724,7 +730,7 @@ public void update_shouldNotCountScheduledToTerminateWhenScaleUp() { Collections.emptySet(), Collections.emptyMap())); for (int i = 0; i < 10; i++) fleetCloud.provision(new Cloud.CloudState(null, 0), 1); - for (int i = 0; i < 5; i++) fleetCloud.scheduleToTerminate("i-" + i, false); + for (int i = 0; i < 5; i++) fleetCloud.scheduleToTerminate("i-" + i, false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); // when fleetCloud.update(); @@ -753,8 +759,8 @@ public void update_shouldDecreaseTargetCapacityAndTerminateInstancesIfScheduled( fleetCloud.setStats(new FleetStateStats("", 4, FleetStateStats.State.active(), Collections.emptySet(), Collections.emptyMap())); - fleetCloud.scheduleToTerminate("i-1", false); - fleetCloud.scheduleToTerminate("i-2", false); + fleetCloud.scheduleToTerminate("i-1", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); + fleetCloud.scheduleToTerminate("i-2", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); // when fleetCloud.update(); @@ -1029,8 +1035,8 @@ public void update_givenManuallyUpdatedFleetShouldCorrectLocalTargetCapacityToKe doNothing().when(jenkins).addNode(any(Node.class)); - fleetCloud.scheduleToTerminate("i-0", false); - fleetCloud.scheduleToTerminate("i-1", false); + fleetCloud.scheduleToTerminate("i-0", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); + fleetCloud.scheduleToTerminate("i-1", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); // when fleetCloud.update(); @@ -1212,7 +1218,7 @@ public void update_shouldUpdateStateWithFleetTargetCapacityMinusToTerminate() th doNothing().when(jenkins).addNode(any(Node.class)); - fleetCloud.scheduleToTerminate("i-0", false); + fleetCloud.scheduleToTerminate("i-0", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); // when fleetCloud.update(); diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudWithMeter.java b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudWithMeter.java index 91762880..17353580 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudWithMeter.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudWithMeter.java @@ -42,9 +42,9 @@ public FleetStateStats update() { } @Override - public boolean scheduleToTerminate(final String instanceId, boolean force) { + public boolean scheduleToTerminate(final String instanceId, boolean ignoreMinConstraints, EC2AgentTerminationReason reason) { try (Meter.Shot s = removeMeter.start()) { - return super.scheduleToTerminate(instanceId, force); + return super.scheduleToTerminate(instanceId, ignoreMinConstraints, reason); } } diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/EC2MaxTotalUsesRetentionStrategyTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/EC2MaxTotalUsesRetentionStrategyTest.java index 3217fe1d..9fd57b98 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/EC2MaxTotalUsesRetentionStrategyTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/EC2MaxTotalUsesRetentionStrategyTest.java @@ -39,12 +39,12 @@ public void shouldNotTerminateWhenUsageIsGreaterThanOne() throws Exception { rs.taskCompleted(executor, null, 0); } if (usageCount == 1) { - verify(cloud, times(1)).scheduleToTerminate("name", true); + verify(cloud, times(1)).scheduleToTerminate("name", true, EC2AgentTerminationReason.MAX_TOTAL_USES_EXHAUSTED); } else if (usageCount == 0) { // We would have called terminate twice: 0 & 1 - verify(cloud, times(2)).scheduleToTerminate("name", true); + verify(cloud, times(2)).scheduleToTerminate("name", true, EC2AgentTerminationReason.MAX_TOTAL_USES_EXHAUSTED); } else { - verify(cloud, times(0)).scheduleToTerminate("name", true); + verify(cloud, times(0)).scheduleToTerminate("name", true, EC2AgentTerminationReason.MAX_TOTAL_USES_EXHAUSTED); } usageCount--; } @@ -68,6 +68,6 @@ public void shouldNotTerminateWhenUsageIsSetUnlimited() throws Exception { if (!computer.isAcceptingTasks()) { rs.taskCompleted(executor, null, 0); } - verify(cloud, times(0)).scheduleToTerminate("name", true); + verify(cloud, times(0)).scheduleToTerminate("name", true, EC2AgentTerminationReason.MAX_TOTAL_USES_EXHAUSTED); } } diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/EC2RetentionStrategyIntegrationTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/EC2RetentionStrategyIntegrationTest.java index 2b4fab3d..99cb8963 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/EC2RetentionStrategyIntegrationTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/EC2RetentionStrategyIntegrationTest.java @@ -11,13 +11,18 @@ import com.amazonaws.services.ec2.model.Reservation; import com.amazonaws.services.ec2.model.TerminateInstancesRequest; import com.amazonaws.services.ec2.model.TerminateInstancesResult; +import hudson.model.Executor; import hudson.model.Node; +import hudson.model.Queue; import hudson.model.queue.QueueTaskFuture; +import hudson.security.AccessControlled; +import jenkins.model.Jenkins; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/EC2RetentionStrategyTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/EC2RetentionStrategyTest.java index a30af6aa..454b1ce5 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/EC2RetentionStrategyTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/EC2RetentionStrategyTest.java @@ -1,5 +1,7 @@ package com.amazon.jenkins.ec2fleet; +import hudson.model.Executor; +import hudson.model.Queue; import hudson.slaves.SlaveComputer; import org.junit.Before; import org.junit.Test; @@ -13,8 +15,11 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -28,153 +33,233 @@ public class EC2RetentionStrategyTest { private EC2FleetCloud cloud; @Mock - private EC2FleetNodeComputer slaveComputer; + private EC2FleetNodeComputer computer; @Mock - private EC2FleetNode slave; + private EC2FleetNode node; + + @Mock + private Queue.Task task; + + @Mock + private Executor executor; @Before public void before() { when(cloud.getIdleMinutes()).thenReturn(10); - PowerMockito.when(slaveComputer.getIdleStartMilliseconds()).thenReturn(System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(11)); - when(slaveComputer.getNode()).thenReturn(slave); - PowerMockito.when(slaveComputer.isIdle()).thenReturn(true); - when(slave.getNodeName()).thenReturn("n-a"); - when(slaveComputer.isAcceptingTasks()).thenReturn(true); - when(slaveComputer.getCloud()).thenReturn(cloud); + PowerMockito.when(computer.getIdleStartMilliseconds()).thenReturn(System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(11)); + when(computer.getNode()).thenReturn(node); + PowerMockito.when(computer.isIdle()).thenReturn(true); + when(node.getNodeName()).thenReturn("n-a"); + when(computer.isAcceptingTasks()).thenReturn(true); + when(computer.getCloud()).thenReturn(cloud); + when(executor.getOwner()).thenReturn(computer); } @Test public void if_idle_time_not_configured_should_do_nothing() { when(cloud.getIdleMinutes()).thenReturn(0); - new EC2RetentionStrategy().check(slaveComputer); + new EC2RetentionStrategy().check(computer); - verify(slaveComputer, times(0)).getNode(); - verify(cloud, times(0)).scheduleToTerminate(anyString(), anyBoolean()); - verify(slaveComputer).setAcceptingTasks(false); - verify(slaveComputer).setAcceptingTasks(true); + verify(computer, times(0)).getNode(); + verify(cloud, times(0)).scheduleToTerminate(anyString(), anyBoolean(), eq(EC2AgentTerminationReason.IDLE_FOR_TOO_LONG)); + verify(computer).setAcceptingTasks(false); + verify(computer).setAcceptingTasks(true); } @Test public void shouldScheduleExcessCapacityForTerminationIfIdle() { + when(computer.isIdle()).thenReturn(Boolean.TRUE); when(cloud.hasExcessCapacity()).thenReturn(Boolean.TRUE); - when(slaveComputer.isIdle()).thenReturn(Boolean.TRUE); + when(cloud.getIdleMinutes()).thenReturn(20); - new EC2RetentionStrategy().check(slaveComputer); + new EC2RetentionStrategy().check(computer); - verify(slaveComputer, times(1)).getNode(); - verify(cloud, times(1)).scheduleToTerminate(anyString(), anyBoolean()); - verify(slaveComputer).setAcceptingTasks(false); + verify(computer, times(1)).getNode(); + verify(cloud, times(1)).scheduleToTerminate(anyString(), anyBoolean(), eq(EC2AgentTerminationReason.EXCESS_CAPACITY)); + verify(computer).setAcceptingTasks(false); } @Test public void shouldNotScheduleExcessCapacityForTerminationIfBusy() { when(cloud.hasExcessCapacity()).thenReturn(Boolean.TRUE); - when(slaveComputer.isIdle()).thenReturn(Boolean.FALSE); + when(computer.isIdle()).thenReturn(Boolean.FALSE); - new EC2RetentionStrategy().check(slaveComputer); + new EC2RetentionStrategy().check(computer); - verify(slaveComputer, times(0)).getNode(); - verify(cloud, times(0)).scheduleToTerminate(anyString(), anyBoolean()); - verify(slaveComputer).setAcceptingTasks(true); + verify(computer, times(0)).getNode(); + verify(cloud, times(0)).scheduleToTerminate(anyString(), anyBoolean(), any(EC2AgentTerminationReason.class)); + verify(computer).setAcceptingTasks(true); } @Test public void shouldNotScheduleTerminationIfNotExcessCapacity() { when(cloud.hasExcessCapacity()).thenReturn(Boolean.FALSE); - when(slaveComputer.isIdle()).thenReturn(Boolean.TRUE); + when(computer.isIdle()).thenReturn(Boolean.TRUE); when(cloud.getIdleMinutes()).thenReturn(0); - new EC2RetentionStrategy().check(slaveComputer); + new EC2RetentionStrategy().check(computer); + + verify(computer, times(0)).getNode(); + verify(cloud, times(0)).scheduleToTerminate(anyString(), anyBoolean(), any(EC2AgentTerminationReason.class)); + verify(computer).setAcceptingTasks(true); + } + + @Test + public void should_do_nothing_if_node_is_null() { + when(cloud.hasExcessCapacity()).thenReturn(Boolean.FALSE); + when(computer.isIdle()).thenReturn(Boolean.TRUE); + when(cloud.getIdleMinutes()).thenReturn(1); + + when(computer.getNode()).thenReturn(null); - verify(slaveComputer, times(0)).getNode(); - verify(cloud, times(0)).scheduleToTerminate(anyString(), anyBoolean()); - verify(slaveComputer).setAcceptingTasks(true); + new EC2RetentionStrategy().check(computer); + + verify(computer, times(1)).getNode(); + verify(cloud, never()).scheduleToTerminate(anyString(), anyBoolean(), any(EC2AgentTerminationReason.class)); } @Test public void if_idle_time_configured_should_do_nothing_if_node_idle_less_time() { - when(slaveComputer.getIdleStartMilliseconds()).thenReturn(System.currentTimeMillis()); + when(computer.getIdleStartMilliseconds()).thenReturn(System.currentTimeMillis()); - new EC2RetentionStrategy().check(slaveComputer); + new EC2RetentionStrategy().check(computer); - verify(slaveComputer, never()).getNode(); - verify(cloud, never()).scheduleToTerminate(anyString(), anyBoolean()); - verify(slaveComputer).setAcceptingTasks(false); - verify(slaveComputer).setAcceptingTasks(true); + verify(computer, never()).getNode(); + verify(cloud, never()).scheduleToTerminate(anyString(), anyBoolean(), any(EC2AgentTerminationReason.class)); + verify(computer).setAcceptingTasks(false); + verify(computer).setAcceptingTasks(true); } @Test public void if_node_not_execute_anything_yet_idle_time_negative_do_nothing() { - when(slaveComputer.getIdleStartMilliseconds()).thenReturn(Long.MIN_VALUE); + when(computer.getIdleStartMilliseconds()).thenReturn(Long.MIN_VALUE); - new EC2RetentionStrategy().check(slaveComputer); + new EC2RetentionStrategy().check(computer); - verify(slaveComputer, times(0)).getNode(); - verify(cloud, times(0)).scheduleToTerminate(anyString(), anyBoolean()); - verify(slaveComputer).setAcceptingTasks(false); - verify(slaveComputer).setAcceptingTasks(true); + verify(computer, times(0)).getNode(); + verify(cloud, times(0)).scheduleToTerminate(anyString(), anyBoolean(), any(EC2AgentTerminationReason.class)); + verify(computer).setAcceptingTasks(false); + verify(computer).setAcceptingTasks(true); } @Test public void if_idle_time_configured_should_terminate_node_if_idle_time_more_then_allowed() { - new EC2RetentionStrategy().check(slaveComputer); + new EC2RetentionStrategy().check(computer); - verify(cloud, times(1)).scheduleToTerminate("n-a", false); - verify(slaveComputer, times(1)).setAcceptingTasks(true); - verify(slaveComputer, times(1)).setAcceptingTasks(false); + verify(cloud, times(1)).scheduleToTerminate("n-a", false, EC2AgentTerminationReason.IDLE_FOR_TOO_LONG); + verify(computer, times(1)).setAcceptingTasks(true); + verify(computer, times(1)).setAcceptingTasks(false); } @Test public void if_computer_has_no_cloud_should_do_nothing() { - when(slaveComputer.getCloud()).thenReturn(null); + when(computer.getCloud()).thenReturn(null); - new EC2RetentionStrategy().check(slaveComputer); + new EC2RetentionStrategy().check(computer); - verify(cloud, times(0)).scheduleToTerminate(anyString(), anyBoolean()); - verify(slaveComputer, times(0)).setAcceptingTasks(true); - verify(slaveComputer, times(0)).setAcceptingTasks(false); + verify(cloud, times(0)).scheduleToTerminate(anyString(), anyBoolean(), any(EC2AgentTerminationReason.class)); + verify(computer, times(0)).setAcceptingTasks(true); + verify(computer, times(0)).setAcceptingTasks(false); } @Test public void if_node_not_idle_should_do_nothing() { - when(slaveComputer.getIdleStartMilliseconds()).thenReturn(0L); - when(slaveComputer.isIdle()).thenReturn(false); + when(computer.getIdleStartMilliseconds()).thenReturn(0L); + when(computer.isIdle()).thenReturn(false); - new EC2RetentionStrategy().check(slaveComputer); + new EC2RetentionStrategy().check(computer); - verify(cloud, never()).scheduleToTerminate("n-a", false); - verify(slaveComputer, times(1)).setAcceptingTasks(true); - verify(slaveComputer, times(1)).setAcceptingTasks(false); + verify(cloud, never()).scheduleToTerminate(eq("n-a"), eq(false), any(EC2AgentTerminationReason.class)); + verify(computer, times(1)).setAcceptingTasks(true); + verify(computer, times(1)).setAcceptingTasks(false); } @Test public void if_node_idle_time_more_them_allowed_but_not_idle_should_do_nothing() { - when(slaveComputer.isIdle()).thenReturn(false); + when(computer.isIdle()).thenReturn(false); - new EC2RetentionStrategy().check(slaveComputer); + new EC2RetentionStrategy().check(computer); - verify(cloud, never()).scheduleToTerminate("n-a", false); - verify(slaveComputer, times(1)).setAcceptingTasks(true); - verify(slaveComputer, times(1)).setAcceptingTasks(false); + verify(cloud, never()).scheduleToTerminate(eq("n-a"), eq(false), any(EC2AgentTerminationReason.class)); + verify(computer, times(1)).setAcceptingTasks(true); + verify(computer, times(1)).setAcceptingTasks(false); } @Test public void if_exception_happen_during_termination_should_throw_it_and_restore_task_accepting() { - when(cloud.scheduleToTerminate(anyString(), anyBoolean())).thenThrow(new IllegalArgumentException("test")); + when(cloud.scheduleToTerminate(anyString(), anyBoolean(), any(EC2AgentTerminationReason.class))).thenThrow(new IllegalArgumentException("test")); try { - new EC2RetentionStrategy().check(slaveComputer); + new EC2RetentionStrategy().check(computer); fail(); } catch (IllegalArgumentException e) { assertEquals("test", e.getMessage()); - verify(cloud, times(1)).scheduleToTerminate("n-a", false); - verify(slaveComputer).setAcceptingTasks(false); - verify(slaveComputer).setAcceptingTasks(true); + verify(cloud, times(1)).scheduleToTerminate(eq("n-a"), eq(false), any(EC2AgentTerminationReason.class)); + verify(computer).setAcceptingTasks(false); + verify(computer).setAcceptingTasks(true); } } - // todo we do nothing if computer doesn't have node + @Test + public void when_unlimited_maxTotalUses_should_not_decrement() { + when(node.getMaxTotalUses()).thenReturn(-1); + + new EC2RetentionStrategy().taskAccepted(executor, task); + + verify(computer, never()).setAcceptingTasks(false); + } + + @Test + public void when_maxTotalUses_greater_than_1_should_decrement() { + when(node.getMaxTotalUses()).thenReturn(2); + + new EC2RetentionStrategy().taskAccepted(executor, task); + + verify(node).setMaxTotalUses(1); + verify(computer, never()).setAcceptingTasks(false); + } + @Test + public void when_maxTotalUses_is_1_should_stop_accepting_tasks_and_not_decrement() { + when(node.getMaxTotalUses()).thenReturn(1); + + new EC2RetentionStrategy().taskAccepted(executor, task); + + verify(computer, times(1)).setAcceptingTasks(false); + verify(node, never()).setMaxTotalUses(anyInt()); + } + + @Test + public void when_otherBusyExecutors_should_not_scheduleToTerminate() { + when(node.getCloud()).thenReturn(cloud); + when(computer.countBusy()).thenReturn(2); + + new EC2RetentionStrategy().taskCompleted(executor, task, 0L); + + verify(cloud, never()).scheduleToTerminate(anyString(), anyBoolean(), any(EC2AgentTerminationReason.class)); + } + + @Test + public void when_noOtherBusyExecutors_and_accepting_tasks_should_not_scheduleToTerminate() { + when(node.getCloud()).thenReturn(cloud); + when(computer.countBusy()).thenReturn(1); + when(computer.isAcceptingTasks()).thenReturn(true); + + new EC2RetentionStrategy().taskCompleted(executor, task, 0L); + + verify(cloud, never()).scheduleToTerminate(anyString(), anyBoolean(), any(EC2AgentTerminationReason.class)); + } + + @Test + public void when_noOtherBusyExecutors_and_not_accepting_tasks_should_scheduleToTerminate() { + when(node.getCloud()).thenReturn(cloud); + when(computer.countBusy()).thenReturn(1); + when(computer.isAcceptingTasks()).thenReturn(false); + + new EC2RetentionStrategy().taskCompleted(executor, task, 0L); + + verify(cloud, times(1)).scheduleToTerminate("n-a", true, EC2AgentTerminationReason.MAX_TOTAL_USES_EXHAUSTED); + } }