[JENKINS-56839] Allow disabling T2/T3 Unlimited Mode for T3 instances

pom.xml

@@ -129,6 +129,17 @@ THE SOFTWARE.
             <version>2.2</version>
             <scope>provided</scope><!-- comes from the core -->
         </dependency>
+        <dependency>
+            <!-- During testing, WireMock pulls in Guava 27. Jenkins provides Guava 11, and doesn't work with Guava
+                 27. WireMock doesn't work with Guava 11. Guava 20 works for both WireMock and Jenkins, so use that.
+
+                 NB: In production, we use Guava 11, as provided by Jenkins, as WireMock is only used during testing.
+            -->
+            <groupId>com.google.guava</groupId>
+            <artifactId>guava</artifactId>
+            <version>20.0</version>
+            <scope>provided</scope>
+        </dependency>
         <dependency>
             <groupId>org.powermock</groupId>
             <artifactId>powermock-module-junit4</artifactId>
@@ -156,6 +167,12 @@ THE SOFTWARE.
                 </exclusion>
             </exclusions>
         </dependency>
+        <dependency>
+            <groupId>com.github.tomakehurst</groupId>
+            <artifactId>wiremock-jre8</artifactId>
+            <version>2.23.2</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-cps</artifactId>
@@ -394,6 +411,30 @@ THE SOFTWARE.
                     <excludeFilterFile>spotbugs-exclude.xml</excludeFilterFile>
                 </configuration>
             </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-enforcer-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>display-info</id>
+                        <configuration>
+                            <rules>
+                                <requireUpperBoundDeps>
+                                    <excludes combine.children="append">
+                                        <!-- WireMock requires more recent versions of these three artifacts, but the
+                                             older versions we actually pull in are API-compatible, so ignore this
+                                             issue:
+                                        -->
+                                        <exclude>org.apache.commons:commons-lang3</exclude>
+                                        <exclude>org.apache.httpcomponents:httpclient</exclude>
+                                        <exclude>org.eclipse.jetty:jetty-security</exclude>
+                                    </excludes>
+                                </requireUpperBoundDeps>
+                            </rules>
+                        </configuration>
+                    </execution>
+                </executions>
+            </plugin>
         </plugins>
     </build>
 </project>

src/main/java/hudson/plugins/ec2/AmazonEC2Cloud.java

@@ -67,6 +67,8 @@ public class AmazonEC2Cloud extends EC2Cloud {
 
     // Used when running unit tests
     private static boolean testMode = false;
+    private static String ec2TestEndpointUrl;
+
     private boolean noDelayProvisioning;
 
     @DataBoundConstructor
@@ -100,6 +102,13 @@ public String getRegion() {
 
     public static URL getEc2EndpointUrl(String region) {
         try {
+            if (isTestMode()) {
+                String testEndpointUrl = getEc2TestEndpointUrl();
+                if (testEndpointUrl != null) {
+                    return new URL(testEndpointUrl);
+                }
+            }
+
             return new URL("https://ec2." + region + "." + AWS_URL_HOST + "/");
         } catch (MalformedURLException e) {
             throw new Error(e); // Impossible
@@ -142,6 +151,14 @@ public static boolean isTestMode() {
         return testMode;
     }
 
+    public static void setEc2TestEndpointUrl(String ec2TestEndpointUrl) {
+        AmazonEC2Cloud.ec2TestEndpointUrl = ec2TestEndpointUrl;
+    }
+
+    public static String getEc2TestEndpointUrl() {
+        return ec2TestEndpointUrl;
+    }
+
     @Extension
     public static class DescriptorImpl extends EC2Cloud.DescriptorImpl {
 

src/main/java/hudson/plugins/ec2/SlaveTemplate.java

@@ -86,8 +86,14 @@ public class SlaveTemplate implements Describable<SlaveTemplate> {
 
     public final boolean monitoring;
 
+    /**
+     * @deprecated Replaced by {@link #burstableUnlimitedMode}.
+     */
+    @Deprecated
     public final boolean t2Unlimited;
 
+    private BurstableUnlimitedMode burstableUnlimitedMode;
+
     public final String labels;
 
     public final Node.Mode mode;
@@ -175,7 +181,8 @@ public SlaveTemplate(String ami, String zone, SpotConfiguration spotConfig, Stri
             String instanceCapStr, String iamInstanceProfile, boolean deleteRootOnTermination,
             boolean useEphemeralDevices, boolean useDedicatedTenancy, String launchTimeoutStr, boolean associatePublicIp,
             String customDeviceMapping, boolean connectBySSHProcess, boolean monitoring,
-            boolean t2Unlimited, ConnectionStrategy connectionStrategy, int maxTotalUses) {
+            boolean t2Unlimited, BurstableUnlimitedMode burstableUnlimitedMode, ConnectionStrategy connectionStrategy,
+            int maxTotalUses) {
 
         if(StringUtils.isNotBlank(remoteAdmin) || StringUtils.isNotBlank(jvmopts) || StringUtils.isNotBlank(tmpDir)){
             LOGGER.log(Level.FINE, "As remoteAdmin, jvmopts or tmpDir is not blank, we must ensure the user has RUN_SCRIPTS rights.");
@@ -234,10 +241,27 @@ public SlaveTemplate(String ami, String zone, SpotConfiguration spotConfig, Stri
         this.useEphemeralDevices = useEphemeralDevices;
         this.customDeviceMapping = customDeviceMapping;
         this.t2Unlimited = t2Unlimited;
+        this.burstableUnlimitedMode = burstableUnlimitedMode;
 
         readResolve(); // initialize
     }
 
+    @Deprecated
+    public SlaveTemplate(String ami, String zone, SpotConfiguration spotConfig, String securityGroups, String remoteFS,
+            InstanceType type, boolean ebsOptimized, String labelString, Node.Mode mode, String description, String initScript,
+            String tmpDir, String userData, String numExecutors, String remoteAdmin, AMITypeData amiType, String jvmopts,
+            boolean stopOnTerminate, String subnetId, List<EC2Tag> tags, String idleTerminationMinutes,
+            String instanceCapStr, String iamInstanceProfile, boolean deleteRootOnTermination,
+            boolean useEphemeralDevices, boolean useDedicatedTenancy, String launchTimeoutStr, boolean associatePublicIp,
+            String customDeviceMapping, boolean connectBySSHProcess, boolean monitoring,
+            boolean t2Unlimited, ConnectionStrategy connectionStrategy, int maxTotalUses) {
+        this(ami, zone, spotConfig, securityGroups, remoteFS, type, ebsOptimized, labelString, mode, description,
+                initScript, tmpDir, userData, numExecutors, remoteAdmin, amiType, jvmopts, stopOnTerminate, subnetId,
+                tags, idleTerminationMinutes, instanceCapStr, iamInstanceProfile, deleteRootOnTermination,
+                useEphemeralDevices, useDedicatedTenancy, launchTimeoutStr, associatePublicIp, customDeviceMapping,
+                connectBySSHProcess, monitoring, t2Unlimited, null, connectionStrategy, maxTotalUses);
+    }
+
     @Deprecated
     public SlaveTemplate(String ami, String zone, SpotConfiguration spotConfig, String securityGroups, String remoteFS,
             InstanceType type, boolean ebsOptimized, String labelString, Node.Mode mode, String description, String initScript,
@@ -318,6 +342,10 @@ public EC2Cloud getParent() {
         return parent;
     }
 
+    public BurstableUnlimitedMode getBurstableUnlimitedMode() {
+        return burstableUnlimitedMode;
+    }
+
     public String getLabelString() {
         return labels;
     }
@@ -599,9 +627,10 @@ private List<EC2AbstractSlave> provisionOndemand(int number, EnumSet<ProvisionOp
         riRequest.setEbsOptimized(ebsOptimized);
         riRequest.setMonitoring(monitoring);
 
-        if (t2Unlimited){
+        BurstableUnlimitedMode burstableUnlimitedMode = getBurstableUnlimitedMode();
+        if (burstableUnlimitedMode != BurstableUnlimitedMode.DEFAULT) {
             CreditSpecificationRequest creditRequest = new CreditSpecificationRequest();
-            creditRequest.setCpuCredits("unlimited");
+            creditRequest.setCpuCredits(burstableUnlimitedMode == BurstableUnlimitedMode.ENABLED ? "unlimited" : "standard");
             riRequest.setCreditSpecification(creditRequest);
         }
 
@@ -1246,6 +1275,11 @@ protected Object readResolve() {
             maxTotalUses = -1;
         }
 
+        // Migrate old t2Unlimited setting:
+        if (burstableUnlimitedMode == null) {
+            burstableUnlimitedMode = t2Unlimited ? BurstableUnlimitedMode.ENABLED : BurstableUnlimitedMode.DEFAULT;
+        }
+
         return this;
     }
 
@@ -1281,6 +1315,18 @@ public boolean isUseHTTPS() {
         return amiType.isWindows() && ((WindowsData) amiType).isUseHTTPS();
     }
 
+    public enum BurstableUnlimitedMode {
+        DEFAULT(Messages.SlaveTemplate_BurstableUnlimitedMode_DEFAULT()),
+        DISABLED(Messages.SlaveTemplate_BurstableUnlimitedMode_DISABLED()),
+        ENABLED(Messages.SlaveTemplate_BurstableUnlimitedMode_ENABLED());
+
+        public final String label;
+
+        BurstableUnlimitedMode(String label) {
+            this.label = label;
+        }
+    }
+
     @Extension
     public static final class DescriptorImpl extends Descriptor<SlaveTemplate> {
 
@@ -1608,5 +1654,4 @@ public FormValidation doCheckConnectionStrategy(@QueryParameter String connectio
                     .orElse(FormValidation.error("Could not find selected connection strategy"));
         }
     }
-
-}
+}

src/main/resources/hudson/plugins/ec2/Messages.properties

@@ -11,3 +11,7 @@ EC2SpotSlave.Spot2= max bid price
 AmazonEC2Cloud.NonUniqName=Cloud name must be unique across EC2 clouds
 
 General.MissingPermission=You do not have the Overall/RunScripts right to modify this field
+
+SlaveTemplate.BurstableUnlimitedMode.DEFAULT=Instance Type Default
+SlaveTemplate.BurstableUnlimitedMode.DISABLED=Disabled
+SlaveTemplate.BurstableUnlimitedMode.ENABLED=Enabled

src/main/resources/hudson/plugins/ec2/SlaveTemplate/config.jelly

@@ -47,8 +47,8 @@ THE SOFTWARE.
     <f:checkbox />
   </f:entry>
 
-  <f:entry title="${%T2 Unlimited}" field="t2Unlimited">
-    <f:checkbox />
+  <f:entry title="${%T2/T3 Unlimited}" field="burstableUnlimitedMode">
+    <f:enum>${it.label}</f:enum>
   </f:entry>
 
   <f:entry title="${%Availability Zone}" field="zone">

src/main/resources/hudson/plugins/ec2/SlaveTemplate/help-burstableUnlimitedMode.html

@@ -0,0 +1,12 @@
+<div>
+    <p>
+        Choose whether to enable
+        <a href="https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/burstable-performance-instances-unlimited-mode.html"
+            >Unlimited Mode for Burstable Performance Instances</a> (T2, T3, and T3a).
+    </p>
+    <p>Unlimited Mode can incur additional charges.</p>
+    <p>
+        <em>Instance Type Default</em> will use the AWS default for the chosen Instance Type: T2 instances by default
+        launch with Unlimited Mode disabled; T3 and T3a instances by default launch with Unlimited Mode enabled.
+    </p>
+</div>

src/test/java/hudson/plugins/ec2/ConfigurationAsCodeTest.java

@@ -124,6 +124,87 @@ public void testBackwardsCompatibleConnectionStrategy() throws Exception {
         assertEquals(ConnectionStrategy.PRIVATE_DNS,slaveTemplate.connectionStrategy);
     }
 
+    @Test
+    @ConfiguredWithCode("BackwardCompatibleT2UnlimitedDefault.yml")
+    public void testBackwardCompatibleT2UnlimitedDefault() throws Exception {
+        // Not setting t2Unlimited in the JCasC config file should yield BurstableUnlimitedMode.DEFAULT
+        // (since the default of t2Unlimited==false used to _not_ send any Unlimited Mode preference to AWS):
+        SlaveTemplate.BurstableUnlimitedMode expectedBurstableUnlimitedMode = SlaveTemplate.BurstableUnlimitedMode.DEFAULT;
+
+        final AmazonEC2Cloud ec2Cloud = (AmazonEC2Cloud) Jenkins.get().getCloud("ec2-us-east-1");
+        assertNotNull(ec2Cloud);
+
+        final List<SlaveTemplate> templates = ec2Cloud.getTemplates();
+        assertEquals(1, templates.size());
+
+        final SlaveTemplate slaveTemplate = templates.get(0);
+        assertEquals(expectedBurstableUnlimitedMode, slaveTemplate.getBurstableUnlimitedMode());
+    }
+
+    @Test
+    @ConfiguredWithCode("BackwardCompatibleT2UnlimitedEnabled.yml")
+    public void testBackwardCompatibleT2UnlimitedEnabled() throws Exception {
+        // t2Unlimited=true in the JCasC config file should yield BurstableUnlimitedMode.ENABLED:
+        SlaveTemplate.BurstableUnlimitedMode expectedBurstableUnlimitedMode = SlaveTemplate.BurstableUnlimitedMode.ENABLED;
+
+        final AmazonEC2Cloud ec2Cloud = (AmazonEC2Cloud) Jenkins.get().getCloud("ec2-us-east-1");
+        assertNotNull(ec2Cloud);
+
+        final List<SlaveTemplate> templates = ec2Cloud.getTemplates();
+        assertEquals(1, templates.size());
+
+        final SlaveTemplate slaveTemplate = templates.get(0);
+        assertEquals(expectedBurstableUnlimitedMode, slaveTemplate.getBurstableUnlimitedMode());
+    }
+
+    @Test
+    @ConfiguredWithCode("BackwardCompatibleT2UnlimitedDisabled.yml")
+    public void testBackwardCompatibleT2UnlimitedDisabled() throws Exception {
+        // t2Unlimited=false in the JCasC config file should yield BurstableUnlimitedMode.DEFAULT
+        // (since t2Unlimited==false used to _not_ send any Unlimited Mode preference to AWS):
+        SlaveTemplate.BurstableUnlimitedMode expectedBurstableUnlimitedMode = SlaveTemplate.BurstableUnlimitedMode.DEFAULT;
+
+        final AmazonEC2Cloud ec2Cloud = (AmazonEC2Cloud) Jenkins.get().getCloud("ec2-us-east-1");
+        assertNotNull(ec2Cloud);
+
+        final List<SlaveTemplate> templates = ec2Cloud.getTemplates();
+        assertEquals(1, templates.size());
+
+        final SlaveTemplate slaveTemplate = templates.get(0);
+        assertEquals(expectedBurstableUnlimitedMode, slaveTemplate.getBurstableUnlimitedMode());
+    }
+
+    @Test
+    @ConfiguredWithCode("BurstableUnlimitedMode.yml")
+    public void testBurstableUnlimitedMode() throws Exception {
+        final AmazonEC2Cloud ec2Cloud = (AmazonEC2Cloud) Jenkins.get().getCloud("ec2-us-east-1");
+        assertNotNull(ec2Cloud);
+
+        final List<SlaveTemplate> templates = ec2Cloud.getTemplates();
+        assertEquals(3, templates.size());
+
+        assertEquals(SlaveTemplate.BurstableUnlimitedMode.DEFAULT, templates.get(0).getBurstableUnlimitedMode());
+        assertEquals(SlaveTemplate.BurstableUnlimitedMode.ENABLED, templates.get(1).getBurstableUnlimitedMode());
+        assertEquals(SlaveTemplate.BurstableUnlimitedMode.DISABLED, templates.get(2).getBurstableUnlimitedMode());
+    }
+
+    @Test
+    @ConfiguredWithCode("BurstableUnlimitedModeOverride.yml")
+    public void testBurstableUnlimitedModeOverride() throws Exception {
+        // Setting burstableUnlimitedMode in the JCasC file should override any t2Unlimited setting that is also
+        // present.
+        final AmazonEC2Cloud ec2Cloud = (AmazonEC2Cloud) Jenkins.get().getCloud("ec2-us-east-1");
+        assertNotNull(ec2Cloud);
+
+        final List<SlaveTemplate> templates = ec2Cloud.getTemplates();
+        assertEquals(3, templates.size());
+
+        assertEquals(SlaveTemplate.BurstableUnlimitedMode.DEFAULT, templates.get(0).getBurstableUnlimitedMode());
+        assertEquals(SlaveTemplate.BurstableUnlimitedMode.ENABLED, templates.get(1).getBurstableUnlimitedMode());
+        assertEquals(SlaveTemplate.BurstableUnlimitedMode.DISABLED, templates.get(2).getBurstableUnlimitedMode());
+    }
+
+
     @Test
     @ConfiguredWithCode("UnixData.yml")
     public void testConfigAsCodeExport() throws Exception {