From 76325ed073be1c335a370de329d1e1a05332a6a4 Mon Sep 17 00:00:00 2001 From: Artem Stasiuk Date: Thu, 11 Jul 2019 09:10:07 -0700 Subject: [PATCH] Fix #111 (#119) * Fix #111 --- .../amazon/jenkins/ec2fleet/CloudNanny.java | 2 +- .../EC2FleetAutoResubmitComputerLauncher.java | 22 +++- .../jenkins/ec2fleet/EC2FleetCloud.java | 41 +++++- .../jenkins/ec2fleet/EC2FleetCloudAware.java | 23 ++++ .../ec2fleet/EC2FleetCloudAwareUtils.java | 40 ++++++ .../amazon/jenkins/ec2fleet/EC2FleetNode.java | 27 +++- .../ec2fleet/EC2FleetNodeComputer.java | 35 +++-- .../jenkins/ec2fleet/EC2FleetStatusInfo.java | 2 +- .../ec2fleet/IdleRetentionStrategy.java | 54 ++++---- .../com/amazon/jenkins/ec2fleet/LazyUuid.java | 22 ++++ .../ec2fleet/EC2FleetCloud/config.jelly | 7 +- .../ec2fleet/AutoResubmitIntegrationTest.java | 6 +- ...FleetAutoResubmitComputerLauncherTest.java | 32 +++-- .../ec2fleet/EC2FleetCloudAwareUtilsTest.java | 120 ++++++++++++++++++ .../jenkins/ec2fleet/EC2FleetCloudTest.java | 32 ++--- .../ec2fleet/EC2FleetNodeComputerTest.java | 23 ++-- .../ec2fleet/IdleRetentionStrategyTest.java | 31 +++-- .../amazon/jenkins/ec2fleet/LazyUuidTest.java | 72 +++++++++++ .../ec2fleet/ProvisionIntegrationTest.java | 12 +- .../ec2fleet/RealEc2ApiIntegrationTest.java | 8 +- .../jenkins/ec2fleet/UiIntegrationTest.java | 70 ++++++++-- 21 files changed, 555 insertions(+), 126 deletions(-) create mode 100644 src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudAware.java create mode 100644 src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudAwareUtils.java create mode 100644 src/main/java/com/amazon/jenkins/ec2fleet/LazyUuid.java create mode 100644 src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudAwareUtilsTest.java create mode 100644 src/test/java/com/amazon/jenkins/ec2fleet/LazyUuidTest.java diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/CloudNanny.java b/src/main/java/com/amazon/jenkins/ec2fleet/CloudNanny.java index 830b9e7f..61f5e139 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/CloudNanny.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/CloudNanny.java @@ -30,7 +30,7 @@ public long getRecurrencePeriod() { } @Override - protected void doRun() throws Exception { + protected void doRun() { final List info = new ArrayList<>(); for (final Cloud cloud : getClouds()) { if (!(cloud instanceof EC2FleetCloud)) continue; diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncher.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncher.java index 298e2614..9bc7ffad 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncher.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncher.java @@ -29,6 +29,7 @@ * @see EC2FleetNode * @see EC2FleetNodeComputer */ +@SuppressWarnings("WeakerAccess") @ThreadSafe public class EC2FleetAutoResubmitComputerLauncher extends DelegatingComputerLauncher { @@ -41,12 +42,8 @@ public class EC2FleetAutoResubmitComputerLauncher extends DelegatingComputerLaun */ private static final int RESCHEDULE_QUIET_PERIOD_SEC = 10; - private final boolean disableTaskResubmit; - - protected EC2FleetAutoResubmitComputerLauncher( - final ComputerLauncher launcher, final boolean disableTaskResubmit) { + public EC2FleetAutoResubmitComputerLauncher(final ComputerLauncher launcher) { super(launcher); - this.disableTaskResubmit = disableTaskResubmit; } /** @@ -77,8 +74,19 @@ protected EC2FleetAutoResubmitComputerLauncher( */ @Override public void afterDisconnect(final SlaveComputer computer, final TaskListener listener) { + // according to jenkins docs could be null in edge cases, check ComputerLauncher.afterDisconnect + if (computer == null) return; + + // in some multi-thread edge cases cloud could be null for some time, just be ok with that + final EC2FleetCloud cloud = ((EC2FleetNodeComputer) computer).getCloud(); + if (cloud == null) { + LOGGER.warning("Edge case cloud is null for computer " + computer.getDisplayName() + + " should be autofixed in a few minutes, if no please create issue for plugin"); + return; + } + final boolean unexpectedDisconnect = computer.isOffline() && computer.getOfflineCause() instanceof OfflineCause.ChannelTermination; - if (!disableTaskResubmit && unexpectedDisconnect) { + if (!cloud.isDisableTaskResubmit() && unexpectedDisconnect) { final List executors = computer.getExecutors(); LOGGER.log(LOG_LEVEL, "Unexpected " + computer.getDisplayName() + " termination, resubmit"); @@ -109,7 +117,7 @@ public void afterDisconnect(final SlaveComputer computer, final TaskListener lis } else { LOGGER.log(LOG_LEVEL, "Unexpected " + computer.getDisplayName() + " termination but resubmit disabled, no actions, disableTaskResubmit: " - + disableTaskResubmit + ", offline: " + computer.isOffline() + + cloud.isDisableTaskResubmit() + ", offline: " + computer.isOffline() + ", offlineCause: " + (computer.getOfflineCause() != null ? computer.getOfflineCause().getClass() : "null")); } diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java index 4fbf3b45..95b9a48a 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java @@ -69,6 +69,22 @@ public class EC2FleetCloud extends Cloud { private static final SimpleFormatter sf = new SimpleFormatter(); private static final Logger LOGGER = Logger.getLogger(EC2FleetCloud.class.getName()); + /** + * Provide unique identifier for this instance of {@link EC2FleetCloud}, transient + * will not be stored. Not available for customer, instead use {@link EC2FleetCloud#name} + * will be used only during Jenkins configuration update config.jelly, + * when new instance of same cloud is created and we need to find old instance and + * repoint resources like {@link Computer} {@link Node} etc. + *

+ * It's lazy to support old versions which don't have this field at all. + *

+ * However it's stable, as soon as it will be created and called first uuid will be same + * for all future calls to the same instances of lazy uuid. + * + * @see EC2FleetCloudAware + */ + private transient LazyUuid id; + /** * Replaced with {@link EC2FleetCloud#awsCredentialsId} *

@@ -122,6 +138,7 @@ public class EC2FleetCloud extends Cloud { @DataBoundConstructor public EC2FleetCloud(final String name, + final String oldId, final String awsCredentialsId, final @Deprecated String credentialsId, final String region, @@ -164,6 +181,12 @@ public EC2FleetCloud(final String name, this.disableTaskResubmit = disableTaskResubmit; this.initOnlineTimeoutSec = initOnlineTimeoutSec; this.initOnlineCheckIntervalSec = initOnlineCheckIntervalSec; + + if (StringUtils.isNotEmpty(oldId)) { + // existent cloud was modified, let's re-assign all dependencies of old cloud instance + // to new one + EC2FleetCloudAwareUtils.reassign(oldId, this); + } } /** @@ -176,6 +199,16 @@ public String getAwsCredentialsId() { return StringUtils.isNotBlank(awsCredentialsId) ? awsCredentialsId : credentialsId; } + /** + * Called old as will be used by new instance of cloud, for + * which this id is old (not current) + * + * @return id of current cloud + */ + public String getOldId() { + return id.getValue(); + } + public boolean isDisableTaskResubmit() { return disableTaskResubmit; } @@ -467,6 +500,8 @@ private Object readResolve() { } private void initCaches() { + id = new LazyUuid(); + plannedNodesCache = new HashSet<>(); fleetInstancesCache = new HashSet<>(); dyingFleetInstancesCache = new HashSet<>(); @@ -527,14 +562,14 @@ private void addNewSlave(final AmazonEC2 ec2, final String instanceId, FleetStat } final EC2FleetAutoResubmitComputerLauncher computerLauncher = new EC2FleetAutoResubmitComputerLauncher( - computerConnector.launch(address, TaskListener.NULL), disableTaskResubmit); + computerConnector.launch(address, TaskListener.NULL)); final Node.Mode nodeMode = restrictUsage ? Node.Mode.EXCLUSIVE : Node.Mode.NORMAL; final EC2FleetNode node = new EC2FleetNode(instanceId, "Fleet slave for " + instanceId, effectiveFsRoot, effectiveNumExecutors, nodeMode, labelString, new ArrayList>(), - name, computerLauncher); + this, computerLauncher); // Initialize our retention strategy - node.setRetentionStrategy(new IdleRetentionStrategy(this)); + node.setRetentionStrategy(new IdleRetentionStrategy()); final Jenkins jenkins = Jenkins.getInstance(); //noinspection SynchronizationOnLocalVariableOrMethodParameter diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudAware.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudAware.java new file mode 100644 index 00000000..4890f3e1 --- /dev/null +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudAware.java @@ -0,0 +1,23 @@ +package com.amazon.jenkins.ec2fleet; + +import javax.annotation.Nonnull; + +/** + * Interface to mark object that it's require cloud change update. Jenkins always creates + * new instance of {@link hudson.slaves.Cloud} each time when you save Jenkins configuration page + * regardless of was it actually changed or not. + *

+ * Jenkins never mutate existent {@link hudson.slaves.Cloud} instance + *

+ * As result all objects which depends on info from cloud + * should be start to consume new instance of object to be able get new configuration if any. + *

+ * {@link EC2FleetCloud} is responsible to update all dependencies with new reference + */ +public interface EC2FleetCloudAware { + + EC2FleetCloud getCloud(); + + void setCloud(@Nonnull EC2FleetCloud cloud); + +} diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudAwareUtils.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudAwareUtils.java new file mode 100644 index 00000000..88cc9586 --- /dev/null +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudAwareUtils.java @@ -0,0 +1,40 @@ +package com.amazon.jenkins.ec2fleet; + +import hudson.model.Computer; +import hudson.model.Node; +import jenkins.model.Jenkins; + +import javax.annotation.Nonnull; +import java.util.logging.Logger; + +/** + * @see EC2FleetCloudAware + */ +@SuppressWarnings("WeakerAccess") +public class EC2FleetCloudAwareUtils { + + private static final Logger LOGGER = Logger.getLogger(EC2FleetCloudAwareUtils.class.getName()); + + public static void reassign(final @Nonnull String oldId, @Nonnull final EC2FleetCloud cloud) { + for (final Computer computer : Jenkins.getActiveInstance().getComputers()) { + checkAndReassign(oldId, cloud, computer); + } + + for (final Node node : Jenkins.getActiveInstance().getNodes()) { + checkAndReassign(oldId, cloud, node); + } + + LOGGER.info("Finish to reassign resources from old cloud with id " + oldId + " to " + cloud.getDisplayName()); + } + + private static void checkAndReassign(final String oldId, final EC2FleetCloud cloud, final Object object) { + if (object instanceof EC2FleetCloudAware) { + final EC2FleetCloudAware cloudAware = (EC2FleetCloudAware) object; + final EC2FleetCloud oldCloud = cloudAware.getCloud(); + if (oldCloud != null && oldId.equals(oldCloud.getOldId())) { + ((EC2FleetCloudAware) object).setCloud(cloud); + LOGGER.info("Reassign " + object + " from " + oldCloud.getDisplayName() + " to " + cloud.getDisplayName()); + } + } + } +} diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetNode.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetNode.java index ec2e211b..4b1abd8a 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetNode.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetNode.java @@ -10,19 +10,20 @@ import hudson.slaves.NodeProperty; import hudson.slaves.RetentionStrategy; +import javax.annotation.Nonnull; import java.io.IOException; import java.util.List; -public class EC2FleetNode extends Slave implements EphemeralNode { +public class EC2FleetNode extends Slave implements EphemeralNode, EC2FleetCloudAware { - private final String cloudName; + private volatile EC2FleetCloud cloud; @SuppressWarnings("WeakerAccess") public EC2FleetNode(final String name, final String nodeDescription, final String remoteFS, final int numExecutors, final Mode mode, final String label, - final List> nodeProperties, final String cloudName, ComputerLauncher launcher) throws IOException, Descriptor.FormException { + final List> nodeProperties, final EC2FleetCloud cloud, ComputerLauncher launcher) throws IOException, Descriptor.FormException { super(name, nodeDescription, remoteFS, numExecutors, mode, label, launcher, RetentionStrategy.NOOP, nodeProperties); - this.cloudName = cloudName; + this.cloud = cloud; } @Override @@ -32,12 +33,26 @@ public Node asNode() { @Override public String getDisplayName() { - return cloudName + " " + name; + // in some multi-thread edge cases cloud could be null for some time, just be ok with that + return (cloud == null ? "unknown fleet" : cloud.getDisplayName()) + " " + name; } @Override public Computer createComputer() { - return new EC2FleetNodeComputer(this); + return new EC2FleetNodeComputer(this, name, cloud); + } + + @Override + public EC2FleetCloud getCloud() { + return cloud; + } + + /** + * {@inheritDoc} + */ + @Override + public void setCloud(@Nonnull EC2FleetCloud cloud) { + this.cloud = cloud; } @Extension diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetNodeComputer.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetNodeComputer.java index 85c6dd68..5a6a0de4 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetNodeComputer.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetNodeComputer.java @@ -4,15 +4,23 @@ import hudson.slaves.SlaveComputer; import javax.annotation.Nonnull; +import javax.annotation.concurrent.ThreadSafe; /** * @see EC2FleetNode * @see EC2FleetAutoResubmitComputerLauncher */ -public class EC2FleetNodeComputer extends SlaveComputer { +@ThreadSafe +public class EC2FleetNodeComputer extends SlaveComputer implements EC2FleetCloudAware { - public EC2FleetNodeComputer(final Slave slave) { + private final String name; + + private volatile EC2FleetCloud cloud; + + public EC2FleetNodeComputer(final Slave slave, @Nonnull final String name, @Nonnull final EC2FleetCloud cloud) { super(slave); + this.name = name; + this.cloud = cloud; } @Override @@ -22,17 +30,28 @@ public EC2FleetNode getNode() { /** * Return label which will represent executor in "Build Executor Status" - * section of Jenkins UI. After reconfiguration actual {@link EC2FleetNode} could - * be removed before this, so name will be just predefined static. + * section of Jenkins UI. * - * @return node display name or if node is null predefined text about that + * @return node display name */ @Nonnull @Override public String getDisplayName() { - // getNode() hit map to find node by name - final EC2FleetNode node = getNode(); - return node == null ? "removing fleet node" : node.getDisplayName(); + // in some multi-thread edge cases cloud could be null for some time, just be ok with that + return (cloud == null ? "unknown fleet" : cloud.getDisplayName()) + " " + name; + } + + /** + * {@inheritDoc} + */ + @Override + public void setCloud(@Nonnull final EC2FleetCloud cloud) { + this.cloud = cloud; + } + + @Override + public EC2FleetCloud getCloud() { + return cloud; } } diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetStatusInfo.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetStatusInfo.java index 73bbbb9c..3999f79d 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetStatusInfo.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetStatusInfo.java @@ -13,7 +13,7 @@ * @see EC2FleetStatusWidget * @see CloudNanny */ -@SuppressWarnings("WeakerAccess") +@SuppressWarnings({"WeakerAccess", "unused"}) @ThreadSafe public class EC2FleetStatusInfo extends Widget { diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/IdleRetentionStrategy.java b/src/main/java/com/amazon/jenkins/ec2fleet/IdleRetentionStrategy.java index 97996b84..a95de580 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/IdleRetentionStrategy.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/IdleRetentionStrategy.java @@ -19,38 +19,38 @@ public class IdleRetentionStrategy extends RetentionStrategy { private static final Logger LOGGER = Logger.getLogger(IdleRetentionStrategy.class.getName()); - private final int maxIdleMinutes; - private final boolean alwaysReconnect; - private final EC2FleetCloud cloud; - - @SuppressWarnings("WeakerAccess") - public IdleRetentionStrategy(final EC2FleetCloud cloud) { - this.maxIdleMinutes = cloud.getIdleMinutes(); - this.alwaysReconnect = cloud.isAlwaysReconnect(); - this.cloud = cloud; - } - /** * Will be called under {@link hudson.model.Queue#withLock(Runnable)} * - * @param c computer + * @param computer computer * @return delay in min before next run */ @GuardedBy("Queue.withLock") @Override - public long check(final SlaveComputer c) { + public long check(final SlaveComputer computer) { + final EC2FleetNodeComputer fc = (EC2FleetNodeComputer) computer; + final EC2FleetCloud cloud = fc.getCloud(); + + // in some multi-thread edge cases cloud could be null for some time, just be ok with that + if (cloud == null) { + LOGGER.warning("Edge case cloud is null for computer " + fc.getDisplayName() + + " should be autofixed in a few minutes, if no please create issue for plugin"); + return RE_CHECK_IN_MINUTE; + } + // Ensure that the EC2FleetCloud cannot be mutated from under us while // we're doing this check + //noinspection SynchronizationOnLocalVariableOrMethodParameter synchronized (cloud) { // Ensure nobody provisions onto this node until we've done // checking - boolean shouldAcceptTasks = c.isAcceptingTasks(); + boolean shouldAcceptTasks = fc.isAcceptingTasks(); boolean justTerminated = false; - c.setAcceptingTasks(false); + fc.setAcceptingTasks(false); try { - if (c.isIdle() && isIdleForTooLong(c)) { + if (fc.isIdle() && isIdleForTooLong(cloud, fc)) { // Find instance ID - Node compNode = c.getNode(); + Node compNode = fc.getNode(); if (compNode == null) { return 0; } @@ -63,12 +63,12 @@ public long check(final SlaveComputer c) { } } - if (alwaysReconnect && !justTerminated && c.isOffline() && !c.isConnecting() && c.isLaunchSupported()) { - LOGGER.log(Level.INFO, "Reconnecting to instance: " + c.getDisplayName()); - c.tryReconnect(); + if (cloud.isAlwaysReconnect() && !justTerminated && fc.isOffline() && !fc.isConnecting() && fc.isLaunchSupported()) { + LOGGER.log(Level.INFO, "Reconnecting to instance: " + fc.getDisplayName()); + fc.tryReconnect(); } } finally { - c.setAcceptingTasks(shouldAcceptTasks); + fc.setAcceptingTasks(shouldAcceptTasks); } } @@ -81,12 +81,12 @@ public void start(SlaveComputer c) { c.connect(false); } - private boolean isIdleForTooLong(final Computer c) { - if (maxIdleMinutes <= 0) return false; - final long idleTime = System.currentTimeMillis() - c.getIdleStartMilliseconds(); - final long maxIdle = TimeUnit.MINUTES.toMillis(maxIdleMinutes); - LOGGER.log(Level.FINE, "Instance: " + c.getDisplayName() + " Age: " + idleTime + " Max Age:" + maxIdle); + private boolean isIdleForTooLong(final EC2FleetCloud cloud, final Computer computer) { + final int idleMinutes = cloud.getIdleMinutes(); + if (idleMinutes <= 0) return false; + final long idleTime = System.currentTimeMillis() - computer.getIdleStartMilliseconds(); + final long maxIdle = TimeUnit.MINUTES.toMillis(idleMinutes); + LOGGER.log(Level.FINE, "Instance: " + computer.getDisplayName() + " Age: " + idleTime + " Max Age:" + maxIdle); return idleTime > maxIdle; } - } diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/LazyUuid.java b/src/main/java/com/amazon/jenkins/ec2fleet/LazyUuid.java new file mode 100644 index 00000000..c6030335 --- /dev/null +++ b/src/main/java/com/amazon/jenkins/ec2fleet/LazyUuid.java @@ -0,0 +1,22 @@ +package com.amazon.jenkins.ec2fleet; + +import javax.annotation.concurrent.ThreadSafe; +import java.util.UUID; + +/** + * Provide uuid string, create it when first call happens + */ +@ThreadSafe +@SuppressWarnings("WeakerAccess") +public class LazyUuid { + + private String value; + + public synchronized String getValue() { + if (value == null) { + value = UUID.randomUUID().toString(); + } + return value; + } + +} diff --git a/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/config.jelly b/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/config.jelly index 5a18b329..5d804186 100644 --- a/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/config.jelly +++ b/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/config.jelly @@ -4,12 +4,17 @@ xmlns:t="/lib/hudson" xmlns:f="/lib/form" xmlns:c="/lib/credentials"> - + + + + + diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/AutoResubmitIntegrationTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/AutoResubmitIntegrationTest.java index 153c0789..9236c8ca 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/AutoResubmitIntegrationTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/AutoResubmitIntegrationTest.java @@ -76,7 +76,7 @@ public void before() { @Test public void should_successfully_resubmit_freestyle_task() throws Exception { - EC2FleetCloud cloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud cloud = new EC2FleetCloud(null, null,"credId", null, "region", null, "fId", "momo", null, new SingleLocalComputerConnector(j), false, false, 0, 0, 10, 1, false, false, false, 0, 0, false); @@ -112,7 +112,7 @@ public void should_successfully_resubmit_freestyle_task() throws Exception { @Test public void should_successfully_resubmit_parametrized_task() throws Exception { - EC2FleetCloud cloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud cloud = new EC2FleetCloud(null, null,"credId", null, "region", null, "fId", "momo", null, new SingleLocalComputerConnector(j), false, false, 0, 0, 10, 1, false, false, false, 0, 0, false); @@ -168,7 +168,7 @@ public void should_successfully_resubmit_parametrized_task() throws Exception { @Test public void should_not_resubmit_if_disabled() throws Exception { - EC2FleetCloud cloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud cloud = new EC2FleetCloud(null, null,"credId", null, "region", null, "fId", "momo", null, new SingleLocalComputerConnector(j), false, false, 0, 0, 10, 1, false, false, true, 0, 0, false); diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncherTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncherTest.java index f307f0b0..ebecd650 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncherTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetAutoResubmitComputerLauncherTest.java @@ -10,7 +10,6 @@ import hudson.model.queue.SubTask; import hudson.slaves.ComputerLauncher; import hudson.slaves.OfflineCause; -import hudson.slaves.SlaveComputer; import jenkins.model.Jenkins; import org.junit.Before; import org.junit.Test; @@ -60,7 +59,7 @@ public class EC2FleetAutoResubmitComputerLauncherTest { private Slave slave; @Mock - private SlaveComputer computer; + private EC2FleetNodeComputer computer; @Mock private Jenkins jenkins; @@ -83,6 +82,9 @@ public class EC2FleetAutoResubmitComputerLauncherTest { @Mock private EC2FleetNode fleetNode; + @Mock + private EC2FleetCloud cloud; + @Before public void before() { executable1 = mock(Actionable.class, withSettings().extraInterfaces(Queue.Executable.class)); @@ -108,11 +110,13 @@ public void before() { when(computer.getExecutors()).thenReturn(Arrays.asList(executor1, executor2)); when(computer.isOffline()).thenReturn(true); + + when(computer.getCloud()).thenReturn(cloud); } @Test public void afterDisconnect_should_do_nothing_if_task_finished_without_cause() { - new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher, false) + new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher) .afterDisconnect(computer, taskListener); verifyZeroInteractions(queue); } @@ -120,7 +124,7 @@ public void afterDisconnect_should_do_nothing_if_task_finished_without_cause() { @Test public void afterDisconnect_should_do_nothing_if_task_finished_offline_but_no_cause() { when(computer.isOffline()).thenReturn(true); - new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher, false) + new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher) .afterDisconnect(computer, taskListener); verifyZeroInteractions(queue); } @@ -129,7 +133,7 @@ public void afterDisconnect_should_do_nothing_if_task_finished_offline_but_no_ca public void afterDisconnect_should_do_nothing_if_task_finished_cause_but_still_online() { when(computer.isOffline()).thenReturn(false); when(computer.getOfflineCause()).thenReturn(new OfflineCause.ChannelTermination(null)); - new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher, false) + new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher) .afterDisconnect(computer, taskListener); verifyZeroInteractions(queue); } @@ -138,16 +142,26 @@ public void afterDisconnect_should_do_nothing_if_task_finished_cause_but_still_o public void taskCompleted_should_resubmit_task_if_offline_and_cause_disconnect() { when(computer.getExecutors()).thenReturn(Arrays.asList(executor1)); when(computer.getOfflineCause()).thenReturn(new OfflineCause.ChannelTermination(null)); - new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher, false) + new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher) .afterDisconnect(computer, taskListener); verify(queue).schedule2(eq(task1), anyInt(), eq(Collections.emptyList())); verifyZeroInteractions(queue); } + @Test + public void taskCompleted_should_not_resubmit_task_if_offline_and_cause_disconnect_but_disabled() { + when(cloud.isDisableTaskResubmit()).thenReturn(true); + when(computer.getExecutors()).thenReturn(Arrays.asList(executor1)); + when(computer.getOfflineCause()).thenReturn(new OfflineCause.ChannelTermination(null)); + new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher) + .afterDisconnect(computer, taskListener); + verifyZeroInteractions(queue); + } + @Test public void taskCompleted_should_resubmit_task_for_all_executors() { when(computer.getOfflineCause()).thenReturn(new OfflineCause.ChannelTermination(null)); - new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher, false) + new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher) .afterDisconnect(computer, taskListener); verify(queue).schedule2(eq(task1), anyInt(), eq(Collections.emptyList())); verify(queue).schedule2(eq(task2), anyInt(), eq(Collections.emptyList())); @@ -157,7 +171,7 @@ public void taskCompleted_should_resubmit_task_for_all_executors() { @Test public void taskCompleted_should_abort_executors_during_resubmit() { when(computer.getOfflineCause()).thenReturn(new OfflineCause.ChannelTermination(null)); - new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher, false) + new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher) .afterDisconnect(computer, taskListener); verify(executor1).interrupt(Result.ABORTED, new EC2TerminationCause("i-12")); verify(executor2).interrupt(Result.ABORTED, new EC2TerminationCause("i-12")); @@ -168,7 +182,7 @@ public void taskCompleted_should_resubmit_task_with_actions() { when(computer.getExecutors()).thenReturn(Arrays.asList(executor1)); when(executable1.getActions()).thenReturn(Arrays.asList(action1)); when(computer.getOfflineCause()).thenReturn(new OfflineCause.ChannelTermination(null)); - new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher, false) + new EC2FleetAutoResubmitComputerLauncher(baseComputerLauncher) .afterDisconnect(computer, taskListener); verify(queue).schedule2(eq(task1), anyInt(), eq(Arrays.asList(action1))); verifyZeroInteractions(queue); diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudAwareUtilsTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudAwareUtilsTest.java new file mode 100644 index 00000000..c7c7848b --- /dev/null +++ b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudAwareUtilsTest.java @@ -0,0 +1,120 @@ +package com.amazon.jenkins.ec2fleet; + +import hudson.model.Computer; +import hudson.model.LabelFinder; +import hudson.model.Node; +import jenkins.model.Jenkins; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; + +import java.util.Arrays; +import java.util.Collections; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@SuppressWarnings("ArraysAsListWithZeroOrOneArgument") +@RunWith(PowerMockRunner.class) +@PrepareForTest({Jenkins.class, LabelFinder.class}) +public class EC2FleetCloudAwareUtilsTest { + + @Mock + private Jenkins jenkins; + + @Mock + private EC2FleetCloud cloud; + + @Mock + private EC2FleetCloud oldCloud; + + @Mock + private EC2FleetCloud otherCloud; + + @Mock + private EC2FleetNodeComputer computer; + + @Mock + private EC2FleetNode node; + + @Before + public void before() { + PowerMockito.mockStatic(LabelFinder.class); + + PowerMockito.mockStatic(Jenkins.class); + PowerMockito.when(Jenkins.getActiveInstance()).thenReturn(jenkins); + + when(oldCloud.getOldId()).thenReturn("cloud"); + when(computer.getCloud()).thenReturn(oldCloud); + when(node.getCloud()).thenReturn(oldCloud); + + when(cloud.getOldId()).thenReturn("cloud"); + when(otherCloud.getOldId()).thenReturn("other"); + + when(jenkins.getNodes()).thenReturn(Collections.emptyList()); + when(jenkins.getComputers()).thenReturn(new Computer[0]); + } + + @Test + public void reassign_nothing_if_no_nodes_or_computers() { + EC2FleetCloudAwareUtils.reassign("cloud", cloud); + } + + @Test + public void reassign_nothing_if_computers_belong_to_diff_cloud_id() { + when(jenkins.getNodes()).thenReturn(Collections.emptyList()); + when(computer.getCloud()).thenReturn(otherCloud); + when(jenkins.getComputers()).thenReturn(new Computer[]{computer}); + + EC2FleetCloudAwareUtils.reassign("cloud", cloud); + + verify(computer, times(0)).setCloud(any(EC2FleetCloud.class)); + } + + @Test + public void reassign_nothing_if_computer_cloud_is_null() { + when(computer.getCloud()).thenReturn(null); + when(jenkins.getComputers()).thenReturn(new Computer[]{computer}); + + EC2FleetCloudAwareUtils.reassign("cloud", cloud); + + verify(computer, times(0)).setCloud(any(EC2FleetCloud.class)); + } + + @Test + public void reassign_if_computer_belong_to_old_cloud() { + when(jenkins.getComputers()).thenReturn(new Computer[]{computer}); + + EC2FleetCloudAwareUtils.reassign("cloud", cloud); + + verify(computer, times(1)).setCloud(cloud); + } + + @Test + public void reassign_if_node_belong_to_same_cloud() { + when(computer.getCloud()).thenReturn(cloud); + when(jenkins.getNodes()).thenReturn(Arrays.asList((Node) node)); + + EC2FleetCloudAwareUtils.reassign("cloud", cloud); + + verify(node, times(1)).setCloud(cloud); + } + + @Test + public void reassign_nothing_if_node_belong_to_other_cloud_id() { + when(computer.getCloud()).thenReturn(cloud); + when(node.getCloud()).thenReturn(otherCloud); + when(jenkins.getNodes()).thenReturn(Arrays.asList((Node) node)); + + EC2FleetCloudAwareUtils.reassign("cloud", cloud); + + verify(node, times(0)).setCloud(cloud); + } + +} diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudTest.java index 0157efcf..b24d746c 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudTest.java @@ -122,7 +122,7 @@ public void provision_fleetIsEmpty() { when(amazonEC2.describeSpotFleetRequests(any(DescribeSpotFleetRequestsRequest.class))) .thenReturn(describeSpotFleetRequestsResult); - EC2FleetCloud fleetCloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud fleetCloud = new EC2FleetCloud(null, null, "credId", null, "region", "", null, null, null, null, false, false, 0, 0, 1, 1, false, false, false, 0, 0, false); @@ -152,7 +152,7 @@ public void updateStatus_doNothingWhenFleetIsEmpty() { when(amazonEC2.describeSpotFleetRequests(any(DescribeSpotFleetRequestsRequest.class))) .thenReturn(describeSpotFleetRequestsResult); - EC2FleetCloud fleetCloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud fleetCloud = new EC2FleetCloud(null, null, "credId", null, "region", "", "fleetId", null, null, null, false, false, 0, 0, 1, 1, false, false, false, 0, @@ -200,7 +200,7 @@ public void updateStatus_shouldAddNodeIfAnyNewDescribed() throws IOException { mockNodeCreatingPart(); - EC2FleetCloud fleetCloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud fleetCloud = new EC2FleetCloud(null, null, "credId", null, "region", "", "fleetId", null, null, PowerMockito.mock(ComputerConnector.class), false, false, 0, 0, 1, 1, false, false, false, @@ -255,7 +255,7 @@ public void updateStatus_shouldAddNodeIfAnyNewDescribed_restrictUsage() throws I mockNodeCreatingPart(); - EC2FleetCloud fleetCloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud fleetCloud = new EC2FleetCloud(null, null, "credId", null, "region", "", "fleetId", null, null, PowerMockito.mock(ComputerConnector.class), false, false, 0, 0, 1, 1, false, true, false, @@ -317,7 +317,7 @@ public void updateStatus_shouldAddNodeWithNumExecutors_whenWeightProvidedButNotE mockNodeCreatingPart(); - EC2FleetCloud fleetCloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud fleetCloud = new EC2FleetCloud(null, null, "credId", null, "region", "", "fleetId", null, null, PowerMockito.mock(ComputerConnector.class), false, false, 0, 0, 1, 1, false, true, false, @@ -367,7 +367,7 @@ public void updateStatus_shouldAddNodeWithScaledNumExecutors_whenWeightPresentAn mockNodeCreatingPart(); - EC2FleetCloud fleetCloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud fleetCloud = new EC2FleetCloud(null, null, "credId", null, "region", "", "fleetId", null, null, PowerMockito.mock(ComputerConnector.class), false, false, 0, 0, 1, 1, false, true, false, @@ -415,7 +415,7 @@ public void updateStatus_shouldAddNodeWithNumExecutors_whenWeightPresentAndEnabl mockNodeCreatingPart(); - EC2FleetCloud fleetCloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud fleetCloud = new EC2FleetCloud(null, null, "credId", null, "region", "", "fleetId", null, null, PowerMockito.mock(ComputerConnector.class), false, false, 0, 0, 1, 1, false, true, false, @@ -464,7 +464,7 @@ public void updateStatus_shouldAddNodeWithRoundToLowScaledNumExecutors_whenWeigh mockNodeCreatingPart(); - EC2FleetCloud fleetCloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud fleetCloud = new EC2FleetCloud(null, null, "credId", null, "region", "", "fleetId", null, null, PowerMockito.mock(ComputerConnector.class), false, false, 0, 0, 1, 1, false, true, false, @@ -513,7 +513,7 @@ public void updateStatus_shouldAddNodeWithRoundToLowScaledNumExecutors_whenWeigh mockNodeCreatingPart(); - EC2FleetCloud fleetCloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud fleetCloud = new EC2FleetCloud(null, null, "credId", null, "region", "", "fleetId", null, null, PowerMockito.mock(ComputerConnector.class), false, false, 0, 0, 1, 1, false, true, false, @@ -562,7 +562,7 @@ public void updateStatus_shouldAddNodeWithScaledToOneNumExecutors_whenWeightPres mockNodeCreatingPart(); - EC2FleetCloud fleetCloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud fleetCloud = new EC2FleetCloud(null, null, "credId", null, "region", "", "fleetId", null, null, PowerMockito.mock(ComputerConnector.class), false, false, 0, 0, 1, 1, false, true, false, @@ -772,7 +772,7 @@ public void descriptorImpl_doFillFleetItems_returnEmptyListIfAnyException() { @Test public void getDisplayName_returnDefaultWhenNull() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( - null, null, null, null, null, null, + null, null, null, null, null, null, null, null, null, null, false, false, null, null, null, null, false, false, false @@ -783,7 +783,7 @@ public void getDisplayName_returnDefaultWhenNull() { @Test public void getDisplayName_returnDisplayName() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( - "CloudName", null, null, null, null, null, + "CloudName", null, null, null, null, null, null, null, null, null, false, false, null, null, null, null, false, false, false @@ -794,7 +794,7 @@ public void getDisplayName_returnDisplayName() { @Test public void getAwsCredentialsId_returnNull_whenNoCredentialsIdOrAwsCredentialsId() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( - null, null, null, null, null, null, + null, null, null, null, null, null, null, null, null, null, false, false, null, null, null, null, false, false, false @@ -805,7 +805,7 @@ public void getAwsCredentialsId_returnNull_whenNoCredentialsIdOrAwsCredentialsId @Test public void getAwsCredentialsId_returnValue_whenCredentialsIdPresent() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( - null, null, "Opa", null, null, null, + null, null, null, "Opa", null, null, null, null, null, null, false, false, null, null, null, null, false, false, false @@ -816,7 +816,7 @@ public void getAwsCredentialsId_returnValue_whenCredentialsIdPresent() { @Test public void getAwsCredentialsId_returnValue_whenAwsCredentialsIdPresent() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( - null, "Opa", null, null, null, null, + null, null, "Opa", null, null, null, null, null, null, null, false, false, null, null, null, null, false, false, false @@ -827,7 +827,7 @@ public void getAwsCredentialsId_returnValue_whenAwsCredentialsIdPresent() { @Test public void getAwsCredentialsId_returnAwsCredentialsId_whenAwsCredentialsIdAndCredentialsIdPresent() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( - null, "A", "B", null, null, null, + null, null, "A", "B", null, null, null, null, null, null, false, false, null, null, null, null, false, false, false diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetNodeComputerTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetNodeComputerTest.java index 37c2f8db..676029b9 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetNodeComputerTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetNodeComputerTest.java @@ -12,7 +12,6 @@ import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; @@ -30,7 +29,7 @@ public class EC2FleetNodeComputerTest { private Queue queue; @Mock - private EC2FleetNode fleetNode; + private EC2FleetCloud cloud; @Before public void before() { @@ -39,22 +38,26 @@ public void before() { when(Queue.getInstance()).thenReturn(queue); when(slave.getNumExecutors()).thenReturn(1); + } - when(fleetNode.getDisplayName()).thenReturn("fleet node name"); + @Test + public void getDisplayName_should_be_ok_with_init_null_cloud() { + EC2FleetNodeComputer computer = spy(new EC2FleetNodeComputer(slave, "a", null)); + Assert.assertEquals("unknown fleet a", computer.getDisplayName()); } @Test - public void getDisplayName_computer_node_removed_returns_predefined() { - EC2FleetNodeComputer computer = spy(new EC2FleetNodeComputer(slave)); - doReturn(null).when(computer).getNode(); - Assert.assertEquals("removing fleet node", computer.getDisplayName()); + public void getDisplayName_should_be_ok_with_set_null_cloud() { + EC2FleetNodeComputer computer = spy(new EC2FleetNodeComputer(slave, "a", cloud)); + computer.setCloud(null); + Assert.assertEquals("unknown fleet a", computer.getDisplayName()); } @Test public void getDisplayName_returns_node_display_name() { - EC2FleetNodeComputer computer = spy(new EC2FleetNodeComputer(slave)); - doReturn(fleetNode).when(computer).getNode(); - Assert.assertEquals("fleet node name", computer.getDisplayName()); + when(cloud.getDisplayName()).thenReturn("a"); + EC2FleetNodeComputer computer = spy(new EC2FleetNodeComputer(slave, "n", cloud)); + Assert.assertEquals("a n", computer.getDisplayName()); } } diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/IdleRetentionStrategyTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/IdleRetentionStrategyTest.java index e21894e0..e770d393 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/IdleRetentionStrategyTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/IdleRetentionStrategyTest.java @@ -1,6 +1,5 @@ package com.amazon.jenkins.ec2fleet; -import hudson.model.Slave; import hudson.slaves.SlaveComputer; import org.junit.Before; import org.junit.Test; @@ -28,10 +27,10 @@ public class IdleRetentionStrategyTest { private EC2FleetCloud cloud; @Mock - private SlaveComputer slaveComputer; + private EC2FleetNodeComputer slaveComputer; @Mock - private Slave slave; + private EC2FleetNode slave; @Before public void before() { @@ -41,13 +40,14 @@ public void before() { PowerMockito.when(slaveComputer.isIdle()).thenReturn(true); when(slave.getNodeName()).thenReturn("n-a"); when(slaveComputer.isAcceptingTasks()).thenReturn(true); + when(slaveComputer.getCloud()).thenReturn(cloud); } @Test public void if_idle_time_not_configured_should_do_nothing() { when(cloud.getIdleMinutes()).thenReturn(0); - new IdleRetentionStrategy(cloud).check(slaveComputer); + new IdleRetentionStrategy().check(slaveComputer); verify(slaveComputer, times(0)).getNode(); verify(cloud, times(0)).terminateInstance(anyString()); @@ -59,7 +59,7 @@ public void if_idle_time_not_configured_should_do_nothing() { public void if_idle_time_configured_should_do_nothing_if_node_idle_less_time() { when(slaveComputer.getIdleStartMilliseconds()).thenReturn(System.currentTimeMillis()); - new IdleRetentionStrategy(cloud).check(slaveComputer); + new IdleRetentionStrategy().check(slaveComputer); verify(slaveComputer, never()).getNode(); verify(cloud, never()).terminateInstance(anyString()); @@ -71,7 +71,7 @@ public void if_idle_time_configured_should_do_nothing_if_node_idle_less_time() { public void if_node_not_execute_anything_yet_idle_time_negative_do_nothing() { when(slaveComputer.getIdleStartMilliseconds()).thenReturn(Long.MIN_VALUE); - new IdleRetentionStrategy(cloud).check(slaveComputer); + new IdleRetentionStrategy().check(slaveComputer); verify(slaveComputer, times(0)).getNode(); verify(cloud, times(0)).terminateInstance(anyString()); @@ -81,19 +81,30 @@ public void if_node_not_execute_anything_yet_idle_time_negative_do_nothing() { @Test public void if_idle_time_configured_should_terminate_node_if_idle_time_more_then_allowed() { - new IdleRetentionStrategy(cloud).check(slaveComputer); + new IdleRetentionStrategy().check(slaveComputer); verify(cloud, times(1)).terminateInstance("n-a"); verify(slaveComputer, times(1)).setAcceptingTasks(true); verify(slaveComputer, times(1)).setAcceptingTasks(false); } + @Test + public void if_computer_has_no_cloud_should_do_nothing() { + when(slaveComputer.getCloud()).thenReturn(null); + + new IdleRetentionStrategy().check(slaveComputer); + + verify(cloud, times(0)).terminateInstance(anyString()); + verify(slaveComputer, times(0)).setAcceptingTasks(true); + verify(slaveComputer, times(0)).setAcceptingTasks(false); + } + @Test public void if_node_not_idle_should_do_nothing() { when(slaveComputer.getIdleStartMilliseconds()).thenReturn(0L); when(slaveComputer.isIdle()).thenReturn(false); - new IdleRetentionStrategy(cloud).check(slaveComputer); + new IdleRetentionStrategy().check(slaveComputer); verify(cloud, never()).terminateInstance("n-a"); verify(slaveComputer, times(1)).setAcceptingTasks(true); @@ -104,7 +115,7 @@ public void if_node_not_idle_should_do_nothing() { public void if_node_idle_time_more_them_allowed_but_not_idle_should_do_nothing() { when(slaveComputer.isIdle()).thenReturn(false); - new IdleRetentionStrategy(cloud).check(slaveComputer); + new IdleRetentionStrategy().check(slaveComputer); verify(cloud, never()).terminateInstance("n-a"); verify(slaveComputer, times(1)).setAcceptingTasks(true); @@ -116,7 +127,7 @@ public void if_exception_happen_during_termination_should_throw_it_and_restore_t when(cloud.terminateInstance(anyString())).thenThrow(new IllegalArgumentException("test")); try { - new IdleRetentionStrategy(cloud).check(slaveComputer); + new IdleRetentionStrategy().check(slaveComputer); fail(); } catch (IllegalArgumentException e) { assertEquals("test", e.getMessage()); diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/LazyUuidTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/LazyUuidTest.java new file mode 100644 index 00000000..9e7066b1 --- /dev/null +++ b/src/test/java/com/amazon/jenkins/ec2fleet/LazyUuidTest.java @@ -0,0 +1,72 @@ +package com.amazon.jenkins.ec2fleet; + +import org.junit.Assert; +import org.junit.Test; + +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.TimeUnit; + +public class LazyUuidTest { + + @Test + public void getValue_provides_uuid() { + Assert.assertEquals(36, new LazyUuid().getValue().length()); + } + + @Test + public void getValue_provides_same_value_if_multiplecall() { + final LazyUuid lazyUuid = new LazyUuid(); + Assert.assertEquals(lazyUuid.getValue(), lazyUuid.getValue()); + Assert.assertEquals(lazyUuid.getValue(), lazyUuid.getValue()); + } + + @Test + public void getValue_is_thread_safe() throws InterruptedException { + final LazyUuid lazyUuid = new LazyUuid(); + + final LazyUuidGetter[] threads = new LazyUuidGetter[3]; + + for (int i = 0; i < threads.length; i++) { + threads[i] = new LazyUuidGetter(lazyUuid); + threads[i].start(); + } + + Thread.sleep(TimeUnit.SECONDS.toMillis(10)); + + final Set history = new HashSet<>(); + + for (final LazyUuidGetter thread : threads) { + thread.interrupt(); + + history.addAll(thread.history); + } + + Assert.assertEquals(1, history.size()); + Assert.assertNotNull(history.iterator().next()); + } + + private static class LazyUuidGetter extends Thread { + + private final Set history; + private final LazyUuid lazyUuid; + + LazyUuidGetter(LazyUuid lazyUuid) { + this.lazyUuid = lazyUuid; + history = new HashSet<>(); + } + + @Override + public void run() { + while (true) { + history.add(lazyUuid.getValue()); + + try { + Thread.sleep(100); + } catch (InterruptedException e) { + return; // stop + } + } + } + } +} diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/ProvisionIntegrationTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/ProvisionIntegrationTest.java index 55371912..52e0685a 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/ProvisionIntegrationTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/ProvisionIntegrationTest.java @@ -46,7 +46,7 @@ public void dont_provide_any_planned_if_empty_and_reached_max_capacity() throws ComputerConnector computerConnector = mock(ComputerConnector.class); when(computerConnector.launch(anyString(), any(TaskListener.class))).thenReturn(computerLauncher); - EC2FleetCloud cloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud cloud = new EC2FleetCloud(null, null, "credId", null, "region", null, "fId", "momo", null, computerConnector, false, false, 0, 0, 0, 1, false, false, false, 0, 0, false); @@ -89,7 +89,7 @@ public void should_add_planned_if_capacity_required_but_not_described_yet() thro ComputerConnector computerConnector = mock(ComputerConnector.class); when(computerConnector.launch(anyString(), any(TaskListener.class))).thenReturn(computerLauncher); - EC2FleetCloud cloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud cloud = new EC2FleetCloud(null, null, "credId", null, "region", null, "fId", "momo", null, computerConnector, false, false, 0, 0, 10, 1, false, false, false, 0, 0, false); @@ -121,7 +121,7 @@ public void should_keep_planned_node_until_node_will_not_be_online_so_jenkins_wi ComputerConnector computerConnector = mock(ComputerConnector.class); when(computerConnector.launch(anyString(), any(TaskListener.class))).thenReturn(computerLauncher); - EC2FleetCloud cloud = spy(new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud cloud = spy(new EC2FleetCloud(null, null, "credId", null, "region", null, "fId", "momo", null, computerConnector, false, false, 0, 0, 10, 1, false, false, false, 300, 15, false)); @@ -147,7 +147,7 @@ public void should_not_keep_planned_node_if_configured_so_jenkins_will_overprovi ComputerConnector computerConnector = mock(ComputerConnector.class); when(computerConnector.launch(anyString(), any(TaskListener.class))).thenReturn(computerLauncher); - final EC2FleetCloud cloud = spy(new EC2FleetCloud(null, "credId", null, "region", + final EC2FleetCloud cloud = spy(new EC2FleetCloud(null, null, "credId", null, "region", null, "fId", "momo", null, computerConnector, false, false, 0, 0, 10, 1, false, false, false, 0, 0, false)); @@ -172,7 +172,7 @@ public void should_not_allow_jenkins_to_provision_if_address_not_available() thr ComputerConnector computerConnector = mock(ComputerConnector.class); when(computerConnector.launch(anyString(), any(TaskListener.class))).thenReturn(computerLauncher); - EC2FleetCloud cloud = spy(new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud cloud = spy(new EC2FleetCloud(null, null, "credId", null, "region", null, "fId", "momo", null, computerConnector, false, false, 0, 0, 10, 1, false, false, false, 0, 0, false)); @@ -224,7 +224,7 @@ public void should_not_convert_planned_to_node_if_state_is_not_running_and_check ComputerConnector computerConnector = mock(ComputerConnector.class); when(computerConnector.launch(anyString(), any(TaskListener.class))).thenReturn(computerLauncher); - EC2FleetCloud cloud = new EC2FleetCloud(null, "credId", null, "region", + EC2FleetCloud cloud = new EC2FleetCloud(null, null, "credId", null, "region", null, "fId", "momo", null, computerConnector, false, false, 0, 0, 10, 1, true, false, false, 0, 0, false); diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/RealEc2ApiIntegrationTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/RealEc2ApiIntegrationTest.java index fb051f15..32409a32 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/RealEc2ApiIntegrationTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/RealEc2ApiIntegrationTest.java @@ -48,10 +48,10 @@ public void shouldSuccessfullyUpdatePluginWithFleetStatus() throws Exception { withFleet(awsCredentials, targetCapacity, new WithFleetBody() { @Override public void run(AmazonEC2 amazonEC2, String fleetId) throws Exception { - EC2FleetCloud cloud = new EC2FleetCloud("", "credId", null, null, null, fleetId, + EC2FleetCloud cloud = new EC2FleetCloud("", null, "credId", null, null, null, fleetId, null, null, null, false, false, 0, 0, 0, 0, false, false, - false,0, 0, false); + false, 0, 0, false); j.jenkins.clouds.add(cloud); // 10 sec refresh time so wait @@ -81,10 +81,10 @@ public void shouldSuccessfullyUpdateBigFleetPluginWithFleetStatus() throws Excep withFleet(awsCredentials, targetCapacity, new WithFleetBody() { @Override public void run(AmazonEC2 amazonEC2, String fleetId) throws Exception { - EC2FleetCloud cloud = new EC2FleetCloud(null, "credId", null, null, null, fleetId, + EC2FleetCloud cloud = new EC2FleetCloud(null, null, "credId", null, null, null, fleetId, null, null, null, false, false, 0, 0, 0, 0, false, false, - false,0, 0, false); + false, 0, 0, false); j.jenkins.clouds.add(cloud); final long start = System.currentTimeMillis(); diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/UiIntegrationTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/UiIntegrationTest.java index 7f7357ca..29f34bc9 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/UiIntegrationTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/UiIntegrationTest.java @@ -6,7 +6,11 @@ import com.gargoylesoftware.htmlunit.html.HtmlPage; import com.gargoylesoftware.htmlunit.html.HtmlTextInput; import hudson.PluginWrapper; +import hudson.model.Node; import hudson.slaves.Cloud; +import hudson.slaves.NodeProperty; +import org.apache.commons.lang.StringUtils; +import org.hamcrest.Matchers; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; @@ -20,7 +24,10 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * Detailed guides https://jenkins.io/doc/developer/testing/ @@ -40,36 +47,72 @@ public void shouldFindThePluginByShortName() { assertNotNull("should have a valid plugin", wrapper); } + @Test + public void shouldShowAsHiddenCloudIdAsOldId() throws IOException, SAXException { + Cloud cloud = new EC2FleetCloud(null, null, null, null, null, null, null, + null, null, null, false, false, + 0, 0, 0, 0, false, false, + false, 0, 0, false); + j.jenkins.clouds.add(cloud); + + HtmlPage page = j.createWebClient().goTo("configure"); + + assertTrue(StringUtils.isNotBlank(((HtmlTextInput) getElementsByNameWithoutJdk(page, "_.oldId").get(0)).getText())); + } + + @Test + public void shouldReplaceCloudForNodesAfterConfigurationSave() throws Exception { + EC2FleetCloud cloud = new EC2FleetCloud(null, null, null, null, null, null, null, + null, null, null, false, false, + 0, 0, 0, 0, false, false, + false, 0, 0, false); + j.jenkins.clouds.add(cloud); + + j.jenkins.addNode(new EC2FleetNode("mock", "", "", 1, + Node.Mode.EXCLUSIVE, "", new ArrayList>(), cloud, + j.createComputerLauncher(null))); + + HtmlPage page = j.createWebClient().goTo("configure"); + HtmlForm form = page.getFormByName("config"); + + ((HtmlTextInput) getElementsByNameWithoutJdk(page, "_.name").get(0)).setText("a"); + + HtmlFormUtil.submit(form); + + final Cloud newCloud = j.jenkins.clouds.get(0); + assertNotSame(cloud, newCloud); + + assertSame(newCloud, ((EC2FleetNode) j.jenkins.getNode("mock")).getCloud()); + } + @Test public void shouldShowInConfigurationClouds() throws IOException, SAXException { - Cloud cloud = new EC2FleetCloud(null, null, null, null, null, null, + Cloud cloud = new EC2FleetCloud(null, null, null, null, null, null, null, null, null, null, false, false, 0, 0, 0, 0, false, false, false, 0, 0, false); j.jenkins.clouds.add(cloud); HtmlPage page = j.createWebClient().goTo("configure"); - System.out.println(page); assertEquals("ec2-fleet", ((HtmlTextInput) getElementsByNameWithoutJdk(page, "_.labelString").get(1)).getText()); } @Test public void shouldShowMultipleClouds() throws IOException, SAXException { - Cloud cloud1 = new EC2FleetCloud("a", null, null, null, null, + Cloud cloud1 = new EC2FleetCloud("a", null, null, null, null, null, null, null, null, null, false, false, 0, 0, 0, 0, false, false, false, 0, 0, false); j.jenkins.clouds.add(cloud1); - Cloud cloud2 = new EC2FleetCloud("b", null, null, null, null, + Cloud cloud2 = new EC2FleetCloud("b", null, null, null, null, null, null, null, null, null, false, false, 0, 0, 0, 0, false, false, false, 0, 0, false); j.jenkins.clouds.add(cloud2); HtmlPage page = j.createWebClient().goTo("configure"); - System.out.println(page); List elementsByName = getElementsByNameWithoutJdk(page, "_.name"); assertEquals(2, elementsByName.size()); @@ -79,20 +122,19 @@ public void shouldShowMultipleClouds() throws IOException, SAXException { @Test public void shouldShowMultipleCloudsWithDefaultName() throws IOException, SAXException { - Cloud cloud1 = new EC2FleetCloud(null, null, null, null, null, + Cloud cloud1 = new EC2FleetCloud(null, null, null, null, null, null, null, null, null, null, false, false, 0, 0, 0, 0, false, false, false, 0, 0, false); j.jenkins.clouds.add(cloud1); - Cloud cloud2 = new EC2FleetCloud(null, null, null, null, null, + Cloud cloud2 = new EC2FleetCloud(null, null, null, null, null, null, null, null, null, null, false, false, 0, 0, 0, 0, false, false, false, 0, 0, false); j.jenkins.clouds.add(cloud2); HtmlPage page = j.createWebClient().goTo("configure"); - System.out.println(page); List elementsByName = getElementsByNameWithoutJdk(page, "_.name"); assertEquals(2, elementsByName.size()); @@ -102,13 +144,13 @@ public void shouldShowMultipleCloudsWithDefaultName() throws IOException, SAXExc @Test public void shouldUpdateProperCloudWhenMultiple() throws IOException, SAXException { - EC2FleetCloud cloud1 = new EC2FleetCloud(null, null, null, null, null, + EC2FleetCloud cloud1 = new EC2FleetCloud(null, null, null, null, null, null, null, null, null, null, false, false, 0, 0, 0, 0, false, false, false, 0, 0, false); j.jenkins.clouds.add(cloud1); - EC2FleetCloud cloud2 = new EC2FleetCloud(null, null, null, null, null, + EC2FleetCloud cloud2 = new EC2FleetCloud(null, null, null, null, null, null, null, null, null, null, false, false, 0, 0, 0, 0, false, false, false, 0, 0, false); @@ -127,13 +169,13 @@ public void shouldUpdateProperCloudWhenMultiple() throws IOException, SAXExcepti @Test public void shouldGetFirstWhenMultipleCloudWithSameName() { - EC2FleetCloud cloud1 = new EC2FleetCloud(null, null, null, null, null, + EC2FleetCloud cloud1 = new EC2FleetCloud(null, null, null, null, null, null, null, null, null, null, false, false, 0, 0, 0, 0, false, false, false, 0, 0, false); j.jenkins.clouds.add(cloud1); - EC2FleetCloud cloud2 = new EC2FleetCloud(null, null, null, null, null, + EC2FleetCloud cloud2 = new EC2FleetCloud(null, null, null, null, null, null, null, null, null, null, false, false, 0, 0, 0, 0, false, false, false, 0, 0, false); @@ -144,13 +186,13 @@ public void shouldGetFirstWhenMultipleCloudWithSameName() { @Test public void shouldGetProperWhenMultipleWithDiffName() { - EC2FleetCloud cloud1 = new EC2FleetCloud("a", null, null, null, null, + EC2FleetCloud cloud1 = new EC2FleetCloud("a", null, null, null, null, null, null, null, null, null, false, false, 0, 0, 0, 0, false, false, false, 0, 0, false); j.jenkins.clouds.add(cloud1); - EC2FleetCloud cloud2 = new EC2FleetCloud("b", null, null, null, null, + EC2FleetCloud cloud2 = new EC2FleetCloud("b", null, null, null, null, null, null, null, null, null, false, false, 0, 0, 0, 0, false, false, false, 0, 0, false);