Add backwards compatibility for executor scaling feature (#424)

* Reverted small name changes in comments and documentation

* Reverted remaming of EC2Fleet to Fleet

* Reverted EC2Fleet to Fleet name changes. Changed EC2Fleet to EC2EC2Fleet

* Reverted additional EC2Fleet to Fleet changes

* Reverted EC2Fleet name change in EC2FleetLabelCloudConfigurationAsCodeTest

* Fixed setting of ExcessCapacityTerminationPolicy in modify fleet request

* Added backwards compatability for Executor Scaling feature for EC2FleetCloud

* Revised tests with backwards compatability changes

* Updated CasC documentation

* Updated CloudNanny cloud reaplcement for better testability. Added new test for cloud replacement when no Scaler is present

Author: GavinBurris42

docs/CONFIGURATION-AS-CODE.md

@@ -85,13 +85,32 @@ jenkins:
         numExecutors: 12
         addNodeOnlyIfRunning: true
         restrictUsage: true
-        executorScaler:
-            nodeHardwareScaler:
-              memoryGiBPerExecutor: 2
-              vCpuPerExecutor: 3
+        executorScaler: "noScaler"
         initOnlineTimeoutSec: 181
         initOnlineCheckIntervalSec: 13
         cloudStatusIntervalSec: 11
         disableTaskResubmit: true
         noDelayProvision: true
 ```
+
+### EC2FleetCloud (Node Hardware Scaling)
+
+```yaml
+jenkins:
+  clouds:
+    - eC2Fleet:
+        name: ec2-fleet
+        computerConnector:
+            sshConnector:
+                credentialsId: cred
+                sshHostKeyVerificationStrategy:
+                  NonVerifyingKeyVerificationStrategy
+        region: us-east-2
+        fleet: my-fleet
+        minSize: 1
+        maxSize: 10
+        executorScaler:
+            nodeHardwareScaler:
+              memoryGiBPerExecutor: 2
+              vCpuPerExecutor: 3
+```

src/main/java/com/amazon/jenkins/ec2fleet/CloudNanny.java

@@ -5,6 +5,7 @@
 import hudson.slaves.Cloud;
 import jenkins.model.Jenkins;
 
+import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -51,6 +52,7 @@ protected void doRun() {
             recurrenceCounter.set(fleetCloud.getCloudStatusIntervalSec());
 
             try {
+                updateCloudWithScaler(getClouds(), fleetCloud);
                 // Update the cluster states
                 fleetCloud.update();
             } catch (Exception e) {
@@ -66,10 +68,34 @@ protected void doRun() {
      *
      * @return basic java list
      */
-    private static List<Cloud> getClouds() {
+    private static Jenkins.CloudList getClouds() {
         return Jenkins.get().clouds;
     }
 
+    private void updateCloudWithScaler(Jenkins.CloudList clouds, EC2FleetCloud oldCloud) throws IOException {
+        if(oldCloud.getExecutorScaler() != null) return;
+
+        EC2FleetCloud.ExecutorScaler scaler = oldCloud.isScaleExecutorsByWeight() ? new EC2FleetCloud.WeightedScaler() :
+                                                                                    new EC2FleetCloud.NoScaler();
+        scaler.withNumExecutors(oldCloud.getNumExecutors());
+        EC2FleetCloud fleetCloudWithScaler = createCloudWithScaler(oldCloud, scaler);
+        clouds.replace(oldCloud, fleetCloudWithScaler);
+        Jenkins.get().save();
+    }
+
+    private EC2FleetCloud createCloudWithScaler(EC2FleetCloud oldCloud, EC2FleetCloud.ExecutorScaler scaler) {
+        return  new EC2FleetCloud(oldCloud.getDisplayName(), oldCloud.getAwsCredentialsId(),
+                oldCloud.getAwsCredentialsId(), oldCloud.getRegion(), oldCloud.getEndpoint(), oldCloud.getFleet(),
+                oldCloud.getLabelString(), oldCloud.getFsRoot(), oldCloud.getComputerConnector(),
+                oldCloud.isPrivateIpUsed(), oldCloud.isAlwaysReconnect(), oldCloud.getIdleMinutes(),
+                oldCloud.getMinSize(), oldCloud.getMaxSize(), oldCloud.getMinSpareSize(), oldCloud.getNumExecutors(),
+                oldCloud.isAddNodeOnlyIfRunning(), oldCloud.isRestrictUsage(),
+                String.valueOf(oldCloud.getMaxTotalUses()), oldCloud.isDisableTaskResubmit(),
+                oldCloud.getInitOnlineTimeoutSec(), oldCloud.getInitOnlineCheckIntervalSec(),
+                oldCloud.getCloudStatusIntervalSec(), oldCloud.isNoDelayProvision(),
+                oldCloud.isScaleExecutorsByWeight(), scaler);
+    }
+
     private AtomicInteger getRecurrenceCounter(EC2FleetCloud fleetCloud) {
         AtomicInteger counter = new AtomicInteger(fleetCloud.getCloudStatusIntervalSec());
         // If a counter already exists, return the value, otherwise set the new counter value and return it.

src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java

@@ -119,6 +119,7 @@ public class EC2FleetCloud extends AbstractEC2FleetCloud {
     private final int numExecutors;
     private final boolean addNodeOnlyIfRunning;
     private final boolean restrictUsage;
+    private final boolean scaleExecutorsByWeight;
     private final ExecutorScaler executorScaler;
     private final Integer initOnlineTimeoutSec;
     private final Integer initOnlineCheckIntervalSec;
@@ -183,6 +184,7 @@ public EC2FleetCloud(@Nonnull final String name,
                          final Integer initOnlineCheckIntervalSec,
                          final Integer cloudStatusIntervalSec,
                          final boolean noDelayProvision,
+                         final boolean scaleExecutorsByWeight,
                          final ExecutorScaler executorScaler) {
         super(StringUtils.isNotBlank(name) ? name : CloudNames.generateUnique(BASE_DEFAULT_FLEET_CLOUD_ID));
         init();
@@ -212,6 +214,7 @@ public EC2FleetCloud(@Nonnull final String name,
         this.initOnlineCheckIntervalSec = initOnlineCheckIntervalSec;
         this.cloudStatusIntervalSec = cloudStatusIntervalSec;
         this.noDelayProvision = noDelayProvision;
+        this.scaleExecutorsByWeight = scaleExecutorsByWeight;
         this.executorScaler = executorScaler == null ? new NoScaler().withNumExecutors(this.numExecutors) :
                                                        executorScaler.withNumExecutors(this.numExecutors);
         if (fleet != null) {
@@ -317,6 +320,10 @@ public int getNumExecutors() {
         return numExecutors;
     }
 
+    public boolean isScaleExecutorsByWeight() {
+        return scaleExecutorsByWeight;
+    }
+
     public ExecutorScaler getExecutorScaler() {
         return this.executorScaler;
     }

src/test/java/com/amazon/jenkins/ec2fleet/AutoResubmitIntegrationTest.java

@@ -81,7 +81,7 @@ public void should_successfully_resubmit_freestyle_task() throws Exception {
                 null, "fId", "momo", null, new LocalComputerConnector(j), false, false,
                 0, 0, 10, 0, 1, false, true,
                 "-1", false, 0, 0,
-                10, false, noScaling);
+                10, false, false, noScaling);
         j.jenkins.clouds.add(cloud);
 
         List<QueueTaskFuture> rs = enqueTask(1);
@@ -117,7 +117,7 @@ public void should_successfully_resubmit_parametrized_task() throws Exception {
                 null, "fId", "momo", null, new LocalComputerConnector(j), false, false,
                 0, 0, 10, 0, 1, false, true,
                 "-1", false, 0, 0,
-                10, false, noScaling);
+                10, false, false, noScaling);
         j.jenkins.clouds.add(cloud);
 
         List<QueueTaskFuture> rs = new ArrayList<>();
@@ -172,7 +172,7 @@ public void should_not_resubmit_if_disabled() throws Exception {
         EC2FleetCloud cloud = new EC2FleetCloud("TestCloud", "credId", null, "region",
                 null, "fId", "momo", null, new LocalComputerConnector(j), false, false,
                 0, 0, 10, 0, 1, false, true,
-                "-1", true, 0, 0, 10, false, noScaling);
+                "-1", true, 0, 0, 10, false, false, noScaling);
         j.jenkins.clouds.add(cloud);
 
         List<QueueTaskFuture> rs = enqueTask(1);

src/test/java/com/amazon/jenkins/ec2fleet/CloudNamesTest.java

@@ -17,7 +17,7 @@ public void isUnique_true() {
             "test-label", null, null, false, false,
             0, 0, 0, 0, 0, true, false,
             "-1", false, 0, 0,
-            10, false, noScaling));
+            10, false, false, noScaling));
 
     Assert.assertTrue(CloudNames.isUnique("TestCloud"));
   }
@@ -28,7 +28,7 @@ public void isUnique_false() {
             "test-label", null, null, false, false,
             0, 0, 0, 0, 0, true, false,
             "-1", false, 0, 0,
-            10, false, noScaling));
+            10, false, false, noScaling));
 
     Assert.assertFalse(CloudNames.isUnique("SomeDefaultName"));
   }
@@ -39,13 +39,13 @@ public void isDuplicated_false() {
         "test-label", null, null, false, false,
         0, 0, 0, 0, 0, true, false,
         "-1", false, 0, 0,
-        10, false, noScaling));
+        10, false, false, noScaling));
 
     j.jenkins.clouds.add(new EC2FleetCloud("TestCloud2", null, null, null, null, null,
         "test-label", null, null, false, false,
         0, 0, 0, 0, 0, true, false,
         "-1", false, 0, 0,
-        10, false, noScaling));
+        10, false, false, noScaling));
 
     Assert.assertFalse(CloudNames.isDuplicated("TestCloud"));
   }
@@ -56,13 +56,13 @@ public void isDuplicated_true() {
         "test-label", null, null, false, false,
         0, 0, 0, 0, 0, true, false,
         "-1", false, 0, 0,
-        10, false, noScaling));
+        10, false, false, noScaling));
 
     j.jenkins.clouds.add(new EC2FleetCloud("TestCloud", null, null, null, null, null,
         "test-label", null, null, false, false,
         0, 0, 0, 0, 0, true, false,
         "-1", false, 0, 0,
-        10, false, noScaling));
+        10, false, false, noScaling));
 
     Assert.assertTrue(CloudNames.isDuplicated("TestCloud"));
   }
@@ -78,7 +78,7 @@ public void generateUnique_addsSuffixOnlyWhenNeeded() {
             "test-label", null, null, false, false,
             0, 0, 0, 0, 0, true, false,
             "-1", false, 0, 0,
-            10, false, noScaling));
+            10, false, false, noScaling));
 
     Assert.assertEquals("UniqueCloud", CloudNames.generateUnique("UniqueCloud"));
   }
@@ -89,13 +89,13 @@ public void generateUnique_addsSuffixCorrectly() {
             "test-label", null, null, false, false,
             0, 0, 0, 0, 0, true, false,
             "-1", false, 0, 0,
-            10, false, noScaling));
+            10, false, false, noScaling));
 
     j.jenkins.clouds.add(new EC2FleetCloud("UniqueCloud-1", null, null, null, null, null,
             "test-label", null, null, false, false,
             0, 0, 0, 0, 0, true, false,
             "-1", false, 0, 0,
-            10, false, noScaling));
+            10, false, false, noScaling));
 
     String actual = CloudNames.generateUnique("UniqueCloud");
     Assert.assertTrue(actual.length() == ("UniqueCloud".length() + CloudNames.SUFFIX_LENGTH + 1));
@@ -108,7 +108,7 @@ public void generateUnique_emptyStringInConstructor() {
             "test-label", null, null, false, false,
             0, 0, 0, 0, 0, true, false,
             "-1", false, 0, 0,
-            10, false, noScaling);
+            10, false, false, noScaling);
 
     EC2FleetLabelCloud fleetLabelCloud = new EC2FleetLabelCloud("", null, null,
             null, null, new LocalComputerConnector(j), false, false,
@@ -128,7 +128,7 @@ public void generateUnique_nonEmptyStringInConstructor() {
             "test-label", null, null, false, false,
             0, 0, 0, 0, 0, true, false,
             "-1", false, 0, 0,
-            10, false, noScaling);
+            10, false, false, noScaling);
 
     EC2FleetLabelCloud fleetLabelCloud = new EC2FleetLabelCloud("UniqueLabelCloud", null, null,
             null, null, new LocalComputerConnector(j), false, false,

src/test/java/com/amazon/jenkins/ec2fleet/CloudNannyTest.java

@@ -1,6 +1,9 @@
 package com.amazon.jenkins.ec2fleet;
 
+import com.amazon.jenkins.ec2fleet.fleet.EC2Fleet;
+import com.amazon.jenkins.ec2fleet.fleet.EC2Fleets;
 import hudson.slaves.Cloud;
+import jenkins.model.Jenkins;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -10,32 +13,30 @@
 import org.powermock.modules.junit4.PowerMockRunner;
 import org.powermock.reflect.Whitebox;
 
-import java.util.ArrayList;
 import java.util.Collections;
-import java.util.List;
 import java.util.Map;
 import java.util.WeakHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.junit.Assert.assertEquals;
-import static org.mockito.Mockito.atLeastOnce;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyNoMoreInteractions;
-import static org.mockito.Mockito.verifyZeroInteractions;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 @RunWith(PowerMockRunner.class)
-@PrepareForTest(CloudNanny.class)
+@PrepareForTest({CloudNanny.class, Jenkins.class, EC2Fleets.class})
 public class CloudNannyTest {
+    @Mock
+    private Jenkins jenkins;
+
+    @Mock
+    private EC2Fleet ec2Fleet;
 
     @Mock
     private EC2FleetCloud cloud1;
 
     @Mock
     private EC2FleetCloud cloud2;
 
-    private List<Cloud> clouds = new ArrayList<>();
+    private Jenkins.CloudList clouds = new Jenkins.CloudList();
 
     private FleetStateStats stats1 = new FleetStateStats(
             "f1", 1, new FleetStateStats.State(true, false, "a"), Collections.emptySet(), Collections.<String, Double>emptyMap());
@@ -55,6 +56,15 @@ public void before() throws Exception {
         PowerMockito.mockStatic(CloudNanny.class);
         PowerMockito.when(CloudNanny.class, "getClouds").thenReturn(clouds);
 
+        PowerMockito.mockStatic(Jenkins.class);
+        PowerMockito.when(Jenkins.get()).thenReturn(jenkins);
+
+        PowerMockito.mockStatic(EC2Fleets.class);
+        when(EC2Fleets.get(anyString())).thenReturn(ec2Fleet);
+        PowerMockito.when(ec2Fleet.getState(anyString(), anyString(), anyString(), anyString()))
+                .thenReturn(new FleetStateStats("", 0, FleetStateStats.State.active(),
+                        Collections.<String>emptySet(), Collections.<String, Double>emptyMap()));
+
         when(cloud1.getLabelString()).thenReturn("a");
         when(cloud2.getLabelString()).thenReturn("");
         when(cloud1.getFleet()).thenReturn("f1");
@@ -187,4 +197,22 @@ public void updateOnlyOneCloud() {
         assertEquals(1, recurrenceCounter1.get());
         assertEquals(cloud2.getCloudStatusIntervalSec(), recurrenceCounter2.get());
     }
+
+    @Test
+    public void doRun_updatesCloudsWithScaler_whenScalerIsNull() {
+        when(cloud1.isScaleExecutorsByWeight()).thenReturn(true);
+        when(cloud2.isScaleExecutorsByWeight()).thenReturn(false);
+
+        clouds.add(cloud1);
+        clouds.add(cloud2);
+        CloudNanny cloudNanny = getMockCloudNannyInstance();
+
+        cloudNanny.doRun();
+
+        cloud1 = (EC2FleetCloud) clouds.get(0);
+        cloud2 = (EC2FleetCloud) clouds.get(1);
+
+        assertEquals(EC2FleetCloud.WeightedScaler.class, cloud1.getExecutorScaler().getClass());
+        assertEquals(EC2FleetCloud.NoScaler.class, cloud2.getExecutorScaler().getClass());
+    }
 }