Refactoring for clarity (jenkinsci#381)

* [refactor] Added class comments for Jenkins extension points

[refactor] Use Computer subtype in EC2RetentionStrategy

* [refactor] Moving AWS files into their own package

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

@@ -14,7 +14,7 @@
 import java.util.logging.Logger;
 
 /**
- * @see EC2FleetCloud
+ * {@link CloudNanny} is responsible for periodically running update (i.e. sync-state-with-AWS) cycles for {@link EC2FleetCloud}s.
  */
 @Extension
 @SuppressWarnings("unused")

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

@@ -22,15 +22,16 @@
 import java.util.logging.Logger;
 
 /**
+ * The {@link EC2FleetAutoResubmitComputerLauncher} is responsible for controlling:
+ *  * how {@link EC2FleetNodeComputer}s are launched
+ *  * how {@link EC2FleetNodeComputer}s connect to agents {@link EC2FleetNode}
+ *
  * This is wrapper for {@link ComputerLauncher} to get notification when slave was disconnected
  * and automatically resubmit {@link hudson.model.Queue.Task} if reason is unexpected termination
  * which usually means EC2 instance was interrupted.
  * <p>
  * This is optional feature, it's enabled by default, but could be disabled by
  * {@link EC2FleetCloud#isDisableTaskResubmit()}
- *
- * @see EC2FleetNode
- * @see EC2FleetNodeComputer
  */
 @SuppressWarnings("WeakerAccess")
 @ThreadSafe

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

@@ -1,10 +1,10 @@
 package com.amazon.jenkins.ec2fleet;
 
+import com.amazon.jenkins.ec2fleet.aws.AwsPermissionChecker;
+import com.amazon.jenkins.ec2fleet.aws.RegionHelper;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleet;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleets;
-import com.amazon.jenkins.ec2fleet.utils.AwsPermissionChecker;
 import com.amazon.jenkins.ec2fleet.utils.EC2FleetCloudAwareUtils;
-import com.amazon.jenkins.ec2fleet.utils.RegionHelper;
 import com.amazonaws.services.ec2.AmazonEC2;
 import com.amazonaws.services.ec2.model.Instance;
 import com.amazonaws.services.ec2.model.InstanceStateName;
@@ -55,15 +55,24 @@
 import java.util.stream.Collectors;
 
 /**
- * @see CloudNanny
+ * The {@link EC2FleetCloud} contains the main configuration values used while creating Jenkins nodes.
+ * EC2FleetCloud can represent either an AWS EC2 Spot Fleet or an AWS AutoScalingGroup.
+ *
+ * Responsibilities include:
+ *  * maintain the configuration values set by user for the Cloud.
+ *      User-initiated changes to such configuration will result in a new EC2FleetCloud object. See {@link Cloud}.
+ *  * provision new EC2 capacity along with supporting provisioning checks for EC2FleetCloud
+ *  * keep {@link FleetStateStats} in sync with state on the AWS side via {@link EC2FleetCloud#update()}
+ *  * provide helper methods to extract certain state values
+ *  * provide helper methods to schedule termination of compute / nodes
  */
 @SuppressWarnings({"unused", "WeakerAccess"})
 public class EC2FleetCloud extends AbstractEC2FleetCloud {
 
     public static final String EC2_INSTANCE_TAG_NAMESPACE = "ec2-fleet-plugin";
     public static final String EC2_INSTANCE_CLOUD_NAME_TAG = EC2_INSTANCE_TAG_NAMESPACE + ":cloud-name";
 
-    public static final String FLEET_CLOUD_ID = "FleetCloud";
+    public static final String DEFAULT_FLEET_CLOUD_ID = "FleetCloud";
 
     public static final int DEFAULT_CLOUD_STATUS_INTERVAL_SEC = 10;
 
@@ -195,7 +204,7 @@ public EC2FleetCloud(final String name,
                          final boolean scaleExecutorsByWeight,
                          final Integer cloudStatusIntervalSec,
                          final boolean noDelayProvision) {
-        super(StringUtils.isBlank(name) ? FLEET_CLOUD_ID : name);
+        super(StringUtils.isBlank(name) ? DEFAULT_FLEET_CLOUD_ID : name);
         init();
         this.credentialsId = credentialsId;
         this.awsCredentialsId = awsCredentialsId;
@@ -238,6 +247,8 @@ public boolean isNoDelayProvision() {
     }
 
     /**
+     * Deprecated.Use {@link EC2FleetCloud#awsCredentialsId}
+     *
      * See {@link EC2FleetCloud#awsCredentialsId} documentation. Don't use fields directly to be able
      * get old version of plugin and for new.
      *

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

@@ -1,11 +1,12 @@
 package com.amazon.jenkins.ec2fleet;
 
+import com.amazon.jenkins.ec2fleet.aws.AwsPermissionChecker;
+import com.amazon.jenkins.ec2fleet.aws.CloudFormationApi;
+import com.amazon.jenkins.ec2fleet.aws.EC2Api;
+import com.amazon.jenkins.ec2fleet.aws.RegionHelper;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleets;
 import com.amazon.jenkins.ec2fleet.fleet.EC2SpotFleet;
-import com.amazon.jenkins.ec2fleet.utils.AwsPermissionChecker;
 import com.amazon.jenkins.ec2fleet.utils.EC2FleetCloudAwareUtils;
-import com.amazon.jenkins.ec2fleet.utils.JenkinsUtils;
-import com.amazon.jenkins.ec2fleet.utils.RegionHelper;
 import com.amazonaws.services.cloudformation.AmazonCloudFormation;
 import com.amazonaws.services.cloudformation.model.StackStatus;
 import com.amazonaws.services.ec2.AmazonEC2;
@@ -62,7 +63,7 @@ public class EC2FleetLabelCloud extends AbstractEC2FleetCloud {
     public static final String EC2_INSTANCE_TAG_NAMESPACE = "ec2-fleet-plugin";
     public static final String EC2_INSTANCE_CLOUD_NAME_TAG = EC2_INSTANCE_TAG_NAMESPACE + ":cloud-name";
 
-    public static final String FLEET_CLOUD_ID = "FleetCloudLabel";
+    public static final String DEFAULT_FLEET_CLOUD_ID = "FleetCloudLabel";
 
     public static final int DEFAULT_CLOUD_STATUS_INTERVAL_SEC = 10;
 
@@ -141,7 +142,7 @@ public EC2FleetLabelCloud(final String name,
                               final Integer cloudStatusIntervalSec,
                               final boolean noDelayProvision,
                               final String ec2KeyPairName) {
-        super(StringUtils.isBlank(name) ? FLEET_CLOUD_ID : name);
+        super(StringUtils.isBlank(name) ? DEFAULT_FLEET_CLOUD_ID : name);
         init();
         this.awsCredentialsId = awsCredentialsId;
         this.region = region;

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

@@ -14,6 +14,9 @@
 import java.io.IOException;
 import java.util.List;
 
+/**
+ * The {@link EC2FleetNode} represents an agent running on an EC2 instance, responsible for creating {@link EC2FleetNodeComputer}.
+ */
 public class EC2FleetNode extends Slave implements EphemeralNode, EC2FleetCloudAware {
 
     private volatile AbstractEC2FleetCloud cloud;

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

@@ -10,8 +10,8 @@
 import java.io.IOException;
 
 /**
- * @see EC2FleetNode
- * @see EC2FleetAutoResubmitComputerLauncher
+ * The {@link EC2FleetNodeComputer} represents the running state of {@link EC2FleetNode} that holds executors.
+ * @see hudson.model.Computer
  */
 @ThreadSafe
 public class EC2FleetNodeComputer extends SlaveComputer implements EC2FleetCloudAware {

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

@@ -14,29 +14,26 @@
 import java.util.logging.Logger;
 
 /**
- * @see EC2FleetCloud
+ * The {@link EC2RetentionStrategy} controls when to take {@link EC2FleetNodeComputer} offline, bring it back online, or even to destroy it.
  */
-public class EC2RetentionStrategy extends RetentionStrategy<SlaveComputer> implements ExecutorListener {
-
-    private static final int RE_CHECK_IN_A_MINUTE = 1;
-
+public class EC2RetentionStrategy extends RetentionStrategy<EC2FleetNodeComputer> implements ExecutorListener {
     private static final Logger LOGGER = Logger.getLogger(EC2RetentionStrategy.class.getName());
+    private static final int RE_CHECK_IN_A_MINUTE = 1;
 
     /**
      * Will be called under {@link hudson.model.Queue#withLock(Runnable)}
      *
-     * @param computer computer
+     * @param fc EC2FleetNodeComputer
      * @return delay in min before next run
      */
     @SuppressFBWarnings(
             value = "BC_UNCONFIRMED_CAST",
             justification = "to ignore EC2FleetNodeComputer cast")
     @Override
-    public long check(final SlaveComputer computer) {
-        final EC2FleetNodeComputer fc = (EC2FleetNodeComputer) computer;
+    public long check(final EC2FleetNodeComputer fc) {
         final AbstractEC2FleetCloud cloud = fc.getCloud();
 
-        LOGGER.fine(String.format("Checking if node '%s' is idle ", computer.getName()));
+        LOGGER.fine(String.format("Checking if node '%s' is idle ", fc.getName()));
 
         // in some multi-thread edge cases cloud could be null for some time, just be ok with that
         if (cloud == null) {
@@ -101,7 +98,7 @@ public long check(final SlaveComputer computer) {
     }
 
     @Override
-    public void start(SlaveComputer c) {
+    public void start(EC2FleetNodeComputer c) {
         LOGGER.log(Level.INFO, "Connecting to instance: " + c.getDisplayName());
         c.connect(false);
     }

src/main/java/com/amazon/jenkins/ec2fleet/utils/JenkinsUtils.java → src/main/java/com/amazon/jenkins/ec2fleet/JenkinsUtils.java

@@ -1,4 +1,4 @@
-package com.amazon.jenkins.ec2fleet.utils;
+package com.amazon.jenkins.ec2fleet;
 
 import hudson.model.Node;
 import jenkins.model.Jenkins;

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

@@ -1,11 +1,12 @@
 package com.amazon.jenkins.ec2fleet;
 
+import com.amazon.jenkins.ec2fleet.aws.CloudFormationApi;
+import com.amazon.jenkins.ec2fleet.aws.EC2Api;
+
 /**
  * Decouple plugin code from dependencies for easy testing. We cannot just make transient fields
  * in required classes as they usually restored by Jenkins without constructor call. Instead
  * we are using registry pattern.
- *
- * @see EC2FleetCloud
  */
 @SuppressWarnings("WeakerAccess")
 public class Registry {

src/main/java/com/amazon/jenkins/ec2fleet/utils/AWSUtils.java → src/main/java/com/amazon/jenkins/ec2fleet/aws/AWSUtils.java

@@ -1,4 +1,4 @@
-package com.amazon.jenkins.ec2fleet.utils;
+package com.amazon.jenkins.ec2fleet.aws;
 
 import com.amazonaws.ClientConfiguration;
 import com.amazonaws.retry.PredefinedRetryPolicies;

src/main/java/com/amazon/jenkins/ec2fleet/utils/AwsPermissionChecker.java → src/main/java/com/amazon/jenkins/ec2fleet/aws/AwsPermissionChecker.java

@@ -1,4 +1,4 @@
-package com.amazon.jenkins.ec2fleet.utils;
+package com.amazon.jenkins.ec2fleet.aws;
 
 import com.amazon.jenkins.ec2fleet.Registry;
 import com.amazon.jenkins.ec2fleet.fleet.AutoScalingGroupFleet;

src/main/java/com/amazon/jenkins/ec2fleet/CloudFormationApi.java → src/main/java/com/amazon/jenkins/ec2fleet/aws/CloudFormationApi.java

@@ -1,6 +1,6 @@
-package com.amazon.jenkins.ec2fleet;
+package com.amazon.jenkins.ec2fleet.aws;
 
-import com.amazon.jenkins.ec2fleet.utils.AWSUtils;
+import com.amazon.jenkins.ec2fleet.EC2FleetLabelParameters;
 import com.amazonaws.ClientConfiguration;
 import com.amazonaws.regions.Region;
 import com.amazonaws.regions.RegionUtils;

src/main/java/com/amazon/jenkins/ec2fleet/EC2Api.java → src/main/java/com/amazon/jenkins/ec2fleet/aws/EC2Api.java

@@ -1,6 +1,5 @@
-package com.amazon.jenkins.ec2fleet;
+package com.amazon.jenkins.ec2fleet.aws;
 
-import com.amazon.jenkins.ec2fleet.utils.AWSUtils;
 import com.amazonaws.ClientConfiguration;
 import com.amazonaws.regions.Region;
 import com.amazonaws.regions.RegionUtils;

src/main/java/com/amazon/jenkins/ec2fleet/utils/RegionHelper.java → src/main/java/com/amazon/jenkins/ec2fleet/aws/RegionHelper.java

@@ -1,4 +1,4 @@
-package com.amazon.jenkins.ec2fleet.utils;
+package com.amazon.jenkins.ec2fleet.aws;
 
 import com.amazon.jenkins.ec2fleet.Registry;
 import com.amazonaws.regions.Regions;

src/main/java/com/amazon/jenkins/ec2fleet/utils/RegionInfo.java → src/main/java/com/amazon/jenkins/ec2fleet/aws/RegionInfo.java

@@ -1,4 +1,4 @@
-package com.amazon.jenkins.ec2fleet.utils;
+package com.amazon.jenkins.ec2fleet.aws;
 
 import java.util.ArrayList;
 import java.util.List;

src/main/java/com/amazon/jenkins/ec2fleet/fleet/AutoScalingGroupFleet.java

@@ -1,7 +1,7 @@
 package com.amazon.jenkins.ec2fleet.fleet;
 
 import com.amazon.jenkins.ec2fleet.FleetStateStats;
-import com.amazon.jenkins.ec2fleet.utils.AWSUtils;
+import com.amazon.jenkins.ec2fleet.aws.AWSUtils;
 import com.amazonaws.ClientConfiguration;
 import com.amazonaws.regions.Region;
 import com.amazonaws.regions.RegionUtils;

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

@@ -1,5 +1,6 @@
 package com.amazon.jenkins.ec2fleet;
 
+import com.amazon.jenkins.ec2fleet.aws.EC2Api;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleet;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleets;
 import com.amazonaws.services.ec2.AmazonEC2;

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

@@ -1,11 +1,12 @@
 package com.amazon.jenkins.ec2fleet;
 
+import com.amazon.jenkins.ec2fleet.aws.EC2Api;
 import com.amazon.jenkins.ec2fleet.fleet.AutoScalingGroupFleet;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleet;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleets;
 import com.amazon.jenkins.ec2fleet.fleet.EC2SpotFleet;
-import com.amazon.jenkins.ec2fleet.utils.RegionInfo;
-import com.amazon.jenkins.ec2fleet.utils.AwsPermissionChecker;
+import com.amazon.jenkins.ec2fleet.aws.RegionInfo;
+import com.amazon.jenkins.ec2fleet.aws.AwsPermissionChecker;
 import com.amazonaws.regions.RegionUtils;
 import com.amazonaws.services.ec2.AmazonEC2;
 import com.amazonaws.services.ec2.AmazonEC2Client;
@@ -1931,7 +1932,7 @@ public void getDisplayName_returnDefaultWhenNull() {
                 false, null, 0, 1, 0,
                 1, true, false, "-1", false
                 , 0, 0, false, 10, false);
-        assertEquals(ec2FleetCloud.getDisplayName(), EC2FleetCloud.FLEET_CLOUD_ID);
+        assertEquals(ec2FleetCloud.getDisplayName(), EC2FleetCloud.DEFAULT_FLEET_CLOUD_ID);
     }
 
     @Test

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

@@ -1,5 +1,6 @@
 package com.amazon.jenkins.ec2fleet;
 
+import com.amazon.jenkins.ec2fleet.aws.EC2Api;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleet;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleets;
 import com.amazonaws.services.ec2.AmazonEC2;

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

@@ -1,5 +1,7 @@
 package com.amazon.jenkins.ec2fleet;
 
+import com.amazon.jenkins.ec2fleet.aws.CloudFormationApi;
+import com.amazon.jenkins.ec2fleet.aws.EC2Api;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleet;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleets;
 import com.amazon.jenkins.ec2fleet.fleet.EC2SpotFleet;
@@ -31,7 +33,6 @@
 import com.amazonaws.services.ec2.model.TerminateInstancesRequest;
 import com.amazonaws.services.ec2.model.TerminateInstancesResult;
 import com.gargoylesoftware.htmlunit.html.DomElement;
-import com.gargoylesoftware.htmlunit.html.HtmlForm;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
 import hudson.Functions;
 import hudson.model.AbstractBuild;
@@ -57,7 +58,6 @@
 import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
-import org.xml.sax.SAXException;
 
 import javax.annotation.Nullable;
 import java.io.IOException;
@@ -91,7 +91,7 @@
 @SuppressWarnings("WeakerAccess")
 public abstract class IntegrationTest {
 
-    protected static final long JOB_SLEEP_TIME = 30;
+    protected static final long JOB_SLEEP_TIME = 5;
 
     @ClassRule
     public static BuildWatcher bw = new BuildWatcher();

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

@@ -1,5 +1,6 @@
 package com.amazon.jenkins.ec2fleet;
 
+import com.amazon.jenkins.ec2fleet.aws.EC2Api;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleet;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleets;
 import com.amazonaws.services.ec2.AmazonEC2;

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

@@ -1,5 +1,6 @@
 package com.amazon.jenkins.ec2fleet;
 
+import com.amazon.jenkins.ec2fleet.aws.EC2Api;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleet;
 import com.amazon.jenkins.ec2fleet.fleet.EC2Fleets;
 import com.amazonaws.services.ec2.AmazonEC2;

src/test/java/com/amazon/jenkins/ec2fleet/utils/AWSUtilsIntegrationTest.java → src/test/java/com/amazon/jenkins/ec2fleet/aws/AWSUtilsIntegrationTest.java

@@ -1,6 +1,6 @@
-package com.amazon.jenkins.ec2fleet.utils;
+package com.amazon.jenkins.ec2fleet.aws;
 
-import com.amazon.jenkins.ec2fleet.utils.AWSUtils;
+import com.amazon.jenkins.ec2fleet.aws.AWSUtils;
 import com.amazonaws.ClientConfiguration;
 import hudson.ProxyConfiguration;
 import org.junit.Assert;

src/test/java/com/amazon/jenkins/ec2fleet/EC2ApiTest.java → src/test/java/com/amazon/jenkins/ec2fleet/aws/EC2ApiTest.java

@@ -1,5 +1,6 @@
-package com.amazon.jenkins.ec2fleet;
+package com.amazon.jenkins.ec2fleet.aws;
 
+import com.amazon.jenkins.ec2fleet.aws.EC2Api;
 import com.amazonaws.services.ec2.AmazonEC2;
 import com.amazonaws.services.ec2.model.AmazonEC2Exception;
 import com.amazonaws.services.ec2.model.CreateTagsRequest;

src/test/java/com/amazon/jenkins/ec2fleet/utils/RegionInfoTest.java → src/test/java/com/amazon/jenkins/ec2fleet/aws/RegionInfoTest.java

@@ -1,6 +1,6 @@
-package com.amazon.jenkins.ec2fleet.utils;
+package com.amazon.jenkins.ec2fleet.aws;
 
-import com.amazon.jenkins.ec2fleet.utils.RegionInfo;
+import com.amazon.jenkins.ec2fleet.aws.RegionInfo;
 import com.amazonaws.regions.Regions;
 
 import org.junit.Test;

src/test/java/com/amazon/jenkins/ec2fleet/fleet/AutoScalingGroupFleetTest.java

@@ -18,7 +18,7 @@
 import java.util.Map;
 
 import com.amazon.jenkins.ec2fleet.FleetStateStats;
-import com.amazon.jenkins.ec2fleet.utils.AWSUtils;
+import com.amazon.jenkins.ec2fleet.aws.AWSUtils;
 import com.amazonaws.ClientConfiguration;
 import com.amazonaws.services.autoscaling.AmazonAutoScalingClient;
 import com.amazonaws.services.autoscaling.model.AutoScalingGroup;

src/test/java/com/amazon/jenkins/ec2fleet/fleet/EC2SpotFleetTest.java

@@ -1,6 +1,6 @@
 package com.amazon.jenkins.ec2fleet.fleet;
 
-import com.amazon.jenkins.ec2fleet.EC2Api;
+import com.amazon.jenkins.ec2fleet.aws.EC2Api;
 import com.amazon.jenkins.ec2fleet.FleetStateStats;
 import com.amazon.jenkins.ec2fleet.Registry;
 import com.amazonaws.services.ec2.AmazonEC2;