diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/compute/EC2ComputeService.java b/apis/ec2/src/main/java/org/jclouds/ec2/compute/EC2ComputeService.java index 682786e046..92d07eac78 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/compute/EC2ComputeService.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/compute/EC2ComputeService.java @@ -22,7 +22,6 @@ import static com.google.common.collect.Iterables.concat; import static com.google.common.collect.Iterables.transform; import static com.google.common.collect.Lists.newArrayList; import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static java.util.concurrent.TimeUnit.SECONDS; import static org.jclouds.compute.config.ComputeServiceProperties.RESOURCENAME_DELIMITER; import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_RUNNING; import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_SUSPENDED; @@ -108,6 +107,7 @@ public class EC2ComputeService extends BaseComputeService { private final LoadingCache securityGroupMap; private final Factory namingConvention; private final boolean generateInstanceNames; + private final Timeouts timeouts; @Inject protected EC2ComputeService(ComputeServiceContext context, Map credentialStore, @@ -140,6 +140,7 @@ public class EC2ComputeService extends BaseComputeService { this.securityGroupMap = securityGroupMap; this.namingConvention = namingConvention; this.generateInstanceNames = generateInstanceNames; + this.timeouts = timeouts; } @Override @@ -301,6 +302,7 @@ public class EC2ComputeService extends BaseComputeService { // given security group, if called very soon after the VM's state reports terminated. Empirically, it seems that // waiting a small time (e.g. enabling logging or debugging!) then the tests pass. We therefore retry. // TODO: this could be moved to a config module, also the narrative above made more concise + long timeout = timeouts.cleanupIncidentalResources; retry(new Predicate() { public boolean apply(RegionAndName input) { try { @@ -314,7 +316,7 @@ public class EC2ComputeService extends BaseComputeService { return false; } } - }, SECONDS.toMillis(3), 50, 1000, MILLISECONDS).apply(new RegionAndName(region, group)); + }, timeout, 50, 1000, MILLISECONDS).apply(new RegionAndName(region, group)); } /** diff --git a/apis/ec2/src/test/resources/delete_keypair.xml b/apis/ec2/src/test/resources/delete_keypair.xml new file mode 100644 index 0000000000..08613c528d --- /dev/null +++ b/apis/ec2/src/test/resources/delete_keypair.xml @@ -0,0 +1,4 @@ + + d4904fd9-82c2-4ea5-adfe-a9cc3EXAMPLE + true + \ No newline at end of file diff --git a/apis/ec2/src/test/resources/delete_placementgroup.xml b/apis/ec2/src/test/resources/delete_placementgroup.xml new file mode 100644 index 0000000000..7a48a14798 --- /dev/null +++ b/apis/ec2/src/test/resources/delete_placementgroup.xml @@ -0,0 +1,4 @@ + + d4904fd9-82c2-4ea5-adfe-a9cc3EXAMPLE + true + \ No newline at end of file diff --git a/apis/ec2/src/test/resources/describe_instances_empty.xml b/apis/ec2/src/test/resources/describe_instances_empty.xml new file mode 100644 index 0000000000..9fc3648b61 --- /dev/null +++ b/apis/ec2/src/test/resources/describe_instances_empty.xml @@ -0,0 +1,15 @@ + + + + r-44a5402d + UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM + + + default + + + + + + + \ No newline at end of file diff --git a/apis/ec2/src/test/resources/describe_keypairs_jcloudssingle.xml b/apis/ec2/src/test/resources/describe_keypairs_jcloudssingle.xml new file mode 100644 index 0000000000..a05079b079 --- /dev/null +++ b/apis/ec2/src/test/resources/describe_keypairs_jcloudssingle.xml @@ -0,0 +1,8 @@ + + + + jclouds#sg-3c6ef654 + 1f:51:ae:28:bf:89:e9:d8:1f:25:5d:37:2d:7d:b8:ca:9f:f5:f1:6f + + + \ No newline at end of file diff --git a/apis/sts/src/main/java/org/jclouds/aws/handlers/ParseAWSErrorFromXmlContent.java b/apis/sts/src/main/java/org/jclouds/aws/handlers/ParseAWSErrorFromXmlContent.java index b83d31e580..c298fc79a4 100644 --- a/apis/sts/src/main/java/org/jclouds/aws/handlers/ParseAWSErrorFromXmlContent.java +++ b/apis/sts/src/main/java/org/jclouds/aws/handlers/ParseAWSErrorFromXmlContent.java @@ -107,8 +107,8 @@ public class ParseAWSErrorFromXmlContent implements HttpErrorHandler { else if (errorCode != null && (errorCode.indexOf("NotFound") != -1 || errorCode.endsWith(".Unknown"))) exception = new ResourceNotFoundException(message, exception); else if ("IncorrectState".equals(errorCode) - || (errorCode != null && (error.getCode().endsWith(".Duplicate") || error.getCode().endsWith( - ".InUse"))) + || (errorCode != null && (error.getCode().endsWith(".Duplicate") + || error.getCode().endsWith(".InUse") || error.getCode().equals("DependencyViolation"))) || (message != null && (message.indexOf("already exists") != -1 || message.indexOf("is in use") != -1))) exception = new IllegalStateException(message, exception); else if (errorCode != null && errorCode.indexOf("AuthFailure") != -1) diff --git a/compute/src/main/java/org/jclouds/compute/config/ComputeServiceProperties.java b/compute/src/main/java/org/jclouds/compute/config/ComputeServiceProperties.java index 90662780bf..ef96a06ac4 100644 --- a/compute/src/main/java/org/jclouds/compute/config/ComputeServiceProperties.java +++ b/compute/src/main/java/org/jclouds/compute/config/ComputeServiceProperties.java @@ -60,6 +60,15 @@ public interface ComputeServiceProperties { */ public static final String TIMEOUT_IMAGE_DELETED = "jclouds.compute.timeout.image-deleted"; + /** + * time in milliseconds to try to clean up incidental resources + * (e.g. security groups, key-pairs, etc). + * + * Override {@link Timeouts#cleanupIncidentalResources default} by setting this property using + * {@link ContextBuilder#overrides} + */ + public static final String TIMEOUT_CLEANUP_INCIDENTAL_RESOURCES = "jclouds.compute.timeout.cleanup-incidental-resources"; + /** * overrides the default specified in the subclass of * {@link BaseComputeServiceContextModule#provideTemplate} diff --git a/compute/src/main/java/org/jclouds/compute/reference/ComputeServiceConstants.java b/compute/src/main/java/org/jclouds/compute/reference/ComputeServiceConstants.java index bc085a87af..86cb160b3e 100644 --- a/compute/src/main/java/org/jclouds/compute/reference/ComputeServiceConstants.java +++ b/compute/src/main/java/org/jclouds/compute/reference/ComputeServiceConstants.java @@ -20,6 +20,7 @@ import static org.jclouds.compute.config.ComputeServiceProperties.INIT_STATUS_MA import static org.jclouds.compute.config.ComputeServiceProperties.OS_VERSION_MAP_JSON; import static org.jclouds.compute.config.ComputeServiceProperties.POLL_INITIAL_PERIOD; import static org.jclouds.compute.config.ComputeServiceProperties.POLL_MAX_PERIOD; +import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_CLEANUP_INCIDENTAL_RESOURCES; import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_IMAGE_AVAILABLE; import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_IMAGE_DELETED; import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_RUNNING; @@ -126,6 +127,14 @@ public final class ComputeServiceConstants { @Inject(optional = true) @Named(TIMEOUT_IMAGE_AVAILABLE) public long imageAvailable = TimeUnit.MINUTES.toMillis(45); + + /** + * current value of {@link ComputeServiceProperties#TIMEOUT_CLEANUP_INCIDENTAL_RESOURCES} defaults to 3 + * seconds. + */ + @Inject(optional = true) + @Named(TIMEOUT_CLEANUP_INCIDENTAL_RESOURCES) + public long cleanupIncidentalResources = TimeUnit.SECONDS.toMillis(3); } private ComputeServiceConstants() { diff --git a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeServiceApiMockTest.java b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeServiceApiMockTest.java index 6782373fde..3d6dcf0e7f 100644 --- a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeServiceApiMockTest.java +++ b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeServiceApiMockTest.java @@ -234,4 +234,61 @@ public class AWSEC2ComputeServiceApiMockTest extends BaseAWSEC2ApiMockTest { assertPosted(DEFAULT_REGION, "Action=DescribeImages&ImageId.1=ami-aecd60c7"); assertPosted(DEFAULT_REGION, "Action=DescribeSpotInstanceRequests"); } + + public void deleteIncidentalResourcesSuccessfully() throws Exception { + enqueueRegions(DEFAULT_REGION); + enqueueXml(DEFAULT_REGION, "/describe_securitygroups_extension_single.xml"); + enqueueXml(DEFAULT_REGION, "/delete_securitygroup.xml"); + enqueueXml(DEFAULT_REGION, "/describe_keypairs_jcloudssingle.xml"); + enqueueXml(DEFAULT_REGION, "/describe_instances_empty.xml"); + enqueueXml(DEFAULT_REGION, "/delete_keypair.xml"); + enqueueXml(DEFAULT_REGION, "/describe_placement_groups.xml"); + enqueueXml(DEFAULT_REGION, "/delete_placementgroup.xml"); + enqueueXml(DEFAULT_REGION, "/describe_placement_groups_empty.xml"); + + AWSEC2ComputeService computeService = (AWSEC2ComputeService) computeService(); + + computeService.cleanUpIncidentalResources(DEFAULT_REGION, "sg-3c6ef654"); + + assertPosted(DEFAULT_REGION, "Action=DescribeRegions"); + assertPosted(DEFAULT_REGION, "Action=DescribeSecurityGroups&GroupName.1=jclouds%23sg-3c6ef654"); + assertPosted(DEFAULT_REGION, "Action=DeleteSecurityGroup&GroupName=jclouds%23sg-3c6ef654"); + assertPosted(DEFAULT_REGION, "Action=DescribeKeyPairs&Filter.1.Name=key-name&Filter.1.Value.1=jclouds%23sg-3c6ef654%23us-east-1%2A"); + assertPosted(DEFAULT_REGION, "Action=DescribeInstances&Filter.1.Name=instance-state-name&Filter.1.Value.1=terminated&Filter.1.Value.2=shutting-down&Filter.2.Name=key-name&Filter.2.Value.1=jclouds%23sg-3c6ef654"); + assertPosted(DEFAULT_REGION, "Action=DeleteKeyPair&KeyName=jclouds%23sg-3c6ef654"); + assertPosted(DEFAULT_REGION, "Action=DescribePlacementGroups&GroupName.1=jclouds%23sg-3c6ef654%23us-east-1"); + assertPosted(DEFAULT_REGION, "Action=DeletePlacementGroup&GroupName=jclouds%23sg-3c6ef654%23us-east-1"); + assertPosted(DEFAULT_REGION, "Action=DescribePlacementGroups&GroupName.1=jclouds%23sg-3c6ef654%23us-east-1"); + } + + public void deleteIncidentalResourcesGivingDependencyViolationForSecurityGroup() throws Exception { + runDeleteIncidentalResourcesGivingErrForSecurityGroup("DependencyViolation"); + } + + public void deleteIncidentalResourcesGivingInUseForSecurityGroup() throws Exception { + runDeleteIncidentalResourcesGivingErrForSecurityGroup("InvalidGroup.InUse"); + } + + protected void runDeleteIncidentalResourcesGivingErrForSecurityGroup(String errCode) throws Exception { + // Does not return delete_securitygroup.xml, but instead gives a 400 error. + // Because super.builder has set TIMEOUT_CLEANUP_INCIDENTAL_RESOURCES to 0, it will not retry. + + enqueueRegions(DEFAULT_REGION); + enqueueXml(DEFAULT_REGION, "/describe_securitygroups_extension_single.xml"); + enqueue(DEFAULT_REGION, new MockResponse().setResponseCode(400).setBody("" + errCode + "resource sg-3c6ef654 has a dependent objecte4f4c78f-4455-43dd-b5cb-9af0bc4bc804")); + enqueueXml(DEFAULT_REGION, "/describe_placement_groups.xml"); + enqueueXml(DEFAULT_REGION, "/delete_placementgroup.xml"); + enqueueXml(DEFAULT_REGION, "/describe_placement_groups_empty.xml"); + + AWSEC2ComputeService computeService = (AWSEC2ComputeService) computeService(); + + computeService.cleanUpIncidentalResources(DEFAULT_REGION, "sg-3c6ef654"); + + assertPosted(DEFAULT_REGION, "Action=DescribeRegions"); + assertPosted(DEFAULT_REGION, "Action=DescribeSecurityGroups&GroupName.1=jclouds%23sg-3c6ef654"); + assertPosted(DEFAULT_REGION, "Action=DeleteSecurityGroup&GroupName=jclouds%23sg-3c6ef654"); + assertPosted(DEFAULT_REGION, "Action=DescribePlacementGroups&GroupName.1=jclouds%23sg-3c6ef654%23us-east-1"); + assertPosted(DEFAULT_REGION, "Action=DeletePlacementGroup&GroupName=jclouds%23sg-3c6ef654%23us-east-1"); + assertPosted(DEFAULT_REGION, "Action=DescribePlacementGroups&GroupName.1=jclouds%23sg-3c6ef654%23us-east-1"); + } } diff --git a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/internal/BaseAWSEC2ApiMockTest.java b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/internal/BaseAWSEC2ApiMockTest.java index bc2170696f..e931fa0c56 100644 --- a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/internal/BaseAWSEC2ApiMockTest.java +++ b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/internal/BaseAWSEC2ApiMockTest.java @@ -22,7 +22,6 @@ import static com.google.common.net.HttpHeaders.CONTENT_TYPE; import static com.google.common.util.concurrent.MoreExecutors.sameThreadExecutor; import static javax.ws.rs.core.MediaType.APPLICATION_XML; import static org.assertj.core.api.Assertions.assertThat; -import static org.jclouds.aws.filters.FormSignerV4.ServiceAndRegion; import static org.jclouds.util.Strings2.toStringAndClose; import static org.testng.Assert.assertEquals; @@ -36,8 +35,10 @@ import org.jclouds.ContextBuilder; import org.jclouds.aws.ec2.AWSEC2Api; import org.jclouds.aws.ec2.AWSEC2ProviderMetadata; import org.jclouds.aws.ec2.config.AWSEC2HttpApiModule; +import org.jclouds.aws.filters.FormSignerV4.ServiceAndRegion; import org.jclouds.compute.ComputeService; import org.jclouds.compute.ComputeServiceContext; +import org.jclouds.compute.config.ComputeServiceProperties; import org.jclouds.concurrent.config.ExecutorServiceModule; import org.jclouds.date.DateService; import org.jclouds.rest.ConfiguresHttpApi; @@ -76,6 +77,7 @@ public class BaseAWSEC2ApiMockTest { protected ContextBuilder builder(Properties overrides) { overrides.setProperty(Constants.PROPERTY_MAX_RETRIES, "1"); + overrides.setProperty(ComputeServiceProperties.TIMEOUT_CLEANUP_INCIDENTAL_RESOURCES, "0"); return ContextBuilder.newBuilder(new AWSEC2ProviderMetadata()) .credentials(ACCESS_KEY, SECRET_KEY) .endpoint("http://localhost:" + regionToServers.get(DEFAULT_REGION).getPort()) diff --git a/providers/aws-ec2/src/test/resources/describe_placement_groups_empty.xml b/providers/aws-ec2/src/test/resources/describe_placement_groups_empty.xml new file mode 100644 index 0000000000..5e100d3f78 --- /dev/null +++ b/providers/aws-ec2/src/test/resources/describe_placement_groups_empty.xml @@ -0,0 +1,5 @@ + + d4904fd9-82c2-4ea5-adfe-a9cc3EXAMPLE + + + \ No newline at end of file