JENKINS-19845, EC2 plugin incorrectly reports current instance count

- Changed tag name to jenkins_slave_type - Made the tag name a static value - Cleaned up indentation - Removed eclipse import format changes
jenkinsci · Sep 11, 2014 · 5286a64 · 5286a64
1 parent deffa18
commit 5286a64
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 31 deletions.
diff --git a/src/main/java/hudson/plugins/ec2/EC2Cloud.java b/src/main/java/hudson/plugins/ec2/EC2Cloud.java
@@ -24,7 +24,6 @@
 package hudson.plugins.ec2;
 
 import com.amazonaws.ClientConfiguration;
-
 import hudson.ProxyConfiguration;
 import hudson.model.Computer;
 import hudson.model.Descriptor;
@@ -226,30 +225,33 @@ public int countCurrentEC2Slaves(String ami) throws AmazonClientException {
         return n;
     }
 
-	/**
-	 * This method checks if the slave is provisioned by this plugin. 
-	 * It does so by: 1) checking if the ami matches and 2) checking
-	 * for a tag with the key jenkins_slave_type. The value of the tag will be ignored.
-	 * 
-	 * See also JENKINS-19845
-	 * 
-	 * @param i
-	 * @param ami
-	 * @return
-	 */
-	private boolean isEc2ProvisionedSlave(Instance i, String ami) {
-		// Check if the ami matches
-		if (ami == null || StringUtils.equals(ami, i.getImageId())) {
-			// Check if there is a ec2slave tag...
-			for (Tag tag : i.getTags()) {
-				if (StringUtils.equals(tag.getKey(), "jenkins_slave_type")) {
-					return true;
-				}
-			}
-			return false;
-		}
-		return false;
-	}
+    /**
+     * This method checks if the slave is provisioned by this plugin. 
+     * An instance is a provisioned slave if:
+     * <ol>
+     *  <li>The AMI id matches.</li>
+     *  <li>There is a tag with the key name {@link EC2Tag.TAG_NAME_JENKINS_SLAVE_TYPE}</li>.
+     *  </ol>
+     * 
+     * @see JENKINS-19845 (https://issues.jenkins-ci.org/browse/JENKINS-19845)
+     * 
+     * @param i the instance to be checked.
+     * @param ami the ami id. 
+     * @return true if the instance is a provisioned slave
+     */
+    protected boolean isEc2ProvisionedSlave(Instance i, String ami) {
+        // Check if the ami matches
+        if (ami == null || StringUtils.equals(ami, i.getImageId())) {
+            // Check if there is a ec2slave tag...
+            for (Tag tag : i.getTags()) {
+                if (StringUtils.equals(tag.getKey(), EC2Tag.TAG_NAME_JENKINS_SLAVE_TYPE)) {
+                    return true;
+                }
+            }
+            return false;
+        }
+        return false;
+    }
 
     /**
      * Debug command to attach to a running instance.

diff --git a/src/main/java/hudson/plugins/ec2/EC2Tag.java b/src/main/java/hudson/plugins/ec2/EC2Tag.java
@@ -39,6 +39,12 @@ public class EC2Tag extends AbstractDescribableImpl<EC2Tag>
    private String name;
    private String value;
 
+   /**
+   * Tag name for the specific jenkings slave type tag, used to identify the 
+   * EC2 instances provisioned by this plugin.
+   */
+   public static final String TAG_NAME_JENKINS_SLAVE_TYPE = "jenkins_slave_type";
+
    @DataBoundConstructor
    public EC2Tag(String name, String value) {
       this.name = name;

diff --git a/src/main/java/hudson/plugins/ec2/SlaveTemplate.java b/src/main/java/hudson/plugins/ec2/SlaveTemplate.java
@@ -62,8 +62,7 @@
  * @author Kohsuke Kawaguchi
  */
 public class SlaveTemplate implements Describable<SlaveTemplate> {
-    public static final String TAG_NAME_JENKINS_SLAVE_TYPE = "jenkins_slave_type";
-	public final String ami;
+    public final String ami;
     public final String description;
     public final String zone;
     public final SpotConfiguration spotConfig;
@@ -372,13 +371,13 @@ private EC2AbstractSlave provisionOndemand(TaskListener listener) throws AmazonC
                 for(EC2Tag t : tags) {
                     inst_tags.add(new Tag(t.getName(), t.getValue()));
                     diFilters.add(new Filter("tag:"+t.getName()).withValues(t.getValue()));
-                    if (StringUtils.equals(t.getName(), TAG_NAME_JENKINS_SLAVE_TYPE)) {
+                    if (StringUtils.equals(t.getName(), EC2Tag.TAG_NAME_JENKINS_SLAVE_TYPE)) {
                     	hasCustomTypeTag = true;
                     }
                 }
             }
             if (!hasCustomTypeTag) {
-            	inst_tags.add(new Tag(TAG_NAME_JENKINS_SLAVE_TYPE, "demand"));
+            	inst_tags.add(new Tag(EC2Tag.TAG_NAME_JENKINS_SLAVE_TYPE, "demand"));
             }
 
             DescribeInstancesRequest diRequest = new DescribeInstancesRequest();
@@ -596,13 +595,13 @@ private EC2AbstractSlave provisionSpot(TaskListener listener) throws AmazonClien
                 inst_tags = new HashSet<Tag>();
                 for(EC2Tag t : tags) {
                     inst_tags.add(new Tag(t.getName(), t.getValue()));
-                    if (StringUtils.equals(t.getName(), TAG_NAME_JENKINS_SLAVE_TYPE)) {
+                    if (StringUtils.equals(t.getName(), EC2Tag.TAG_NAME_JENKINS_SLAVE_TYPE)) {
                     	hasCustomTypeTag = true;
                     }
                 }
             }
             if (!hasCustomTypeTag) {
-            	inst_tags.add(new Tag(TAG_NAME_JENKINS_SLAVE_TYPE, "spot"));
+            	inst_tags.add(new Tag(EC2Tag.TAG_NAME_JENKINS_SLAVE_TYPE, "spot"));
             }
 
             if (StringUtils.isNotBlank(getIamInstanceProfile())) {