From 2b95c2b3232353fd30ecaab7864e4b67b2852e65 Mon Sep 17 00:00:00 2001 From: Vineeth Bandi <38046649+vineeth-bandi@users.noreply.github.com> Date: Tue, 31 Oct 2023 12:24:40 -0500 Subject: [PATCH] Revert "Update constructor to follow CasC best practices (#406)" (#416) This reverts commit ae77bfa67821a0b05e46eab2f88249f38fac4baf. --- docs/CONFIGURATION-AS-CODE.md | 126 ++------- .../jenkins/ec2fleet/CloudConstants.java | 33 --- .../jenkins/ec2fleet/EC2FleetCloud.java | 256 +++++------------- .../jenkins/ec2fleet/EC2FleetLabelCloud.java | 196 ++++---------- .../ec2fleet/EC2FleetCloud/config.jelly | 2 +- .../EC2FleetCloudConfigurationAsCodeTest.java | 23 +- .../jenkins/ec2fleet/EC2FleetCloudTest.java | 22 +- .../credentials-id-configuration-as-code.yml | 5 - ...ax-less-than-min-configuration-as-code.yml | 6 - 9 files changed, 157 insertions(+), 512 deletions(-) delete mode 100644 src/main/java/com/amazon/jenkins/ec2fleet/CloudConstants.java delete mode 100644 src/test/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/credentials-id-configuration-as-code.yml delete mode 100644 src/test/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/max-less-than-min-configuration-as-code.yml diff --git a/docs/CONFIGURATION-AS-CODE.md b/docs/CONFIGURATION-AS-CODE.md index 3f918d90..fdc2c501 100644 --- a/docs/CONFIGURATION-AS-CODE.md +++ b/docs/CONFIGURATION-AS-CODE.md @@ -8,32 +8,32 @@ [Definition](https://github.com/jenkinsci/ec2-fleet-plugin/blob/master/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java#L156-L179) -| Property | Type | Required | Description | -|----------------------------|---------|------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------| -| name | string | yes, default ```"FleetCloud-XXXXXXXX"``` | A unique name for Jenkins cloud. "" signals the plugin to generate a unique default name for the Cloud. e.g. FleetCloud-jBGChqOP | -| awsCredentialsId | string | no, default ```null``` | [Leave blank to use AWS EC2 instance role](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_use_switch-role-ec2.html) | -| computerConnector | object | yes | for example ```sshConnector``` | -| region | string | yes | ```us-east-2```, full [list](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Concepts.RegionsAndAvailabilityZones.html) | -| fleet | string | yes | my-fleet | -| endpoint | string | no | Set only if you need to use custom endpoint ```http://a.com``` | -| fsRoot | string | no | my-root | -| privateIpUsed | boolean | no, default ```false``` | connect to EC2 instance by private id instead of public | -| alwaysReconnect | boolean | no, default ```false``` | | -| labelString | string | yes | | -| idleMinutes | int | no, default ```0``` | | -| minSize | int | no, default ```0``` | | -| maxSize | int | no, default ```1``` | | -| minSpareSize | int | no, default ```0``` | | minimum number of instances allowed to be idle, ready to pickup work. maxSize overrides minSpareSize. Such instances are exempted from 'Max Idle Minutes Before Scaledown' config. -| maxTotalUses | int | no, default ```-1``` i.e. unlimited uses | | maximum number of times a node can be used. Overrides minSize and minSpareSize, if set. -| numExecutors | int | no, default ```1``` | | -| addNodeOnlyIfRunning | boolean | no, default ```false``` | | -| restrictUsage | boolean | no, default ```false``` | if ```true``` fleet nodes will executed only jobs with same label | -| scaleExecutorsByWeight | boolean | no, default ```false``` | | -| disableTaskResubmit | boolean | no, default ```false``` | | -| initOnlineTimeoutSec | int | no, default ```180``` | | -| initOnlineCheckIntervalSec | int | no, default ```15``` | | -| cloudStatusIntervalSec | int | no, default ```10``` | | -| noDelayProvision | boolean | no, default ```false``` | | +| Property | Type | Required | Description | +|----------------------------|---------|------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------| +| name | string | yes, default ```""``` | A unique name for Jenkins cloud. "" signals the plugin to generate a unique default name for the Cloud. e.g. FleetCloud-jBGChqOP | +| awsCredentialsId | string | no, default ```null``` | [Leave blank to use AWS EC2 instance role](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_use_switch-role-ec2.html) | +| computerConnector | object | yes | for example ```sshConnector``` | +| region | string | yes | ```us-east-2```, full [list](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Concepts.RegionsAndAvailabilityZones.html) | +| fleet | string | yes | my-fleet | +| endpoint | string | no | Set only if you need to use custome endpoint ```http://a.com``` | +| fsRoot | string | no | my-root | +| privateIpUsed | boolean | no, default ```false``` | connect to EC2 instance by private id instead of public | +| alwaysReconnect | boolean | no, default ```false``` || +| labelString | string | yes || +| idleMinutes | int | no, default ```0``` || +| minSize | int | no, default ```0``` || +| maxSize | int | no, default ```0``` || +| minSpareSize | int | no, default ```0``` || minimum number of instances allowed to be idle, ready to pickup work. maxSize overrides minSpareSize. Such instances are exempted from 'Max Idle Minutes Before Scaledown' config. +| maxTotalUses | int | no, default ```-1``` i.e. unlimited uses || maximum number of times a node can be used. Overrides minSize and minSpareSize, if set. +| numExecutors | int | no, default ```1``` || +| addNodeOnlyIfRunning | boolean | no, default ```false``` || +| restrictUsage | boolean | no, default ```false``` | if ```true``` fleet nodes will executed only jobs with same label | +| scaleExecutorsByWeight | boolean | no, default ```false``` || +| disableTaskResubmit | boolean | no, default ```false``` || +| initOnlineTimeoutSec | int | no, default ```180``` || +| initOnlineCheckIntervalSec | int | no, default ```15``` || +| cloudStatusIntervalSec | int | no, default ```10``` || +| noDelayProvision | boolean | no, default ```false``` || ## EC2FleetLabelCloud @@ -41,29 +41,6 @@ More about this type [here](LABEL-BASED-CONFIGURATION.md) [Definition](https://github.com/jenkinsci/ec2-fleet-plugin/blob/master/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud.java#L123-L145) -| Property | Type | Required | Description | -|----------------------------|---------|-----------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------| -| name | string | yes, default ```"FleetCloudLabel-XXXXXXXX"``` | A unique name for Jenkins cloud. "" signals the plugin to generate a unique default name for the Cloud. e.g. `FleetCloudLabel-jBGChqOP` | -| awsCredentialsId | string | no, default ```null``` | [Leave blank to use AWS EC2 instance role](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_use_switch-role-ec2.html) | -| computerConnector | object | yes | for example ```sshConnector``` | -| region | string | yes | ```us-east-2```, full [list](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Concepts.RegionsAndAvailabilityZones.html) | -| endpoint | string | no | Set only if you need to use custom endpoint ```http://a.com``` | -| fsRoot | string | no | my-root | -| privateIpUsed | boolean | no, default ```false``` | connect to EC2 instance by private id instead of public | -| alwaysReconnect | boolean | no, default ```false``` | | -| idleMinutes | int | no, default ```0``` | | -| minSize | int | no, default ```0``` | | -| maxSize | int | no, default ```1``` | | -| maxTotalUses | int | no, default ```-1``` i.e. unlimited uses | | maximum number of times a node can be used. Overrides minSize and minSpareSize, if set. -| numExecutors | int | no, default ```1``` | | -| restrictUsage | boolean | no, default ```false``` | if ```true``` fleet nodes will executed only jobs with same label | -| disableTaskResubmit | boolean | no, default ```false``` | | -| initOnlineTimeoutSec | int | no, default ```180``` | | -| initOnlineCheckIntervalSec | int | no, default ```15``` | | -| cloudStatusIntervalSec | int | no, default ```10``` | | -| noDelayProvision | boolean | no, default ```false``` | | -| ec2KeyPairName | string | yes | AWS EC2 SSH Key-Pair Name | - ## Examples ### EC2FleetCloud (min set of properties) @@ -72,7 +49,7 @@ More about this type [here](LABEL-BASED-CONFIGURATION.md) jenkins: clouds: - ec2Fleet: - name: "" + name: ec2-fleet computerConnector: sshConnector: credentialsId: cred @@ -80,7 +57,8 @@ jenkins: NonVerifyingKeyVerificationStrategy region: us-east-2 fleet: my-fleet - labelString: "" + minSize: 1 + maxSize: 10 ``` ### EC2FleetCloud (All properties) @@ -103,7 +81,6 @@ jenkins: labelString: myLabel idleMinutes: 33 minSize: 15 - minSpareSize: 20 maxSize: 90 numExecutors: 12 addNodeOnlyIfRunning: true @@ -115,48 +92,3 @@ jenkins: disableTaskResubmit: true noDelayProvision: true ``` - -### EC2FleetLabelCloud (min set of properties) - -```yaml -jenkins: - clouds: - - ec2FleetLabel: - name: "" - computerConnector: - sshConnector: - credentialsId: cred - sshHostKeyVerificationStrategy: - NonVerifyingKeyVerificationStrategy - region: us-east-2 - ec2KeyPairName: ec2KeyPair -``` - -### EC2FleetLabelCloud (All properties) - -```yaml -jenkins: - clouds: - - ec2FleetLabel: - name: ec2-fleet-label - awsCredentialsId: xx - computerConnector: - sshConnector: - credentialsId: cred - region: us-east-2 - endpoint: http://a.com - fsRoot: my-root - privateIpUsed: true - alwaysReconnect: true - idleMinutes: 33 - minSize: 15 - maxSize: 90 - numExecutors: 12 - restrictUsage: true - initOnlineTimeoutSec: 181 - initOnlineCheckIntervalSec: 13 - cloudStatusIntervalSec: 11 - disableTaskResubmit: true - noDelayProvision: true - ec2KeyPairName: ec2KeyPair -``` diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/CloudConstants.java b/src/main/java/com/amazon/jenkins/ec2fleet/CloudConstants.java deleted file mode 100644 index 651a01f3..00000000 --- a/src/main/java/com/amazon/jenkins/ec2fleet/CloudConstants.java +++ /dev/null @@ -1,33 +0,0 @@ -package com.amazon.jenkins.ec2fleet; - -class CloudConstants { - - static final String EC2_INSTANCE_TAG_NAMESPACE = "ec2-fleet-plugin"; - static final String EC2_INSTANCE_CLOUD_NAME_TAG = EC2_INSTANCE_TAG_NAMESPACE + ":cloud-name"; - static final boolean DEFAULT_PRIVATE_IP_USED = false; - - static final boolean DEFAULT_ALWAYS_RECONNECT = false; - - static final int DEFAULT_IDLE_MINUTES = 0; - - static final int DEFAULT_MIN_SIZE = 0; - - static final int DEFAULT_MAX_SIZE = 1; - - static final int DEFAULT_MIN_SPARE_SIZE = 0; - - static final int DEFAULT_NUM_EXECUTORS = 1; - - static final boolean DEFAULT_ADD_NODE_ONLY_IF_RUNNING = false; - - static final boolean DEFAULT_RESTRICT_USAGE = false; - - static final boolean DEFAULT_SCALE_EXECUTORS_BY_WEIGHT = false; - - static final int DEFAULT_CLOUD_STATUS_INTERVAL_SEC = 10; - - static final int DEFAULT_INIT_ONLINE_TIMEOUT_SEC = 3 * 60; - static final int DEFAULT_INIT_ONLINE_CHECK_INTERVAL_SEC = 15; - - static final int DEFAULT_MAX_TOTAL_USES = -1; -} diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java index 285a818b..f19db6a9 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java @@ -28,7 +28,6 @@ import net.sf.json.JSONObject; import org.apache.commons.lang.StringUtils; import org.kohsuke.stapler.DataBoundConstructor; -import org.kohsuke.stapler.DataBoundSetter; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.StaplerRequest; @@ -56,8 +55,6 @@ import java.util.logging.SimpleFormatter; import java.util.stream.Collectors; -import static com.amazon.jenkins.ec2fleet.CloudConstants.*; - /** * 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. @@ -73,8 +70,18 @@ @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 BASE_DEFAULT_FLEET_CLOUD_ID = "FleetCloud"; + public static final int DEFAULT_CLOUD_STATUS_INTERVAL_SEC = 10; + + private static final int DEFAULT_INIT_ONLINE_TIMEOUT_SEC = 3 * 60; + private static final int DEFAULT_INIT_ONLINE_CHECK_INTERVAL_SEC = 15; + + private static final int DEFAULT_MAX_TOTAL_USES = -1; + private static final SimpleFormatter sf = new SimpleFormatter(); private static final Logger LOGGER = Logger.getLogger(EC2FleetCloud.class.getName()); private static final ScheduledExecutorService EXECUTOR = Executors.newSingleThreadScheduledExecutor(); @@ -94,43 +101,43 @@ public class EC2FleetCloud extends AbstractEC2FleetCloud { * Will be deleted in future when usage for old version <= 1.1.9 will be totally dropped. */ @Deprecated - private String credentialsId; + private final String credentialsId; - private String awsCredentialsId; + private final String awsCredentialsId; private final String region; - private String endpoint; + private final String endpoint; /** * In fact fleet ID, not name or anything else */ private final String fleet; - private String fsRoot; + private final String fsRoot; private final ComputerConnector computerConnector; - private boolean privateIpUsed = DEFAULT_PRIVATE_IP_USED; - private boolean alwaysReconnect = DEFAULT_ALWAYS_RECONNECT; + private final boolean privateIpUsed; + private final boolean alwaysReconnect; private final String labelString; - private int idleMinutes = DEFAULT_IDLE_MINUTES; - private int minSize = DEFAULT_MIN_SIZE; - private int maxSize = DEFAULT_MAX_SIZE; - private int minSpareSize = DEFAULT_MIN_SPARE_SIZE; - private int numExecutors = DEFAULT_NUM_EXECUTORS; - private boolean addNodeOnlyIfRunning = DEFAULT_ADD_NODE_ONLY_IF_RUNNING; - private boolean restrictUsage = DEFAULT_RESTRICT_USAGE; - private boolean scaleExecutorsByWeight = DEFAULT_SCALE_EXECUTORS_BY_WEIGHT; - private Integer initOnlineTimeoutSec = DEFAULT_INIT_ONLINE_TIMEOUT_SEC; - private Integer initOnlineCheckIntervalSec = DEFAULT_INIT_ONLINE_CHECK_INTERVAL_SEC; - private Integer cloudStatusIntervalSec = DEFAULT_CLOUD_STATUS_INTERVAL_SEC; - private Integer maxTotalUses = DEFAULT_MAX_TOTAL_USES; + private final Integer idleMinutes; + private final int minSize; + private final int maxSize; + private final int minSpareSize; + private final int numExecutors; + private final boolean addNodeOnlyIfRunning; + private final boolean restrictUsage; + private final boolean scaleExecutorsByWeight; + private final Integer initOnlineTimeoutSec; + private final Integer initOnlineCheckIntervalSec; + private final Integer cloudStatusIntervalSec; + private final Integer maxTotalUses; /** * @see EC2FleetAutoResubmitComputerLauncher */ - private boolean disableTaskResubmit; + private final boolean disableTaskResubmit; /** * @see NoDelayProvisionStrategy */ - private boolean noDelayProvision; + private final boolean noDelayProvision; /** * {@link EC2FleetCloud#update()} updating this field, this is one thread @@ -156,28 +163,6 @@ public class EC2FleetCloud extends AbstractEC2FleetCloud { private transient AtomicInteger plannedNodeCounter = new AtomicInteger(1); @DataBoundConstructor - public EC2FleetCloud(@Nonnull final String name, - final String awsCredentialsId, - final String region, - final String fleet, - final ComputerConnector computerConnector, - final String labelString) { - super(StringUtils.isNotBlank(name) ? name : CloudNames.generateUnique(BASE_DEFAULT_FLEET_CLOUD_ID)); - init(); - //TODO: eventually move this out of this constructor since it is not technically a required field - setAwsCredentialsId(awsCredentialsId); - this.region = region; - this.fleet = fleet; - this.computerConnector = computerConnector; - this.labelString = labelString; - - if (fleet != null) { - this.stats = EC2Fleets.get(fleet).getState( - getAwsCredentialsId(), region, endpoint, getFleet()); - } - } - - @Deprecated public EC2FleetCloud(@Nonnull final String name, final String awsCredentialsId, final @Deprecated String credentialsId, @@ -189,7 +174,7 @@ public EC2FleetCloud(@Nonnull final String name, final ComputerConnector computerConnector, final boolean privateIpUsed, final boolean alwaysReconnect, - final int idleMinutes, + final Integer idleMinutes, final int minSize, final int maxSize, final int minSpareSize, @@ -203,26 +188,35 @@ public EC2FleetCloud(@Nonnull final String name, final boolean scaleExecutorsByWeight, final Integer cloudStatusIntervalSec, final boolean noDelayProvision) { - this(name, awsCredentialsId, region, fleet, computerConnector, labelString); + super(StringUtils.isNotBlank(name) ? name : CloudNames.generateUnique(BASE_DEFAULT_FLEET_CLOUD_ID)); + init(); this.credentialsId = credentialsId; - setEndpoint(endpoint); - setFsRoot(fsRoot); - setPrivateIpUsed(privateIpUsed); - setAlwaysReconnect(alwaysReconnect); - setIdleMinutes(idleMinutes); - setMinSize(minSize); - setMaxSize(maxSize); - setMinSpareSize(minSpareSize); - setMaxTotalUses(maxTotalUses); - setNumExecutors(numExecutors); - setAddNodeOnlyIfRunning(addNodeOnlyIfRunning); - setRestrictUsage(restrictUsage); - setScaleExecutorsByWeight(scaleExecutorsByWeight); - setDisableTaskResubmit(disableTaskResubmit); - setInitOnlineTimeoutSec(initOnlineTimeoutSec); - setInitOnlineCheckIntervalSec(initOnlineCheckIntervalSec); - setCloudStatusIntervalSec(cloudStatusIntervalSec); - setNoDelayProvision(noDelayProvision); + this.awsCredentialsId = awsCredentialsId; + this.region = region; + this.endpoint = endpoint; + this.fleet = fleet; + this.fsRoot = fsRoot; + this.computerConnector = computerConnector; + this.labelString = labelString; + this.idleMinutes = idleMinutes; + this.privateIpUsed = privateIpUsed; + this.alwaysReconnect = alwaysReconnect; + if (minSize < 0) { + warning("Cloud parameter 'minSize' can't be less than 0, setting to 0"); + } + this.minSize = Math.max(0, minSize); + this.maxSize = maxSize; + this.minSpareSize = Math.max(0, minSpareSize); + this.maxTotalUses = StringUtils.isBlank(maxTotalUses) ? DEFAULT_MAX_TOTAL_USES : Integer.parseInt(maxTotalUses); + this.numExecutors = Math.max(numExecutors, 1); + this.addNodeOnlyIfRunning = addNodeOnlyIfRunning; + this.restrictUsage = restrictUsage; + this.scaleExecutorsByWeight = scaleExecutorsByWeight; + this.disableTaskResubmit = disableTaskResubmit; + this.initOnlineTimeoutSec = initOnlineTimeoutSec; + this.initOnlineCheckIntervalSec = initOnlineCheckIntervalSec; + this.cloudStatusIntervalSec = cloudStatusIntervalSec; + this.noDelayProvision = noDelayProvision; if (fleet != null) { this.stats = EC2Fleets.get(fleet).getState( @@ -234,11 +228,6 @@ public boolean isNoDelayProvision() { return noDelayProvision; } - @DataBoundSetter - public void setNoDelayProvision(boolean noDelayProvision) { - this.noDelayProvision = noDelayProvision; - } - /** * Deprecated.Use {@link EC2FleetCloud#awsCredentialsId} * @@ -251,36 +240,12 @@ public String getAwsCredentialsId() { return StringUtils.isNotBlank(awsCredentialsId) ? awsCredentialsId : credentialsId; } - @DataBoundSetter - public void setAwsCredentialsId(String awsCredentialsId) { - this.awsCredentialsId = awsCredentialsId; - } - - public String getCredentialsId() { - return credentialsId; - } - - @DataBoundSetter - public void setCredentialsId(String credentialsId) { - this.credentialsId = credentialsId; - } - public boolean isDisableTaskResubmit() { return disableTaskResubmit; } - @DataBoundSetter - public void setDisableTaskResubmit(boolean disableTaskResubmit) { - this.disableTaskResubmit = disableTaskResubmit; - } - public int getInitOnlineTimeoutSec() { - return initOnlineTimeoutSec; - } - - @DataBoundSetter - public void setInitOnlineTimeoutSec(Integer initOnlineTimeoutSec) { - this.initOnlineTimeoutSec = initOnlineTimeoutSec; + return initOnlineTimeoutSec == null ? DEFAULT_INIT_ONLINE_TIMEOUT_SEC : initOnlineTimeoutSec; } public int getScheduledFutureTimeoutSec() { @@ -289,21 +254,11 @@ public int getScheduledFutureTimeoutSec() { } public int getCloudStatusIntervalSec() { - return cloudStatusIntervalSec; - } - - @DataBoundSetter - public void setCloudStatusIntervalSec(Integer cloudStatusIntervalSec) { - this.cloudStatusIntervalSec = cloudStatusIntervalSec; + return cloudStatusIntervalSec == null ? DEFAULT_CLOUD_STATUS_INTERVAL_SEC : cloudStatusIntervalSec; } public int getInitOnlineCheckIntervalSec() { - return initOnlineCheckIntervalSec; - } - - @DataBoundSetter - public void setInitOnlineCheckIntervalSec(Integer initOnlineCheckIntervalSec) { - this.initOnlineCheckIntervalSec = initOnlineCheckIntervalSec; + return initOnlineCheckIntervalSec == null ? DEFAULT_INIT_ONLINE_CHECK_INTERVAL_SEC : initOnlineCheckIntervalSec; } public String getRegion() { @@ -314,11 +269,6 @@ public String getEndpoint() { return endpoint; } - @DataBoundSetter - public void setEndpoint(String endpoint) { - this.endpoint = endpoint; - } - public String getFleet() { return fleet; } @@ -327,20 +277,10 @@ public String getFsRoot() { return fsRoot; } - @DataBoundSetter - public void setFsRoot(String fsRoot) { - this.fsRoot = fsRoot; - } - public boolean isAddNodeOnlyIfRunning() { return addNodeOnlyIfRunning; } - @DataBoundSetter - public void setAddNodeOnlyIfRunning(boolean addNodeOnlyIfRunning) { - this.addNodeOnlyIfRunning = addNodeOnlyIfRunning; - } - public ComputerConnector getComputerConnector() { return computerConnector; } @@ -349,105 +289,42 @@ public boolean isPrivateIpUsed() { return privateIpUsed; } - @DataBoundSetter - public void setPrivateIpUsed(boolean privateIpUsed) { - this.privateIpUsed = privateIpUsed; - } - public boolean isAlwaysReconnect() { return alwaysReconnect; } - @DataBoundSetter - public void setAlwaysReconnect(boolean alwaysReconnect) { - this.alwaysReconnect = alwaysReconnect; - } - public String getLabelString() { return labelString; } public int getIdleMinutes() { - return idleMinutes; - } - - @DataBoundSetter - public void setIdleMinutes(int idleMinutes) { - this.idleMinutes = Math.max(0, idleMinutes); + return (idleMinutes != null) ? idleMinutes : 0; } public int getMaxSize() { return maxSize; } - @DataBoundSetter - public void setMaxSize(int maxSize) { - if (maxSize < minSize) { - int newMaxSize = Math.max(1, minSize); - warning("Cloud parameter 'maxSize' can't be less than 'minSize' or 1, setting to %d", newMaxSize); - maxSize = newMaxSize; - } - this.maxSize = Math.max(1, maxSize); - } - public int getMinSize() { return minSize; } - @DataBoundSetter - public void setMinSize(int minSize) { - if (minSize < 0) { - warning("Cloud parameter 'minSize' can't be less than 0, setting to 0"); - } - minSize = Math.max(0, minSize); - //TODO: This validation is only in place for unit tests since constructor is run twice on CasC load but not - // for unit tests - if (minSize > maxSize) { - warning("Cloud parameter 'minSize' cannot be greater than 'maxSize', setting 'maxSize' to %d. " + - "Ignore this if caused after a CasC load.", minSize); - this.maxSize = minSize; - } - this.minSize = minSize; - } - - public synchronized int getMinSpareSize() { + public int getMinSpareSize() { return minSpareSize; } - @DataBoundSetter - public synchronized void setMinSpareSize(int minSpareSize) { - this.minSpareSize = Math.max(0, minSpareSize); - } - public int getMaxTotalUses() { return maxTotalUses; } - @DataBoundSetter - public void setMaxTotalUses(String maxTotalUses) { - if (StringUtils.isNotBlank(maxTotalUses)) { - this.maxTotalUses = Integer.parseInt(maxTotalUses); - } - } - public int getNumExecutors() { return numExecutors; } - @DataBoundSetter - public void setNumExecutors(int numExecutors) { - this.numExecutors = Math.max(numExecutors, 1); - } - public boolean isScaleExecutorsByWeight() { return scaleExecutorsByWeight; } - @DataBoundSetter - public void setScaleExecutorsByWeight(boolean scaleExecutorsByWeight) { - this.scaleExecutorsByWeight = scaleExecutorsByWeight; - } - public String getJvmSettings() { return ""; } @@ -456,11 +333,6 @@ public boolean isRestrictUsage() { return restrictUsage; } - @DataBoundSetter - public void setRestrictUsage(boolean restrictUsage) { - this.restrictUsage = restrictUsage; - } - // Visible for testing synchronized Set getPlannedNodesCache() { return plannedNodesCache; diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud.java index 9a409cf2..24cf4c5e 100644 --- a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud.java +++ b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud.java @@ -32,7 +32,6 @@ import net.sf.json.JSONObject; import org.apache.commons.lang.StringUtils; import org.kohsuke.stapler.DataBoundConstructor; -import org.kohsuke.stapler.DataBoundSetter; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.StaplerRequest; @@ -55,64 +54,58 @@ import java.util.logging.SimpleFormatter; import java.util.stream.Collectors; -import static com.amazon.jenkins.ec2fleet.CloudConstants.*; - /** * @see CloudNanny */ @SuppressWarnings({"unused", "WeakerAccess"}) 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 BASE_DEFAULT_FLEET_CLOUD_ID = "FleetCloudLabel"; + public static final int DEFAULT_CLOUD_STATUS_INTERVAL_SEC = 10; + + private static final int DEFAULT_INIT_ONLINE_TIMEOUT_SEC = 3 * 60; + private static final int DEFAULT_INIT_ONLINE_CHECK_INTERVAL_SEC = 15; + // private static final String NEW_EC2_KEY_PAIR_VALUE = "- New Key Pair -"; private static final SimpleFormatter sf = new SimpleFormatter(); private static final Logger LOGGER = Logger.getLogger(EC2FleetLabelCloud.class.getName()); - private String awsCredentialsId; + private final String awsCredentialsId; private final String region; - private String endpoint; + private final String endpoint; - private String fsRoot; + private final String fsRoot; private final ComputerConnector computerConnector; - private boolean privateIpUsed = DEFAULT_PRIVATE_IP_USED; - private boolean alwaysReconnect = DEFAULT_ALWAYS_RECONNECT; - private int idleMinutes = DEFAULT_IDLE_MINUTES; - private int minSize = DEFAULT_MIN_SIZE; - private int maxSize = DEFAULT_MAX_SIZE; - private int numExecutors = DEFAULT_NUM_EXECUTORS; - private boolean restrictUsage = DEFAULT_RESTRICT_USAGE; - private Integer initOnlineTimeoutSec = DEFAULT_INIT_ONLINE_TIMEOUT_SEC; - private Integer initOnlineCheckIntervalSec = DEFAULT_INIT_ONLINE_CHECK_INTERVAL_SEC; - private Integer cloudStatusIntervalSec = DEFAULT_CLOUD_STATUS_INTERVAL_SEC; + private final boolean privateIpUsed; + private final boolean alwaysReconnect; + private final Integer idleMinutes; + private final Integer minSize; + private final Integer maxSize; + private final Integer numExecutors; + private final boolean restrictUsage; + private final Integer initOnlineTimeoutSec; + private final Integer initOnlineCheckIntervalSec; + private final Integer cloudStatusIntervalSec; private final String ec2KeyPairName; /** * @see EC2FleetAutoResubmitComputerLauncher */ - private boolean disableTaskResubmit; + private final boolean disableTaskResubmit; /** * @see NoDelayProvisionStrategy */ - private boolean noDelayProvision; + private final boolean noDelayProvision; private transient Map states; @DataBoundConstructor - public EC2FleetLabelCloud(@Nonnull final String name, - final String region, - final ComputerConnector computerConnector, - final String ec2KeyPairName) { - super(StringUtils.isNotBlank(name) ? name : CloudNames.generateUnique(BASE_DEFAULT_FLEET_CLOUD_ID)); - init(); - this.region = region; - this.computerConnector = computerConnector; - this.ec2KeyPairName = ec2KeyPairName; - } - - @Deprecated public EC2FleetLabelCloud(final String name, final String awsCredentialsId, final String region, @@ -121,7 +114,7 @@ public EC2FleetLabelCloud(final String name, final ComputerConnector computerConnector, final boolean privateIpUsed, final boolean alwaysReconnect, - final int idleMinutes, + final Integer idleMinutes, final Integer minSize, final Integer maxSize, final Integer numExecutors, @@ -132,22 +125,26 @@ public EC2FleetLabelCloud(final String name, final Integer cloudStatusIntervalSec, final boolean noDelayProvision, final String ec2KeyPairName) { - this(name, region, computerConnector, ec2KeyPairName); - setAwsCredentialsId(awsCredentialsId); - setEndpoint(endpoint); - setFsRoot(fsRoot); - setPrivateIpUsed(privateIpUsed); - setAlwaysReconnect(alwaysReconnect); - setIdleMinutes(idleMinutes); - setMinSize(minSize); - setMaxSize(maxSize); - setNumExecutors(numExecutors); - setRestrictUsage(restrictUsage); - setDisableTaskResubmit(disableTaskResubmit); - setInitOnlineTimeoutSec(initOnlineTimeoutSec); - setInitOnlineCheckIntervalSec(initOnlineCheckIntervalSec); - setCloudStatusIntervalSec(cloudStatusIntervalSec); - setNoDelayProvision(noDelayProvision); + super(StringUtils.isNotBlank(name) ? name : CloudNames.generateUnique(BASE_DEFAULT_FLEET_CLOUD_ID)); + init(); + this.awsCredentialsId = awsCredentialsId; + this.region = region; + this.endpoint = endpoint; + this.fsRoot = fsRoot; + this.computerConnector = computerConnector; + this.idleMinutes = idleMinutes; + this.privateIpUsed = privateIpUsed; + this.alwaysReconnect = alwaysReconnect; + this.minSize = minSize; + this.maxSize = maxSize; + this.numExecutors = numExecutors; + this.restrictUsage = restrictUsage; + this.disableTaskResubmit = disableTaskResubmit; + this.initOnlineTimeoutSec = initOnlineTimeoutSec; + this.initOnlineCheckIntervalSec = initOnlineCheckIntervalSec; + this.cloudStatusIntervalSec = cloudStatusIntervalSec; + this.noDelayProvision = noDelayProvision; + this.ec2KeyPairName = ec2KeyPairName; } public String getEc2KeyPairName() { @@ -158,54 +155,24 @@ public boolean isNoDelayProvision() { return noDelayProvision; } - @DataBoundSetter - public void setNoDelayProvision(boolean noDelayProvision) { - this.noDelayProvision = noDelayProvision; - } - public String getAwsCredentialsId() { return awsCredentialsId; } - @DataBoundSetter - public void setAwsCredentialsId(String awsCredentialsId) { - this.awsCredentialsId = awsCredentialsId; - } - public boolean isDisableTaskResubmit() { return disableTaskResubmit; } - @DataBoundSetter - public void setDisableTaskResubmit(boolean disableTaskResubmit) { - this.disableTaskResubmit = disableTaskResubmit; - } - public int getInitOnlineTimeoutSec() { - return initOnlineTimeoutSec; - } - - @DataBoundSetter - public void setInitOnlineTimeoutSec(Integer initOnlineTimeoutSec) { - this.initOnlineTimeoutSec = initOnlineTimeoutSec; + return initOnlineTimeoutSec == null ? DEFAULT_INIT_ONLINE_TIMEOUT_SEC : initOnlineTimeoutSec; } public int getCloudStatusIntervalSec() { - return cloudStatusIntervalSec; - } - - @DataBoundSetter - public void setCloudStatusIntervalSec(Integer cloudStatusIntervalSec) { - this.cloudStatusIntervalSec = cloudStatusIntervalSec; + return cloudStatusIntervalSec == null ? DEFAULT_CLOUD_STATUS_INTERVAL_SEC : cloudStatusIntervalSec; } public int getInitOnlineCheckIntervalSec() { - return initOnlineCheckIntervalSec; - } - - @DataBoundSetter - public void setInitOnlineCheckIntervalSec(Integer initOnlineCheckIntervalSec) { - this.initOnlineCheckIntervalSec = initOnlineCheckIntervalSec; + return initOnlineCheckIntervalSec == null ? DEFAULT_INIT_ONLINE_CHECK_INTERVAL_SEC : initOnlineCheckIntervalSec; } public String getRegion() { @@ -216,20 +183,10 @@ public String getEndpoint() { return endpoint; } - @DataBoundSetter - public void setEndpoint(String endpoint) { - this.endpoint = endpoint; - } - public String getFsRoot() { return fsRoot; } - @DataBoundSetter - public void setFsRoot(String fsRoot) { - this.fsRoot = fsRoot; - } - public ComputerConnector getComputerConnector() { return computerConnector; } @@ -238,72 +195,26 @@ public boolean isPrivateIpUsed() { return privateIpUsed; } - @DataBoundSetter - public void setPrivateIpUsed(boolean privateIpUsed) { - this.privateIpUsed = privateIpUsed; - } - public boolean isAlwaysReconnect() { return alwaysReconnect; } - @DataBoundSetter - public void setAlwaysReconnect(boolean alwaysReconnect) { - this.alwaysReconnect = alwaysReconnect; - } - public int getIdleMinutes() { - return idleMinutes; - } - - @DataBoundSetter - public void setIdleMinutes(int idleMinutes) { - this.idleMinutes = Math.max(0, idleMinutes); + return (idleMinutes != null) ? idleMinutes : 0; } - public int getMaxSize() { + public Integer getMaxSize() { return maxSize; } - @DataBoundSetter - public void setMaxSize(int maxSize) { - if (maxSize < minSize) { - int newMaxSize = Math.max(1, minSize); - warning("Cloud parameter 'maxSize' can't be less than 'minSize' or 1, setting to %d", newMaxSize); - maxSize = newMaxSize; - } - this.maxSize = Math.max(1, maxSize); - } - - public int getMinSize() { + public Integer getMinSize() { return minSize; } - @DataBoundSetter - public void setMinSize(int minSize) { - if (minSize < 0) { - warning("Cloud parameter 'minSize' can't be less than 0, setting to 0"); - } - minSize = Math.max(0, minSize); - //TODO: This validation is only in place for unit tests since constructor is run twice on CasC load but not - // for unit tests - if (minSize > maxSize) { - warning("Cloud parameter 'minSize' cannot be greater than 'maxSize', setting 'maxSize' to %d. " + - "Ignore this if caused after a CasC load.", minSize); - this.maxSize = minSize; - } - this.minSize = minSize; - } - - public int getNumExecutors() { + public Integer getNumExecutors() { return numExecutors; } - @DataBoundSetter - public void setNumExecutors(int numExecutors) { - this.numExecutors = Math.max(numExecutors, 1); - } - public String getJvmSettings() { return ""; } @@ -312,11 +223,6 @@ public boolean isRestrictUsage() { return restrictUsage; } - @DataBoundSetter - public void setRestrictUsage(boolean restrictUsage) { - this.restrictUsage = restrictUsage; - } - @Override public synchronized boolean hasExcessCapacity() { // TODO: Check if the current count of instances are greater than allowed 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 733ab87f..87e5af98 100644 --- a/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/config.jelly +++ b/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/config.jelly @@ -94,7 +94,7 @@ - + diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudConfigurationAsCodeTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudConfigurationAsCodeTest.java index c2d2cec6..2d0dbc23 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudConfigurationAsCodeTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudConfigurationAsCodeTest.java @@ -61,7 +61,7 @@ public void shouldCreateCloudFromMinConfiguration() { assertEquals(cloud.getLabelString(), null); assertEquals(cloud.getIdleMinutes(), 0); assertEquals(cloud.getMinSize(), 0); - assertEquals(cloud.getMaxSize(), 1); + assertEquals(cloud.getMaxSize(), 0); assertEquals(cloud.getNumExecutors(), 1); assertEquals(cloud.isAddNodeOnlyIfRunning(), false); assertEquals(cloud.isRestrictUsage(), false); @@ -116,25 +116,4 @@ public void configurationWithEmptyName_shouldUseDefault() { assertEquals(("FleetCloud".length() + CloudNames.SUFFIX_LENGTH + 1), cloud.name.length()); } } - - @Test - @ConfiguredWithCode("EC2FleetCloud/credentials-id-configuration-as-code.yml") - public void configurationWithCredentialsId() { - assertEquals(jenkinsRule.jenkins.clouds.size(), 1); - EC2FleetCloud cloud = (EC2FleetCloud) jenkinsRule.jenkins.clouds.getByName("ec2-fleet"); - - assertEquals("ec2-fleet", cloud.name); - assertEquals(cloud.getCredentialsId(), "cred"); - } - - @Test - @ConfiguredWithCode("EC2FleetCloud/max-less-than-min-configuration-as-code.yml") - public void configurationWithMaxSizeLessThanMinSize() { - assertEquals(jenkinsRule.jenkins.clouds.size(), 1); - EC2FleetCloud cloud = (EC2FleetCloud) jenkinsRule.jenkins.clouds.getByName("ec2-fleet"); - - assertEquals("ec2-fleet", cloud.name); - assertEquals(cloud.getMinSize(), 10); - assertEquals(cloud.getMaxSize(), 10); - } } \ No newline at end of file diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudTest.java index 539636e0..623582e7 100644 --- a/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudTest.java +++ b/src/test/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudTest.java @@ -1958,7 +1958,7 @@ public void getDisplayName_returnDisplayName() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( "CloudName", null, null, null, null, null, null, null, null, false, - false, 0, 0, 1, 0, + false, null, 0, 1, 0, 1, true, false, "-1", false , 0, 0, false, 10, false); @@ -1970,7 +1970,7 @@ public void getAwsCredentialsId_returnNull_whenNoCredentialsIdOrAwsCredentialsId EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( "TestCloud", null, null, null, null, null, null, null, null, false, - false, 0, 0, 1, 0, + false, null, 0, 1, 0, 1, true, false, "-1", false, 0, 0, false, 10, false); @@ -1982,7 +1982,7 @@ public void getAwsCredentialsId_returnValue_whenCredentialsIdPresent() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( "TestCloud", null, "Opa", null, null, null, null, null, null, false, - false, 0, 0, 1, 0, + false, null, 0, 1, 0, 1, true, false, "-1", false , 0, 0, false, 10, false); @@ -1994,7 +1994,7 @@ public void getAwsCredentialsId_returnValue_whenAwsCredentialsIdPresent() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( "TestCloud", "Opa", null, null, null, null, null, null, null, false, - false, 0, 0, 1, 0, + false, null, 0, 1, 0, 1, true, false, "-1", false , 0, 0, false, 10, false); @@ -2006,7 +2006,7 @@ public void getAwsCredentialsId_returnAwsCredentialsId_whenAwsCredentialsIdAndCr EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( "TestCloud", "A", "B", null, null, null, null, null, null, false, - false, 0, 0, 1, 0, + false, null, 0, 1, 0, 1, true, false, "-1", false , 0, 0, false, 10, false); @@ -2020,7 +2020,7 @@ public void getCloudStatusInterval_returnCloudStatusInterval() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( "CloudName", null, null, null, null, null, null, null, null, false, - false, 0, 0, 1, 0, + false, null, 0, 1, 0, 1, true, false, "-1", false , 0, 0, false, 45, false); @@ -2032,7 +2032,7 @@ public void create_numExecutorsLessThenOneShouldUpgradedToOne() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( "CloudName", null, null, null, null, null, null, null, null, false, - false, 0, 0, 1, 0, + false, null, 0, 1, 0, 0, true, false, "-1", false , 0, 0, false, 45, false); @@ -2045,7 +2045,7 @@ public void hasUnlimitedUsesForNodes_shouldReturnTrueWhenUnlimited() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( "CloudName", null, null, null, null, null, null, null, null, false, - false, 0, 0, 1, 0, + false, null, 0, 1, 0, 0, true, false, String.valueOf(maxTotalUses), false , 0, 0, false, 45, false); @@ -2058,7 +2058,7 @@ public void hasUnlimitedUsesForNodes_shouldReturnDefaultTrueForNull() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( "CloudName", null, null, null, null, null, null, null, null, false, - false, 0, 0, 1, 0, + false, null, 0, 1, 0, 0, true, false, maxTotalUses, false , 0, 0, false, 45, false); @@ -2071,7 +2071,7 @@ public void hasUnlimitedUsesForNodes_shouldReturnDefaultTrueForEmptyString() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( "CloudName", null, null, null, null, null, null, null, null, false, - false, 0, 0, 1, 0, + false, null, 0, 1, 0, 0, true, false, maxTotalUses, false , 0, 0, false, 45, false); @@ -2084,7 +2084,7 @@ public void hasUnlimitedUsesForNodes_shouldReturnFalseWhenLimited() { EC2FleetCloud ec2FleetCloud = new EC2FleetCloud( "CloudName", null, null, null, null, null, null, null, null, false, - false, 0, 0, 1, 0, + false, null, 0, 1, 0, 0, true, false, String.valueOf(maxTotalUses), false , 0, 0, false, 45, false); diff --git a/src/test/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/credentials-id-configuration-as-code.yml b/src/test/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/credentials-id-configuration-as-code.yml deleted file mode 100644 index 01371d7e..00000000 --- a/src/test/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/credentials-id-configuration-as-code.yml +++ /dev/null @@ -1,5 +0,0 @@ -jenkins: - clouds: - - ec2Fleet: - name: ec2-fleet - credentialsId: cred \ No newline at end of file diff --git a/src/test/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/max-less-than-min-configuration-as-code.yml b/src/test/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/max-less-than-min-configuration-as-code.yml deleted file mode 100644 index e60fa107..00000000 --- a/src/test/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/max-less-than-min-configuration-as-code.yml +++ /dev/null @@ -1,6 +0,0 @@ -jenkins: - clouds: - - ec2Fleet: - name: ec2-fleet - minSize: 10 - maxSize: 0 \ No newline at end of file