Merge pull request jenkinsci#132 from sirca/feature/multiple-clouds-p…

…er-region JENKINS-24359: Overcoming limit of one cloud per region.
ngrennan-inflection · Apr 11, 2015 · cbe044e · cbe044e
2 parents 1ba2206 + 3fb3bda
commit cbe044e
Show file tree

Hide file tree

Showing 7 changed files with 74 additions and 43 deletions.
diff --git a/src/main/java/hudson/plugins/ec2/AmazonEC2Cloud.java b/src/main/java/hudson/plugins/ec2/AmazonEC2Cloud.java
@@ -25,18 +25,16 @@
 
 import hudson.Extension;
 import hudson.Util;
+import hudson.model.Failure;
 import hudson.slaves.Cloud;
 import hudson.util.FormValidation;
 import hudson.util.ListBoxModel;
 
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
-import java.util.Set;
-
 import javax.servlet.ServletException;
 
 import jenkins.model.Jenkins;
@@ -63,17 +61,30 @@ public class AmazonEC2Cloud extends EC2Cloud {
     private String region;
 
     public static final String CLOUD_ID_PREFIX = "ec2-";
-    
+
     // Used when running unit tests
     public static boolean testMode;
-    
-    
+
+
     @DataBoundConstructor
-    public AmazonEC2Cloud(boolean useInstanceProfileForCredentials, String accessId, String secretKey, String region, String privateKey, String instanceCapStr, List<? extends SlaveTemplate> templates) {
-        super(CLOUD_ID_PREFIX + region, useInstanceProfileForCredentials, accessId, secretKey, privateKey, instanceCapStr, templates);
+    public AmazonEC2Cloud(String cloudName, boolean useInstanceProfileForCredentials, String accessId, String secretKey, String region, String privateKey, String instanceCapStr, List<? extends SlaveTemplate> templates) {
+        super(createCloudId(cloudName), useInstanceProfileForCredentials, accessId, secretKey, privateKey, instanceCapStr, templates);
         this.region = region;
     }
 
+    public String getCloudName() {
+        return this.name.substring(CLOUD_ID_PREFIX.length());
+    }
+
+    @Override
+    public String getDisplayName() {
+        return getCloudName();
+    }
+
+    private static String createCloudId(String cloudName) {
+        return CLOUD_ID_PREFIX + cloudName.trim();
+    }
+
     public String getRegion() {
         if (region == null)
             region = DEFAULT_EC2_HOST; // Backward compatibility
@@ -91,7 +102,7 @@ public static URL getEc2EndpointUrl(String region) {
 			throw new Error(e); // Impossible
 		}
     }
-    
+
     @Override
     public URL getEc2EndpointUrl() {
     	return getEc2EndpointUrl(getRegion());
@@ -108,11 +119,32 @@ public URL getS3EndpointUrl() {
 
     @Extension
     public static class DescriptorImpl extends EC2Cloud.DescriptorImpl {
+
         @Override
 		public String getDisplayName() {
             return "Amazon EC2";
         }
 
+        public FormValidation doCheckCloudName(@QueryParameter String value) {
+            try {
+                Jenkins.checkGoodName(value);
+            } catch (Failure e){
+                return FormValidation.error(e.getMessage());
+            }
+
+            String cloudId = createCloudId(value);
+            int found = 0;
+            for (Cloud c : Jenkins.getInstance().clouds) {
+                if (c.name.equals(cloudId)) {
+                    found++;
+                }
+            }
+            if (found>1) {
+                return FormValidation.error(Messages.AmazonEC2Cloud_NonUniqName());
+            }
+            return FormValidation.ok();
+        }
+
 		public ListBoxModel doFillRegionItems(@QueryParameter boolean useInstanceProfileForCredentials,
 				@QueryParameter String accessId, @QueryParameter String secretKey,
 				@QueryParameter String region) throws IOException, ServletException {
@@ -123,20 +155,12 @@ public ListBoxModel doFillRegionItems(@QueryParameter boolean useInstanceProfile
 			}
 
 			if (useInstanceProfileForCredentials || (!StringUtils.isEmpty(accessId) && !StringUtils.isEmpty(secretKey))) {
-				int prefixLen = CLOUD_ID_PREFIX.length();
-				Set<String> cloudRegions = new HashSet<String>();
-				for (Cloud c : Jenkins.getInstance().clouds) {
-					cloudRegions.add(c.name.substring(prefixLen));
-				}
-
 				AWSCredentialsProvider credentialsProvider = createCredentialsProvider(useInstanceProfileForCredentials, accessId, secretKey);
 				AmazonEC2 client = connect(credentialsProvider, new URL("http://ec2.amazonaws.com"));
 				DescribeRegionsResult regions = client.describeRegions();
 				List<Region> regionList = regions.getRegions();
 				for (Region r : regionList) {
 					String name = r.getRegionName();
-					if ((region == null || !region.equals(name)) && cloudRegions.contains(name))
-						continue;
 					model.add(name, name);
 				}
 			}

diff --git a/src/main/resources/hudson/plugins/ec2/AmazonEC2Cloud/config-entries.jelly b/src/main/resources/hudson/plugins/ec2/AmazonEC2Cloud/config-entries.jelly
@@ -22,6 +22,9 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 THE SOFTWARE.
 -->
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
+  <f:entry title="${%Name}" field="cloudName">
+    <f:textbox />
+  </f:entry>
   <f:entry title="${%Access Key ID}" field="accessId">
     <f:textbox />
   </f:entry>
@@ -31,8 +34,7 @@ THE SOFTWARE.
   <f:entry title="${%Use EC2 instance profile to obtain credentials}" field="useInstanceProfileForCredentials">
     <f:checkbox />
   </f:entry>
-  <f:description>The regions will be populated once the keys above are entered. Since at most one cloud can be defined per
-  region, region names which are associated with other clouds are not shown here.
+  <f:description>The regions will be populated once the keys above are entered.
   </f:description>
   <f:entry title="${%Region}" field="region">
     <f:select/>

diff --git a/src/main/resources/hudson/plugins/ec2/EC2Cloud/computerSet.jelly b/src/main/resources/hudson/plugins/ec2/EC2Cloud/computerSet.jelly
@@ -27,7 +27,7 @@ THE SOFTWARE.
       <td />
       <td colspan="${monitors.size()+1}">
         <f:form action="${rootURL}/cloud/${it.name}/provision" method="post" name="provision">
-          <input type="submit" class="ec2-provision-button" value="${%Provision via EC2}" />
+          <input type="submit" class="ec2-provision-button" value="${%Provision via} ${it.displayName}" />
           <select name="template">
             <j:forEach var="t" items="${it.templates}">
               <option value="${t.description}">${t.displayName}</option>

diff --git a/src/main/resources/hudson/plugins/ec2/Messages.properties b/src/main/resources/hudson/plugins/ec2/Messages.properties
@@ -10,3 +10,4 @@ EC2SpotSlave.AmazonEC2SpotInstance=Amazon EC2 Spot Instance
 EC2SpotSlave.Spot1=Spot $
 EC2SpotSlave.Spot2= max bid price
 
+AmazonEC2Cloud.NonUniqName=Cloud name must be unique across EC2 clouds
diff --git a/src/test/java/hudson/plugins/ec2/AmazonEC2CloudTest.java b/src/test/java/hudson/plugins/ec2/AmazonEC2CloudTest.java
@@ -32,23 +32,25 @@
  */
 public class AmazonEC2CloudTest extends HudsonTestCase {
 
-	protected void setUp() throws Exception {
-		super.setUp();
-		AmazonEC2Cloud.testMode = true;
-	}
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        AmazonEC2Cloud.testMode = true;
+    }
 
-	protected void tearDown() throws Exception {
-		super.tearDown();
-		AmazonEC2Cloud.testMode = false;
-	}
+    @Override
+    protected void tearDown() throws Exception {
+        super.tearDown();
+        AmazonEC2Cloud.testMode = false;
+    }
 
-	public void testConfigRoundtrip() throws Exception {
-		AmazonEC2Cloud orig = new AmazonEC2Cloud(true, "abc", "def", "us-east-1",
-				"ghi", "3", Collections.<SlaveTemplate> emptyList());
-		hudson.clouds.add(orig);
-		submit(createWebClient().goTo("configure").getFormByName("config"));
+    public void testConfigRoundtrip() throws Exception {
+        AmazonEC2Cloud orig = new AmazonEC2Cloud("us-east-1", true, "abc", "def", "us-east-1",
+                "ghi", "3", Collections.<SlaveTemplate> emptyList());
+        hudson.clouds.add(orig);
+        submit(createWebClient().goTo("configure").getFormByName("config"));
 
-		assertEqualBeans(orig, hudson.clouds.iterator().next(),
-				"region,useInstanceProfileForCredentials,accessId,secretKey,privateKey,instanceCap");
-	}
+        assertEqualBeans(orig, hudson.clouds.iterator().next(),
+                "cloudName,region,useInstanceProfileForCredentials,accessId,secretKey,privateKey,instanceCap");
+    }
 }
diff --git a/src/test/java/hudson/plugins/ec2/SlaveTemplateTest.java b/src/test/java/hudson/plugins/ec2/SlaveTemplateTest.java
@@ -38,11 +38,13 @@
  */
 public class SlaveTemplateTest extends HudsonTestCase {
 
+    @Override
     protected void setUp() throws Exception {
         super.setUp();
         AmazonEC2Cloud.testMode = true;
     }
 
+    @Override
     protected void tearDown() throws Exception {
         super.tearDown();
         AmazonEC2Cloud.testMode = false;
@@ -63,7 +65,7 @@ public void testConfigRoundtrip() throws Exception {
         List<SlaveTemplate> templates = new ArrayList<SlaveTemplate>();
         templates.add(orig);
 
-        AmazonEC2Cloud ac = new AmazonEC2Cloud(false, "abc", "def", "us-east-1", "ghi", "3", templates);
+        AmazonEC2Cloud ac = new AmazonEC2Cloud("us-east-1", false, "abc", "def", "us-east-1", "ghi", "3", templates);
         hudson.clouds.add(ac);
 
         submit(createWebClient().goTo("configure").getFormByName("config"));
@@ -86,7 +88,7 @@ public void testConfigRoundtripWithPrivateDns() throws Exception {
         List<SlaveTemplate> templates = new ArrayList<SlaveTemplate>();
         templates.add(orig);
 
-        AmazonEC2Cloud ac = new AmazonEC2Cloud(false, "abc", "def", "us-east-1", "ghi", "3", templates);
+        AmazonEC2Cloud ac = new AmazonEC2Cloud("us-east-1", false, "abc", "def", "us-east-1", "ghi", "3", templates);
         hudson.clouds.add(ac);
 
         submit(createWebClient().goTo("configure").getFormByName("config"));
@@ -115,7 +117,7 @@ public void testConfigWithSpotBidPrice() throws Exception {
         List<SlaveTemplate> templates = new ArrayList<SlaveTemplate>();
         templates.add(orig);
 
-        AmazonEC2Cloud ac = new AmazonEC2Cloud(false, "abc", "def", "us-east-1", "ghi", "3", templates);
+        AmazonEC2Cloud ac = new AmazonEC2Cloud("us-east-1", false, "abc", "def", "us-east-1", "ghi", "3", templates);
         hudson.clouds.add(ac);
 
         submit(createWebClient().goTo("configure").getFormByName("config"));
@@ -143,7 +145,7 @@ public void testConfigRoundtripIamRole() throws Exception {
         List<SlaveTemplate> templates = new ArrayList<SlaveTemplate>();
         templates.add(orig);
 
-        AmazonEC2Cloud ac = new AmazonEC2Cloud(false, "abc", "def", "us-east-1", "ghi", "3", templates);
+        AmazonEC2Cloud ac = new AmazonEC2Cloud("us-east-1", false, "abc", "def", "us-east-1", "ghi", "3", templates);
         hudson.clouds.add(ac);
 
         submit(createWebClient().goTo("configure").getFormByName("config"));
@@ -199,7 +201,7 @@ public void testWindowsConfigRoundTrip() throws Exception {
         List<SlaveTemplate> templates = new ArrayList<SlaveTemplate>();
         templates.add(orig);
 
-        AmazonEC2Cloud ac = new AmazonEC2Cloud(false, "abc", "def", "us-east-1", "ghi", "3", templates);
+        AmazonEC2Cloud ac = new AmazonEC2Cloud("us-east-1", false, "abc", "def", "us-east-1", "ghi", "3", templates);
         hudson.clouds.add(ac);
 
         submit(createWebClient().goTo("configure").getFormByName("config"));
@@ -222,7 +224,7 @@ public void testUnixConfigRoundTrip() throws Exception {
         List<SlaveTemplate> templates = new ArrayList<SlaveTemplate>();
         templates.add(orig);
 
-        AmazonEC2Cloud ac = new AmazonEC2Cloud(false, "abc", "def", "us-east-1", "ghi", "3", templates);
+        AmazonEC2Cloud ac = new AmazonEC2Cloud("us-east-1", false, "abc", "def", "us-east-1", "ghi", "3", templates);
         hudson.clouds.add(ac);
 
         submit(createWebClient().goTo("configure").getFormByName("config"));

diff --git a/src/test/java/hudson/plugins/ec2/TemplateLabelsTest.java b/src/test/java/hudson/plugins/ec2/TemplateLabelsTest.java
@@ -55,7 +55,7 @@ private void setUpCloud(String label, Node.Mode mode) throws Exception{
 		List<SlaveTemplate> templates = new ArrayList<SlaveTemplate>();
 		templates.add(template);
 
-		ac = new AmazonEC2Cloud(false, "us-east-1", "abc", "def", "ghi", "3", templates);
+		ac = new AmazonEC2Cloud("us-east-1", false, "abc", "def", "us-east-1", "ghi", "3", templates);
 	}
 
 	public void testLabelAtom() throws Exception{
Original file line number	Diff line number	Diff line change
Expand Up		@@ -10,3 +10,4 @@ EC2SpotSlave.AmazonEC2SpotInstance=Amazon EC2 Spot Instance
		EC2SpotSlave.Spot1=Spot $
		EC2SpotSlave.Spot2= max bid price

		AmazonEC2Cloud.NonUniqName=Cloud name must be unique across EC2 clouds