From 3701e27e868e45cfafaeb93b52d0359fafd5b55b Mon Sep 17 00:00:00 2001 From: adriancole Date: Thu, 10 Jan 2013 13:51:05 -0800 Subject: [PATCH] fix issue #1149: subnet id for spot instances --- .../BindLaunchSpecificationToFormParams.java | 2 + .../AWSEC2CreateNodesInGroupThenAddToSet.java | 6 +- .../aws/ec2/domain/LaunchSpecification.java | 42 ++++++-- .../ec2/xml/LaunchSpecificationHandler.java | 2 + ...ndLaunchSpecificationToFormParamsTest.java | 9 ++ .../AWSEC2ComputeServiceExpectTest.java | 95 +++++++++++++++++++ 6 files changed, 145 insertions(+), 11 deletions(-) create mode 100644 providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeServiceExpectTest.java diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/binders/BindLaunchSpecificationToFormParams.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/binders/BindLaunchSpecificationToFormParams.java index 54c82975e8..588d8ab05c 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/binders/BindLaunchSpecificationToFormParams.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/binders/BindLaunchSpecificationToFormParams.java @@ -66,6 +66,8 @@ public class BindLaunchSpecificationToFormParams implements Binder, Function 0) options.withSecurityGroupIds(launchSpec.getSecurityGroupIds()); options.asType(checkNotNull(launchSpec.getInstanceType(), "instanceType")); + if (launchSpec.getSubnetId() != null) + options.withSubnetId(launchSpec.getSubnetId()); if (launchSpec.getKernelId() != null) options.withKernelId(launchSpec.getKernelId()); if (launchSpec.getKeyName() != null) diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/AWSEC2CreateNodesInGroupThenAddToSet.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/AWSEC2CreateNodesInGroupThenAddToSet.java index 05a86355d6..fdd3f0e85c 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/AWSEC2CreateNodesInGroupThenAddToSet.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/AWSEC2CreateNodesInGroupThenAddToSet.java @@ -96,9 +96,11 @@ public class AWSEC2CreateNodesInGroupThenAddToSet extends EC2CreateNodesInGroupT int count, Template template, RunInstancesOptions instanceOptions) { Float spotPrice = getSpotPriceOrNull(template.getOptions()); if (spotPrice != null) { + AWSEC2TemplateOptions awsOptions = AWSEC2TemplateOptions.class.cast(template.getOptions()); LaunchSpecification spec = AWSRunInstancesOptions.class.cast(instanceOptions).getLaunchSpecificationBuilder() - .imageId(template.getImage().getProviderId()).availabilityZone(zone).build(); - RequestSpotInstancesOptions options = AWSEC2TemplateOptions.class.cast(template.getOptions()).getSpotOptions(); + .imageId(template.getImage().getProviderId()).availabilityZone(zone).subnetId(awsOptions.getSubnetId()) + .build(); + RequestSpotInstancesOptions options = awsOptions.getSpotOptions(); if (logger.isDebugEnabled()) logger.debug(">> requesting %d spot instances region(%s) price(%f) spec(%s) options(%s)", count, region, spotPrice, spec, options); diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/domain/LaunchSpecification.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/domain/LaunchSpecification.java index 7b390b2150..de6c7ea822 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/domain/LaunchSpecification.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/domain/LaunchSpecification.java @@ -54,6 +54,7 @@ public class LaunchSpecification { protected String kernelId; protected String keyName; protected String availabilityZone; + protected String subnetId; protected String ramdiskId; protected Boolean monitoringEnabled; protected ImmutableSet.Builder blockDeviceMappings = ImmutableSet @@ -69,6 +70,7 @@ public class LaunchSpecification { kernelId = null; keyName = null; availabilityZone = null; + subnetId = null; ramdiskId = null; monitoringEnabled = false; blockDeviceMappings = ImmutableSet.builder(); @@ -117,7 +119,12 @@ public class LaunchSpecification { this.availabilityZone = availabilityZone; return this; } - + + public Builder subnetId(String subnetId) { + this.subnetId = subnetId; + return this; + } + public Builder ramdiskId(String ramdiskId) { this.ramdiskId = ramdiskId; return this; @@ -177,17 +184,18 @@ public class LaunchSpecification { } public LaunchSpecification build() { - return new LaunchSpecification(instanceType, imageId, kernelId, ramdiskId, availabilityZone, keyName, - securityGroupIdToNames.build(), blockDeviceMappings.build(), monitoringEnabled, + return new LaunchSpecification(instanceType, imageId, kernelId, ramdiskId, availabilityZone, subnetId, + keyName, securityGroupIdToNames.build(), blockDeviceMappings.build(), monitoringEnabled, securityGroupIds.build(), securityGroupNames.build(), userData); } public static Builder fromLaunchSpecification(LaunchSpecification in) { return new Builder().instanceType(in.getInstanceType()).imageId(in.getImageId()).kernelId(in.getKernelId()) - .ramdiskId(in.getRamdiskId()).availabilityZone(in.getAvailabilityZone()).keyName(in.getKeyName()) - .securityGroupIdToNames(in.getSecurityGroupIdToNames()).securityGroupIds(in.getSecurityGroupIds()) - .securityGroupNames(in.getSecurityGroupNames()).blockDeviceMappings(in.getBlockDeviceMappings()) - .monitoringEnabled(in.isMonitoringEnabled()).userData(in.getUserData()); + .ramdiskId(in.getRamdiskId()).availabilityZone(in.getAvailabilityZone()).subnetId(in.getSubnetId()) + .keyName(in.getKeyName()).securityGroupIdToNames(in.getSecurityGroupIdToNames()) + .securityGroupIds(in.getSecurityGroupIds()).securityGroupNames(in.getSecurityGroupNames()) + .blockDeviceMappings(in.getBlockDeviceMappings()).monitoringEnabled(in.isMonitoringEnabled()) + .userData(in.getUserData()); } } @@ -196,6 +204,7 @@ public class LaunchSpecification { protected final String kernelId; protected final String ramdiskId; protected final String availabilityZone; + protected final String subnetId; protected final String keyName; protected final Map securityGroupIdToNames; protected final Set blockDeviceMappings; @@ -205,7 +214,7 @@ public class LaunchSpecification { protected final byte[] userData; public LaunchSpecification(String instanceType, String imageId, String kernelId, String ramdiskId, - String availabilityZone, String keyName, Map securityGroupIdToNames, + String availabilityZone, String subnetId, String keyName, Map securityGroupIdToNames, Iterable blockDeviceMappings, Boolean monitoringEnabled, Set securityGroupIds, Set securityGroupNames, byte[] userData) { this.instanceType = checkNotNull(instanceType, "instanceType"); @@ -213,6 +222,7 @@ public class LaunchSpecification { this.kernelId = kernelId; this.ramdiskId = ramdiskId; this.availabilityZone = availabilityZone; + this.subnetId = subnetId; this.keyName = keyName; this.securityGroupIdToNames = ImmutableMap.copyOf(checkNotNull(securityGroupIdToNames, "securityGroupIdToNames")); this.blockDeviceMappings = ImmutableSortedSet.copyOf(checkNotNull(blockDeviceMappings, "blockDeviceMappings")); @@ -268,6 +278,14 @@ public class LaunchSpecification { public String getAvailabilityZone() { return availabilityZone; } + + /** + * The ID of the subnet in which to launch the Spot Instance. + */ + @Nullable + public String getSubnetId() { + return subnetId; + } /** * Optional. RAM disk associated with this instance. @@ -309,6 +327,7 @@ public class LaunchSpecification { final int prime = 31; int result = 1; result = prime * result + ((availabilityZone == null) ? 0 : availabilityZone.hashCode()); + result = prime * result + ((subnetId == null) ? 0 : subnetId.hashCode()); result = prime * result + ((blockDeviceMappings == null) ? 0 : blockDeviceMappings.hashCode()); result = prime * result + ((imageId == null) ? 0 : imageId.hashCode()); result = prime * result + ((instanceType == null) ? 0 : instanceType.hashCode()); @@ -337,6 +356,11 @@ public class LaunchSpecification { return false; } else if (!availabilityZone.equals(other.availabilityZone)) return false; + if (subnetId == null) { + if (other.subnetId != null) + return false; + } else if (!subnetId.equals(other.subnetId)) + return false; if (blockDeviceMappings == null) { if (other.blockDeviceMappings != null) return false; @@ -399,7 +423,7 @@ public class LaunchSpecification { @Override public String toString() { return "[instanceType=" + instanceType + ", imageId=" + imageId + ", kernelId=" + kernelId + ", ramdiskId=" - + ramdiskId + ", availabilityZone=" + availabilityZone + ", keyName=" + keyName + + ramdiskId + ", availabilityZone=" + availabilityZone + ", subnetId=" + subnetId + ", keyName=" + keyName + ", securityGroupIdToNames=" + securityGroupIdToNames + ", blockDeviceMappings=" + blockDeviceMappings + ", securityGroupIds=" + securityGroupIds + ", securityGroupNames=" + securityGroupNames + ", monitoringEnabled=" + monitoringEnabled + ", userData=" + Arrays.toString(userData) + "]"; diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/xml/LaunchSpecificationHandler.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/xml/LaunchSpecificationHandler.java index 5944b74e00..e8390ff98e 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/xml/LaunchSpecificationHandler.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/xml/LaunchSpecificationHandler.java @@ -110,6 +110,8 @@ public class LaunchSpecificationHandler extends HandlerForGeneratedRequestWithRe builder.keyName(currentOrNull()); } else if (qName.equals("availabilityZone")) { builder.availabilityZone(currentOrNull()); + } else if (qName.equals("subnetId")) { + builder.subnetId(currentOrNull()); } else if (qName.equals("ramdiskId")) { builder.ramdiskId(currentOrNull()); } else if (qName.equals("enabled")) { diff --git a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/binders/BindLaunchSpecificationToFormParamsTest.java b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/binders/BindLaunchSpecificationToFormParamsTest.java index 113c81c6ab..bfdd423093 100644 --- a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/binders/BindLaunchSpecificationToFormParamsTest.java +++ b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/binders/BindLaunchSpecificationToFormParamsTest.java @@ -66,4 +66,13 @@ public class BindLaunchSpecificationToFormParamsTest { assertEquals(binder.apply(spec), ImmutableMap.of("LaunchSpecification.InstanceType", "t1.micro", "LaunchSpecification.ImageId", "ami-123", "LaunchSpecification.SecurityGroupId.1", "sid-foo")); } + + @Test + public void testApplyWithSubnetId() throws UnknownHostException { + LaunchSpecification spec = LaunchSpecification.builder().instanceType(InstanceType.T1_MICRO).imageId("ami-123") + .subnetId("subnet-xyz").build(); + + assertEquals(binder.apply(spec), ImmutableMap.of("LaunchSpecification.InstanceType", "t1.micro", + "LaunchSpecification.ImageId", "ami-123", "LaunchSpecification.SubnetId", "subnet-xyz")); + } } diff --git a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeServiceExpectTest.java b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeServiceExpectTest.java new file mode 100644 index 0000000000..45004a9906 --- /dev/null +++ b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeServiceExpectTest.java @@ -0,0 +1,95 @@ +/** + * Licensed to jclouds, Inc. (jclouds) under one or more + * contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. jclouds licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.jclouds.aws.ec2.compute; + +import static org.testng.Assert.assertEquals; + +import javax.ws.rs.core.MediaType; + +import org.jclouds.aws.ec2.compute.internal.BaseAWSEC2ComputeServiceExpectTest; +import org.jclouds.compute.ComputeService; +import org.jclouds.compute.domain.NodeMetadata; +import org.jclouds.compute.domain.Template; +import org.jclouds.http.HttpRequest; +import org.jclouds.http.HttpResponse; +import org.testng.annotations.Test; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableMap.Builder; +import com.google.common.collect.Iterables; + +/** + * Tests the compute service abstraction of the EC2 api. + * + * @author Adrian Cole + */ +@Test(groups = "unit", testName = "AWSEC2ComputeServiceExpectTest") +public class AWSEC2ComputeServiceExpectTest extends BaseAWSEC2ComputeServiceExpectTest { + + public void testLaunchVPCSpotInstanceMissesVPCId() throws Exception { + HttpRequest requestSpotInstancesRequest = formSigner.filter(HttpRequest.builder().method("POST") + .endpoint("https://ec2." + region + ".amazonaws.com/") + .addHeader("Host", "ec2." + region + ".amazonaws.com") + .addFormParam("Action", "RequestSpotInstances") + .addFormParam("InstanceCount", "1") + .addFormParam("LaunchSpecification.ImageId", "ami-be3adfd7") + .addFormParam("LaunchSpecification.InstanceType", "m1.small") + .addFormParam("LaunchSpecification.KeyName", "Demo") + .addFormParam("LaunchSpecification.Placement.AvailabilityZone", "us-east-1a") + .addFormParam("LaunchSpecification.SubnetId", "subnet-xyz") + .addFormParam("LaunchSpecification.UserData", "I2Nsb3VkLWNvbmZpZwpyZXBvX3VwZ3JhZGU6IG5vbmUK") + .addFormParam("SpotPrice", "1.0").build()); + + HttpResponse requestSpotInstancesResponse = HttpResponse.builder().statusCode(200) + .payload(payloadFromResourceWithContentType( + "/request_spot_instances-ebs.xml", MediaType.APPLICATION_XML)).build(); + + HttpRequest describeSpotInstanceRequest = formSigner.filter(HttpRequest.builder().method("POST") + .endpoint("https://ec2." + region + ".amazonaws.com/") + .addHeader("Host", "ec2." + region + ".amazonaws.com") + .addFormParam("Action", "DescribeSpotInstanceRequests") + .addFormParam("SpotInstanceRequestId.1", "sir-228e6406").build()); + + HttpResponse describeSpotInstanceResponse = HttpResponse.builder().statusCode(200) + .payload(payloadFromResourceWithContentType( + "/request_spot_instances-ebs.xml", MediaType.APPLICATION_XML)).build(); + + Builder requestResponseMap = ImmutableMap. builder(); + requestResponseMap.put(describeRegionsRequest, describeRegionsResponse); + requestResponseMap.put(describeAvailabilityZonesRequest, describeAvailabilityZonesResponse); + requestResponseMap.put(describeImagesRequest, describeImagesResponse); + requestResponseMap.put(createKeyPairRequest, createKeyPairResponse); + requestResponseMap.put(createSecurityGroupRequest, createSecurityGroupResponse); + requestResponseMap.put(describeSecurityGroupRequest, describeSecurityGroupResponse); + requestResponseMap.put(authorizeSecurityGroupIngressRequest22, authorizeSecurityGroupIngressResponse); + requestResponseMap.put(authorizeSecurityGroupIngressRequestGroup, authorizeSecurityGroupIngressResponse); + requestResponseMap.put(requestSpotInstancesRequest, requestSpotInstancesResponse); + requestResponseMap.put(describeSpotInstanceRequest, describeSpotInstanceResponse); + + ComputeService createsVPCSpotInstance = requestsSendResponses(requestResponseMap.build()); + + Template template = createsVPCSpotInstance.templateBuilder().locationId("us-east-1a").build(); + + template.getOptions().as(AWSEC2TemplateOptions.class).spotPrice(1f).subnetId("subnet-xyz").keyPair("Demo").blockUntilRunning(false); + + NodeMetadata node = Iterables.getOnlyElement(createsVPCSpotInstance.createNodesInGroup("demoGroup", 1, template)); + assertEquals(node.getId(), "us-east-1/sir-228e6406"); + } + +}