Skip to content
Permalink
Browse files
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 803dfea25551d92453e99fd39e7e366d41007592
Showing 4 changed files with 19 additions and 17 deletions.
@@ -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;
}
@@ -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;
}

@@ -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>
@@ -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>
@@ -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 -->
@@ -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);
@@ -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);
@@ -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.