Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Change primary template identification to be its name, not its AMI.
Many useful configurations will have multiple slave templates that use
the same AMI (but differ in other aspects). This patch changes various
parts of the code to properly identify templates by their names (in the
'description' field) instead of their AMI. As an example, the manual slave
provisioning button, before this patch, would allow the user to select a
template from a drop-down list, but then launched the slave using the
template's AMI, which could have resulted in the wrong template being
used for the launch. This patch also changes the global config page so
that the 'description' field is the first field in each template
definition, to emphasize its importance.

Addresses [JENKINS-7960] and [JENKINS-15158].
  • Loading branch information
kpfleming committed Feb 12, 2013
1 parent f0d1e98 commit 803dfea
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 17 deletions.
14 changes: 7 additions & 7 deletions src/main/java/hudson/plugins/ec2/EC2Cloud.java
Expand Up @@ -157,9 +157,9 @@ public List<SlaveTemplate> getTemplates() {
return Collections.unmodifiableList(templates);
}

public SlaveTemplate getTemplate(String ami) {
public SlaveTemplate getTemplate(String template) {
for (SlaveTemplate t : templates)
if(t.ami.equals(ami))
if(t.description.equals(template))
return t;
return null;
}
Expand Down Expand Up @@ -219,15 +219,15 @@ public void doAttach(StaplerRequest req, StaplerResponse rsp, @QueryParameter St
rsp.sendRedirect2(req.getContextPath()+"/computer/"+node.getNodeName());
}

public void doProvision(StaplerRequest req, StaplerResponse rsp, @QueryParameter String ami) throws ServletException, IOException {
public void doProvision(StaplerRequest req, StaplerResponse rsp, @QueryParameter String template) throws ServletException, IOException {
checkPermission(PROVISION);
if(ami==null) {
sendError("The 'ami' query parameter is missing",req,rsp);
if(template==null) {
sendError("The 'template' query parameter is missing",req,rsp);
return;
}
SlaveTemplate t = getTemplate(ami);
SlaveTemplate t = getTemplate(template);
if(t==null) {
sendError("No such AMI: "+ami,req,rsp);
sendError("No such template: "+template,req,rsp);
return;
}

Expand Down
Expand Up @@ -28,9 +28,9 @@ THE SOFTWARE.
<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}" />
<select name="ami">
<select name="template">
<j:forEach var="t" items="${it.templates}">
<option value="${t.ami}">${t.displayName}</option>
<option value="${t.description}">${t.displayName}</option>
</j:forEach>
</select>
<st:once>
Expand Down
Expand Up @@ -24,6 +24,10 @@ 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" >
<table width="100%">

<f:entry title="${%Description}" help="/help/system-config/master-slave/description.html" field="description">
<f:textbox />
</f:entry>

<f:entry title="${%AMI ID}" field="ami">
<f:textbox />
</f:entry>
Expand All @@ -34,10 +38,6 @@ THE SOFTWARE.
<f:enum>${it.name()}</f:enum>
</f:entry>

<f:entry title="${%Description}" help="/help/system-config/master-slave/description.html" field="description">
<f:textbox />
</f:entry>

<f:entry title="${%Availability Zone}" field="zone">
<!-- this is preferred but there is a problem with making it work FRU 22 Feb 12
See: https://groups.google.com/group/jenkinsci-dev/t/af37fa7fe2769b0c -->
Expand Down
10 changes: 6 additions & 4 deletions src/test/java/hudson/plugins/ec2/SlaveTemplateTest.java
Expand Up @@ -48,14 +48,15 @@ protected void tearDown() throws Exception {

public void testConfigRoundtrip() throws Exception {
String ami = "ami1";
String description = "foo ami";

EC2Tag tag1 = new EC2Tag( "name1", "value1" );
EC2Tag tag2 = new EC2Tag( "name2", "value2" );
List<EC2Tag> tags = new ArrayList<EC2Tag>();
tags.add( tag1 );
tags.add( tag2 );

SlaveTemplate orig = new SlaveTemplate(ami, EC2Slave.TEST_ZONE, "default", "foo", "22", InstanceType.M1Large, "ttt", Node.Mode.NORMAL, "foo ami", "bar", "aaa", "10", "rrr", "fff", "-Xmx1g", false, "subnet 456", tags, null, false, null);
SlaveTemplate orig = new SlaveTemplate(ami, EC2Slave.TEST_ZONE, "default", "foo", "22", InstanceType.M1Large, "ttt", Node.Mode.NORMAL, description, "bar", "aaa", "10", "rrr", "fff", "-Xmx1g", false, "subnet 456", tags, null, false, null);

List<SlaveTemplate> templates = new ArrayList<SlaveTemplate>();
templates.add(orig);
Expand All @@ -64,20 +65,21 @@ public void testConfigRoundtrip() throws Exception {
hudson.clouds.add(ac);

submit(createWebClient().goTo("configure").getFormByName("config"));
SlaveTemplate received = ((EC2Cloud)hudson.clouds.iterator().next()).getTemplate(ami);
SlaveTemplate received = ((EC2Cloud)hudson.clouds.iterator().next()).getTemplate(description);
assertEqualBeans(orig, received, "ami,zone,description,remoteFS,type,jvmopts,stopOnTerminate,securityGroups,subnetId,usePrivateDnsName");
}

public void testConfigRoundtripWithPrivateDns() throws Exception {
String ami = "ami1";
String description = "foo ami";

EC2Tag tag1 = new EC2Tag( "name1", "value1" );
EC2Tag tag2 = new EC2Tag( "name2", "value2" );
List<EC2Tag> tags = new ArrayList<EC2Tag>();
tags.add( tag1 );
tags.add( tag2 );

SlaveTemplate orig = new SlaveTemplate(ami, EC2Slave.TEST_ZONE, "default", "foo", "22", InstanceType.M1Large, "ttt", Node.Mode.NORMAL,"foo ami", "bar", "aaa", "10", "rrr", "fff", "-Xmx1g", false, "subnet 456", tags, null, true, null);
SlaveTemplate orig = new SlaveTemplate(ami, EC2Slave.TEST_ZONE, "default", "foo", "22", InstanceType.M1Large, "ttt", Node.Mode.NORMAL, description, "bar", "aaa", "10", "rrr", "fff", "-Xmx1g", false, "subnet 456", tags, null, true, null);

List<SlaveTemplate> templates = new ArrayList<SlaveTemplate>();
templates.add(orig);
Expand All @@ -86,7 +88,7 @@ public void testConfigRoundtripWithPrivateDns() throws Exception {
hudson.clouds.add(ac);

submit(createWebClient().goTo("configure").getFormByName("config"));
SlaveTemplate received = ((EC2Cloud)hudson.clouds.iterator().next()).getTemplate(ami);
SlaveTemplate received = ((EC2Cloud)hudson.clouds.iterator().next()).getTemplate(description);
assertEqualBeans(orig, received, "ami,zone,description,remoteFS,type,jvmopts,stopOnTerminate,securityGroups,subnetId,tags,usePrivateDnsName");
}
}

0 comments on commit 803dfea

Please sign in to comment.