From 94ac48070d4fcc983ba66b28d7c08fd5593f4811 Mon Sep 17 00:00:00 2001 From: Aled Sage Date: Thu, 10 May 2012 16:47:37 +0100 Subject: [PATCH] Issue 888: use GroupNamingConvention for keyName/securityGroup; don't include region in name --- .../ec2/compute/EC2ComputeService.java | 22 ++++---- .../functions/CreateUniqueKeyPair.java | 27 ++++------ .../RunningInstanceToNodeMetadata.java | 28 ++++------ ...rityGroupsAsNeededAndReturnRunOptions.java | 15 +++--- .../functions/CreateUniqueKeyPairTest.java | 51 +++++++++++++------ .../RunningInstanceToNodeMetadataTest.java | 21 ++++++-- ...GroupsAsNeededAndReturnRunOptionsTest.java | 18 +++++-- .../functions/GroupNamingConvention.java | 11 +++- ...dAppendUniqueStringToThoseWhichRepeat.java | 33 ++++++++++-- .../aws/ec2/compute/AWSEC2ComputeService.java | 5 +- .../AWSRunningInstanceToNodeMetadata.java | 6 ++- ...rityGroupsAsNeededAndReturnRunOptions.java | 6 ++- .../compute/AWSEC2ComputeServiceLiveTest.java | 7 ++- .../AWSRunningInstanceToNodeMetadataTest.java | 24 +++++++-- ...GroupsAsNeededAndReturnRunOptionsTest.java | 20 +++++--- 15 files changed, 196 insertions(+), 98 deletions(-) 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 c30ec5a7e8..57816cd25e 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 @@ -45,6 +45,8 @@ import org.jclouds.compute.domain.Hardware; import org.jclouds.compute.domain.Image; import org.jclouds.compute.domain.NodeMetadata; import org.jclouds.compute.domain.TemplateBuilder; +import org.jclouds.compute.functions.GroupNamingConvention; +import org.jclouds.compute.functions.GroupNamingConvention.Factory; import org.jclouds.compute.internal.BaseComputeService; import org.jclouds.compute.internal.PersistNodeCredentials; import org.jclouds.compute.options.TemplateOptions; @@ -87,6 +89,7 @@ public class EC2ComputeService extends BaseComputeService { private final EC2Client ec2Client; private final ConcurrentMap credentialsMap; private final LoadingCache securityGroupMap; + private final Factory namingConvention; @Inject protected EC2ComputeService(ComputeServiceContext context, Map credentialStore, @@ -104,7 +107,7 @@ public class EC2ComputeService extends BaseComputeService { PersistNodeCredentials persistNodeCredentials, Timeouts timeouts, @Named(Constants.PROPERTY_USER_THREADS) ExecutorService executor, EC2Client ec2Client, ConcurrentMap credentialsMap, @Named("SECURITY") LoadingCache securityGroupMap, - Optional imageExtension) { + Optional imageExtension, GroupNamingConvention.Factory namingConvention) { super(context, credentialStore, images, sizes, locations, listNodesStrategy, getNodeMetadataStrategy, runNodesAndAddToSetStrategy, rebootNodeStrategy, destroyNodeStrategy, startNodeStrategy, stopNodeStrategy, templateBuilderProvider, templateOptionsProvider, nodeRunning, nodeTerminated, nodeSuspended, @@ -113,6 +116,7 @@ public class EC2ComputeService extends BaseComputeService { this.ec2Client = ec2Client; this.credentialsMap = credentialsMap; this.securityGroupMap = securityGroupMap; + this.namingConvention = namingConvention; } @Inject(optional = true) @@ -126,7 +130,8 @@ public class EC2ComputeService extends BaseComputeService { void deleteSecurityGroup(String region, String group) { checkNotEmpty(region, "region"); checkNotEmpty(group, "group"); - String groupName = String.format("jclouds#%s#%s", group, region).replace('#', delimiter); + String groupName = namingConvention.create().sharedNameForGroup(group); + if (ec2Client.getSecurityGroupServices().describeSecurityGroupsInRegion(region, groupName).size() > 0) { logger.debug(">> deleting securityGroup(%s)", groupName); ec2Client.getSecurityGroupServices().deleteSecurityGroupInRegion(region, groupName); @@ -139,13 +144,12 @@ public class EC2ComputeService extends BaseComputeService { @VisibleForTesting void deleteKeyPair(String region, String group) { for (KeyPair keyPair : ec2Client.getKeyPairServices().describeKeyPairsInRegion(region)) { - if ( - // when the keypair is unique per group - keyPair.getKeyName().equals("jclouds"+ delimiter + group) - || keyPair.getKeyName().matches(String.format("jclouds#%s#%s", group, "[0-9a-f]+").replace('#', delimiter)) - // old keypair pattern too verbose as it has an unnecessary - // region qualifier - || keyPair.getKeyName().matches(String.format("jclouds#%s#%s#%s", group, region, "[0-9a-f]+").replace('#', delimiter))) { + String keyName = keyPair.getKeyName(); + Predicate keyNameMatcher = namingConvention.create().containsGroup(group); + String oldKeyNameRegex = String.format("jclouds#%s#%s#%s", group, region, "[0-9a-f]+").replace('#', delimiter); + // old keypair pattern too verbose as it has an unnecessary region qualifier + + if (keyNameMatcher.apply(keyName) || keyName.matches(oldKeyNameRegex)) { Set instancesUsingKeyPair = extractIdsFromInstances(filter(concat(ec2Client.getInstanceServices() .describeInstancesInRegion(region)), usingKeyPairAndNotDead(keyPair))); if (instancesUsingKeyPair.size() > 0) { diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/CreateUniqueKeyPair.java b/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/CreateUniqueKeyPair.java index 2d7e0190f2..e11771395b 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/CreateUniqueKeyPair.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/CreateUniqueKeyPair.java @@ -19,12 +19,12 @@ package org.jclouds.ec2.compute.functions; import static com.google.common.base.Preconditions.checkNotNull; -import static org.jclouds.compute.config.ComputeServiceProperties.RESOURCENAME_DELIMITER; import javax.annotation.Resource; import javax.inject.Named; import javax.inject.Singleton; +import org.jclouds.compute.functions.GroupNamingConvention; import org.jclouds.compute.reference.ComputeServiceConstants; import org.jclouds.ec2.EC2Client; import org.jclouds.ec2.compute.domain.RegionAndName; @@ -33,7 +33,6 @@ import org.jclouds.logging.Logger; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; -import com.google.common.base.Supplier; import com.google.inject.Inject; /** @@ -46,12 +45,12 @@ public class CreateUniqueKeyPair implements Function { @Named(ComputeServiceConstants.COMPUTE_LOGGER) protected Logger logger = Logger.NULL; protected final EC2Client ec2Client; - protected final Supplier randomSuffix; + protected final GroupNamingConvention.Factory namingConvention; @Inject - public CreateUniqueKeyPair(EC2Client ec2Client, Supplier randomSuffix) { + public CreateUniqueKeyPair(EC2Client ec2Client, GroupNamingConvention.Factory namingConvention) { this.ec2Client = ec2Client; - this.randomSuffix = randomSuffix; + this.namingConvention = checkNotNull(namingConvention, "namingConvention"); } @Override @@ -65,22 +64,18 @@ public class CreateUniqueKeyPair implements Function { checkNotNull(group, "group"); logger.debug(">> creating keyPair region(%s) group(%s)", region, group); KeyPair keyPair = null; + String prefix = group; + while (keyPair == null) { + String keyName = namingConvention.create().uniqueNameForGroup(prefix); try { - keyPair = ec2Client.getKeyPairServices().createKeyPairInRegion(region, getNextName(region, group)); - logger.debug("<< created keyPair(%s)", keyPair); + keyPair = ec2Client.getKeyPairServices().createKeyPairInRegion(region, keyName); } catch (IllegalStateException e) { - + logger.trace(" invalid keyname (%s in %s); retrying", keyName, region); } } + + logger.debug("<< created keyPair(%s)", keyPair); return keyPair; } - - @Inject(optional=true) - @Named(RESOURCENAME_DELIMITER) - char delimiter = '#'; - - private String getNextName(String region, String group) { - return String.format("jclouds#%s#%s#%s", group, region, randomSuffix.get()).replace('#', delimiter); - } } diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/RunningInstanceToNodeMetadata.java b/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/RunningInstanceToNodeMetadata.java index ae286b251c..fa66954a23 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/RunningInstanceToNodeMetadata.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/RunningInstanceToNodeMetadata.java @@ -21,16 +21,14 @@ package org.jclouds.ec2.compute.functions; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Predicates.not; import static com.google.common.collect.Iterables.filter; -import static org.jclouds.compute.config.ComputeServiceProperties.RESOURCENAME_DELIMITER; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.NoSuchElementException; import java.util.Set; -import java.util.Map.Entry; import javax.annotation.Resource; -import javax.inject.Named; import javax.inject.Singleton; import org.jclouds.collect.Memoized; @@ -42,6 +40,7 @@ import org.jclouds.compute.domain.NodeMetadataBuilder; import org.jclouds.compute.domain.NodeState; import org.jclouds.compute.domain.Volume; import org.jclouds.compute.domain.internal.VolumeImpl; +import org.jclouds.compute.functions.GroupNamingConvention; import org.jclouds.domain.Credentials; import org.jclouds.domain.Location; import org.jclouds.domain.LoginCredentials; @@ -61,10 +60,10 @@ import com.google.common.base.Supplier; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSet.Builder; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Sets; -import com.google.common.collect.ImmutableSet.Builder; import com.google.common.util.concurrent.UncheckedExecutionException; import com.google.inject.Inject; @@ -82,16 +81,19 @@ public class RunningInstanceToNodeMetadata implements Function> imageMap; protected final Map credentialStore; protected final Map instanceToNodeState; + protected final GroupNamingConvention.Factory namingConvention; @Inject protected RunningInstanceToNodeMetadata(Map instanceToNodeState, Map credentialStore, Supplier> imageMap, - @Memoized Supplier> locations, @Memoized Supplier> hardware) { + @Memoized Supplier> locations, @Memoized Supplier> hardware, + GroupNamingConvention.Factory namingConvention) { this.locations = checkNotNull(locations, "locations"); this.hardware = checkNotNull(hardware, "hardware"); this.imageMap = checkNotNull(imageMap, "imageMap"); this.instanceToNodeState = checkNotNull(instanceToNodeState, "instanceToNodeState"); this.credentialStore = checkNotNull(credentialStore, "credentialStore"); + this.namingConvention = checkNotNull(namingConvention, "namingConvention"); } @Override @@ -197,24 +199,16 @@ public class RunningInstanceToNodeMetadata implements Function data) { String group = null; try { - group = Iterables.getOnlyElement(Iterables.filter(data, new Predicate() { - - @Override - public boolean apply(String input) { - return input.startsWith("jclouds" + delimiter) && input.contains(delimiter + instance.getRegion()); - } - })).split(delimiter + "")[1]; + Predicate containsAnyGroup = namingConvention.create().containsAnyGroup(); + String encodedGroup = Iterables.getOnlyElement(Iterables.filter(data, containsAnyGroup)); + group = namingConvention.create().extractGroup(encodedGroup); } catch (NoSuchElementException e) { logger.debug("no group parsed from %s's data: %s", instance.getId(), data); } catch (IllegalArgumentException e) { - logger.debug("too many groups match %s%s; %s's data: %s", "jclouds", delimiter, instance.getId(), data); + logger.debug("too many groups match naming convention; %s's data: %s", instance.getId(), data); } return group; } diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions.java b/apis/ec2/src/main/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions.java index 31b2a03f5a..75b29e79e1 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions.java @@ -21,7 +21,6 @@ package org.jclouds.ec2.compute.strategy; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; -import static org.jclouds.compute.config.ComputeServiceProperties.RESOURCENAME_DELIMITER; import static org.jclouds.crypto.SshKeys.fingerprintPrivateKey; import static org.jclouds.crypto.SshKeys.sha1PrivateKey; @@ -33,6 +32,8 @@ import javax.inject.Provider; import javax.inject.Singleton; import org.jclouds.compute.domain.Template; +import org.jclouds.compute.functions.GroupNamingConvention; +import org.jclouds.compute.functions.GroupNamingConvention.Factory; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.ec2.compute.domain.RegionAndName; import org.jclouds.ec2.compute.domain.RegionNameAndIngressRules; @@ -63,16 +64,19 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions { public final LoadingCache securityGroupMap; @VisibleForTesting public final Provider optionsProvider; + private final Factory namingConvention; @Inject public CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions(Function makeKeyPair, ConcurrentMap credentialsMap, @Named("SECURITY") LoadingCache securityGroupMap, - Provider optionsProvider) { + Provider optionsProvider, + GroupNamingConvention.Factory namingConvention) { this.makeKeyPair = checkNotNull(makeKeyPair, "makeKeyPair"); this.credentialsMap = checkNotNull(credentialsMap, "credentialsMap"); this.securityGroupMap = checkNotNull(securityGroupMap, "securityGroupMap"); this.optionsProvider = checkNotNull(optionsProvider, "optionsProvider"); + this.namingConvention = checkNotNull(namingConvention, "namingConvention"); } public RunInstancesOptions execute(String region, String group, Template template) { @@ -160,16 +164,13 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions { return keyPair.getKeyName(); } - @Inject(optional = true) - @Named(RESOURCENAME_DELIMITER) - char delimiter = '#'; - @VisibleForTesting public Set getSecurityGroupsForTagAndOptions(String region, @Nullable String group, TemplateOptions options) { Builder groups = ImmutableSet.builder(); if (group != null) { - String markerGroup = String.format("jclouds#%s#%s", group, region).replace('#', delimiter); + String markerGroup = namingConvention.create().sharedNameForGroup(group); + groups.add(markerGroup); RegionNameAndIngressRules regionNameAndIngessRulesForMarkerGroup; diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/CreateUniqueKeyPairTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/CreateUniqueKeyPairTest.java index 45fa06b887..1cfe6fc1d3 100644 --- a/apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/CreateUniqueKeyPairTest.java +++ b/apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/CreateUniqueKeyPairTest.java @@ -26,73 +26,92 @@ import static org.testng.Assert.assertEquals; import java.net.UnknownHostException; +import org.jclouds.ec2.EC2ApiMetadata; import org.jclouds.ec2.EC2Client; import org.jclouds.ec2.domain.KeyPair; import org.jclouds.ec2.services.KeyPairClient; import org.testng.annotations.Test; import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.TypeLiteral; +import com.google.inject.name.Names; /** * @author Adrian Cole */ @Test(groups = "unit", testName = "CreateUniqueKeyPairTest") public class CreateUniqueKeyPairTest { - @SuppressWarnings( { "unchecked" }) + @Test public void testApply() throws UnknownHostException { - EC2Client client = createMock(EC2Client.class); + final EC2Client client = createMock(EC2Client.class); KeyPairClient keyClient = createMock(KeyPairClient.class); - Supplier uniqueIdSupplier = createMock(Supplier.class); - KeyPair pair = createMock(KeyPair.class); expect(client.getKeyPairServices()).andReturn(keyClient).atLeastOnce(); - expect(uniqueIdSupplier.get()).andReturn("1"); - expect(keyClient.createKeyPairInRegion("region", "jclouds#group#region#1")).andReturn(pair); + expect(keyClient.createKeyPairInRegion("region", "jclouds#group#1")).andReturn(pair); replay(client); replay(keyClient); - replay(uniqueIdSupplier); - CreateUniqueKeyPair parser = new CreateUniqueKeyPair(client, uniqueIdSupplier); + CreateUniqueKeyPair parser = Guice.createInjector(new AbstractModule() { + + @Override + protected void configure() { + Names.bindProperties(binder(),new EC2ApiMetadata().getDefaultProperties()); + bind(new TypeLiteral>() { + }).toInstance(Suppliers.ofInstance("1")); + bind(EC2Client.class).toInstance(client); + } + + }).getInstance(CreateUniqueKeyPair.class); assertEquals(parser.createNewKeyPairInRegion("region", "group"), pair); verify(client); verify(keyClient); - verify(uniqueIdSupplier); } @SuppressWarnings( { "unchecked" }) @Test public void testApplyWithIllegalStateException() throws UnknownHostException { - EC2Client client = createMock(EC2Client.class); + final EC2Client client = createMock(EC2Client.class); KeyPairClient keyClient = createMock(KeyPairClient.class); - Supplier uniqueIdSupplier = createMock(Supplier.class); + final Supplier uniqueIdSupplier = createMock(Supplier.class); KeyPair pair = createMock(KeyPair.class); expect(client.getKeyPairServices()).andReturn(keyClient).atLeastOnce(); expect(uniqueIdSupplier.get()).andReturn("1"); - expect(keyClient.createKeyPairInRegion("region", "jclouds#group#region#1")).andThrow(new IllegalStateException()); + expect(keyClient.createKeyPairInRegion("region", "jclouds#group#1")).andThrow(new IllegalStateException()); expect(uniqueIdSupplier.get()).andReturn("2"); - expect(keyClient.createKeyPairInRegion("region", "jclouds#group#region#2")).andReturn(pair); + expect(keyClient.createKeyPairInRegion("region", "jclouds#group#2")).andReturn(pair); replay(client); replay(keyClient); replay(uniqueIdSupplier); - CreateUniqueKeyPair parser = new CreateUniqueKeyPair(client, uniqueIdSupplier); + CreateUniqueKeyPair parser = Guice.createInjector(new AbstractModule() { + + @Override + protected void configure() { + Names.bindProperties(binder(),new EC2ApiMetadata().getDefaultProperties()); + bind(new TypeLiteral>() { + }).toInstance(uniqueIdSupplier); + bind(EC2Client.class).toInstance(client); + } + + }).getInstance(CreateUniqueKeyPair.class); assertEquals(parser.createNewKeyPairInRegion("region", "group"), pair); verify(client); verify(keyClient); verify(uniqueIdSupplier); - } - } diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/RunningInstanceToNodeMetadataTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/RunningInstanceToNodeMetadataTest.java index 7370e74ddf..9cdd4205cc 100644 --- a/apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/RunningInstanceToNodeMetadataTest.java +++ b/apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/RunningInstanceToNodeMetadataTest.java @@ -32,11 +32,13 @@ import org.jclouds.compute.domain.NodeMetadataBuilder; import org.jclouds.compute.domain.NodeState; import org.jclouds.compute.domain.OperatingSystem; import org.jclouds.compute.domain.OsFamily; +import org.jclouds.compute.functions.GroupNamingConvention; import org.jclouds.domain.Credentials; import org.jclouds.domain.Location; import org.jclouds.domain.LocationBuilder; import org.jclouds.domain.LocationScope; import org.jclouds.domain.LoginCredentials; +import org.jclouds.ec2.EC2ApiMetadata; import org.jclouds.ec2.compute.config.EC2ComputeServiceDependenciesModule; import org.jclouds.ec2.compute.domain.RegionAndName; import org.jclouds.ec2.domain.InstanceState; @@ -53,6 +55,9 @@ import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.name.Names; /** * @author Adrian Cole @@ -221,14 +226,14 @@ public class RunningInstanceToNodeMetadataTest { public void testGroupNameIsSetWhenCustomKeyNameIsSetAndSecurityGroupIsGenerated() { checkGroupName(RunningInstance.builder().instanceId("id").imageId("image").instanceType("m1.small") .instanceState(InstanceState.RUNNING).region("us-east-1").keyName("custom-key") - .groupId("jclouds#groupname#us-east-1").build()); + .groupId("jclouds#groupname").build()); } @Test public void testGroupNameIsSetWhenCustomSecurityGroupIsSetAndKeyNameIsGenerated() { checkGroupName(RunningInstance.builder().instanceId("id").imageId("image").instanceType("m1.small") .instanceState(InstanceState.RUNNING).region("us-east-1").groupId("custom-sec") - .keyName("jclouds#groupname#us-east-1#23").build()); + .keyName("jclouds#groupname#23").build()); } protected RunningInstance firstInstanceFromResource(String resource) { @@ -278,9 +283,19 @@ public class RunningInstanceToNodeMetadataTest { } }; + + GroupNamingConvention.Factory namingConvention = Guice.createInjector(new AbstractModule() { + + @Override + protected void configure() { + Names.bindProperties(binder(),new EC2ApiMetadata().getDefaultProperties()); + } + + }).getInstance(GroupNamingConvention.Factory.class); + RunningInstanceToNodeMetadata parser = new RunningInstanceToNodeMetadata(instanceToNodeState, credentialStore, Suppliers.> ofInstance(instanceToImage), locationSupplier, - hardwareSupplier); + hardwareSupplier, namingConvention); return parser; } diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java index d07797015e..c81e0fedee 100644 --- a/apis/ec2/src/test/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java +++ b/apis/ec2/src/test/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java @@ -34,6 +34,7 @@ import javax.inject.Provider; import org.jclouds.aws.domain.Region; import org.jclouds.compute.domain.Hardware; import org.jclouds.compute.domain.Template; +import org.jclouds.compute.functions.GroupNamingConvention; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.domain.LoginCredentials; import org.jclouds.ec2.compute.domain.EC2HardwareBuilder; @@ -416,7 +417,7 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest { // setup constants String region = Region.AP_SOUTHEAST_1; String group = "group"; - String generatedMarkerGroup = "jclouds#group#" + Region.AP_SOUTHEAST_1; + String generatedMarkerGroup = "jclouds#group"; Set groupIds = ImmutableSet. of(); int[] ports = new int[] {}; boolean shouldAuthorizeSelf = true; @@ -450,7 +451,7 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest { // setup constants String region = Region.AP_SOUTHEAST_1; String group = "group"; - String generatedMarkerGroup = "jclouds#group#" + Region.AP_SOUTHEAST_1; + String generatedMarkerGroup = "jclouds#group"; Set groupIds = ImmutableSet. of(); int[] ports = new int[] { 22, 80 }; boolean shouldAuthorizeSelf = true; @@ -484,7 +485,7 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest { // setup constants String region = Region.AP_SOUTHEAST_1; String group = "group"; - String generatedMarkerGroup = "jclouds#group#" + Region.AP_SOUTHEAST_1; + String generatedMarkerGroup = "jclouds#group"; Set groupIds = ImmutableSet. of(); int[] ports = new int[] {}; boolean shouldAuthorizeSelf = true; @@ -517,7 +518,7 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest { // setup constants String region = Region.AP_SOUTHEAST_1; String group = "group"; - String generatedMarkerGroup = "jclouds#group#" + Region.AP_SOUTHEAST_1; + String generatedMarkerGroup = "jclouds#group"; Set groupIds = ImmutableSet. of("group1", "group2"); int[] ports = new int[] {}; boolean shouldAuthorizeSelf = true; @@ -559,8 +560,15 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest { Function makeKeyPair = createMock(Function.class); ConcurrentMap credentialsMap = createMock(ConcurrentMap.class); LoadingCache securityGroupMap = createMock(LoadingCache.class); + GroupNamingConvention.Factory namingConventionFactory = createMock(GroupNamingConvention.Factory.class); + GroupNamingConvention namingConvention = createMock(GroupNamingConvention.class); + expect(namingConventionFactory.create()).andReturn(namingConvention).anyTimes(); + expect(namingConvention.sharedNameForGroup("group")).andReturn("jclouds#group").anyTimes(); + replay(namingConventionFactory); + replay(namingConvention); + return new CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions(makeKeyPair, credentialsMap, - securityGroupMap, OPTIONS_PROVIDER); + securityGroupMap, OPTIONS_PROVIDER, namingConventionFactory); } private void replayStrategy(CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions strategy) { diff --git a/compute/src/main/java/org/jclouds/compute/functions/GroupNamingConvention.java b/compute/src/main/java/org/jclouds/compute/functions/GroupNamingConvention.java index 0fc1867f2a..1e785c8942 100644 --- a/compute/src/main/java/org/jclouds/compute/functions/GroupNamingConvention.java +++ b/compute/src/main/java/org/jclouds/compute/functions/GroupNamingConvention.java @@ -156,8 +156,17 @@ public interface GroupNamingConvention { String groupInSharedNameOrNull(String encoded); /** - * identifies if this name has a group encoded in it. + * A predicate that identifies if an input has the given group encoded in it. */ Predicate containsGroup(String group); + /** + * A predicate that identifies if an input has any group encoded in it. + */ + Predicate containsAnyGroup(); + + /** + * Extracts the group from a shared/unique name. Or returns null if not in correct format to contain a group. + */ + String extractGroup(String encoded); } \ No newline at end of file diff --git a/compute/src/main/java/org/jclouds/compute/internal/FormatSharedNamesAndAppendUniqueStringToThoseWhichRepeat.java b/compute/src/main/java/org/jclouds/compute/internal/FormatSharedNamesAndAppendUniqueStringToThoseWhichRepeat.java index 5edb0ebabb..0ff449939b 100644 --- a/compute/src/main/java/org/jclouds/compute/internal/FormatSharedNamesAndAppendUniqueStringToThoseWhichRepeat.java +++ b/compute/src/main/java/org/jclouds/compute/internal/FormatSharedNamesAndAppendUniqueStringToThoseWhichRepeat.java @@ -111,9 +111,8 @@ public class FormatSharedNamesAndAppendUniqueStringToThoseWhichRepeat implements this.delimiter = delimiter; this.suffixSupplier = checkNotNull(suffixSupplier, "suffixSupplier"); this.groupValidator = checkNotNull(groupValidator, "groupValidator"); - this.sharedFormat = "".equals(prefix) ? "%s" : new StringBuilder().append(prefix).append(delimiter).append("%s") - .toString(); - this.uniqueFormat = new StringBuilder(sharedFormat).append(delimiter).append("%s").toString(); + this.sharedFormat = "".equals(prefix) ? "%s" : prefix + delimiter + "%s"; + this.uniqueFormat = sharedFormat + delimiter + "%s"; this.uniqueGroupPattern = Pattern.compile("^" + ("".equals(prefix) ? "" : (prefix + delimiter)) + "(.+)" + delimiter + "[^" + delimiter + "]+"); this.sharedGroupPattern = Pattern.compile("^" + ("".equals(prefix) ? "" : (prefix + delimiter)) + "(.+)$"); @@ -172,4 +171,32 @@ public class FormatSharedNamesAndAppendUniqueStringToThoseWhichRepeat implements }; } + @Override + public Predicate containsAnyGroup() { + return new Predicate() { + + @Override + public boolean apply(String input) { + try { + return groupInUniqueNameOrNull(input) != null || groupInSharedNameOrNull(input) != null; + } catch (NoSuchElementException e) { + return false; + } + } + + @Override + public String toString() { + return "containsAnyGroup()"; + } + }; + } + + @Override + public String extractGroup(String encoded) { + String result = groupInUniqueNameOrNull(encoded); + if (result != null) return result; + + result = groupInSharedNameOrNull(encoded); + return result; + } } diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeService.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeService.java index eccd369768..c838f9daa3 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeService.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeService.java @@ -51,6 +51,7 @@ import org.jclouds.compute.domain.NodeMetadata; import org.jclouds.compute.domain.NodeMetadataBuilder; import org.jclouds.compute.domain.Template; import org.jclouds.compute.domain.TemplateBuilder; +import org.jclouds.compute.functions.GroupNamingConvention; import org.jclouds.compute.internal.PersistNodeCredentials; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.reference.ComputeServiceConstants.Timeouts; @@ -114,12 +115,12 @@ public class AWSEC2ComputeService extends EC2ComputeService { @Named("PLACEMENT") LoadingCache placementGroupMap, @Named("DELETED") Predicate placementGroupDeleted, @Named(PROPERTY_EC2_GENERATE_INSTANCE_NAMES) boolean generateInstanceNames, AWSEC2AsyncClient aclient, - Optional imageExtension) { + Optional imageExtension, GroupNamingConvention.Factory namingConvention) { super(context, credentialStore, images, sizes, locations, listNodesStrategy, getNodeMetadataStrategy, runNodesAndAddToSetStrategy, rebootNodeStrategy, destroyNodeStrategy, startNodeStrategy, stopNodeStrategy, templateBuilderProvider, templateOptionsProvider, nodeRunning, nodeTerminated, nodeSuspended, initScriptRunnerFactory, runScriptOnNodeFactory, initAdminAccess, persistNodeCredentials, - timeouts, executor, ec2Client, credentialsMap, securityGroupMap, imageExtension); + timeouts, executor, ec2Client, credentialsMap, securityGroupMap, imageExtension, namingConvention); this.ec2Client = ec2Client; this.placementGroupMap = placementGroupMap; this.placementGroupDeleted = placementGroupDeleted; diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/functions/AWSRunningInstanceToNodeMetadata.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/functions/AWSRunningInstanceToNodeMetadata.java index 66ba9e35fc..6bba366dc7 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/functions/AWSRunningInstanceToNodeMetadata.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/functions/AWSRunningInstanceToNodeMetadata.java @@ -35,6 +35,7 @@ import org.jclouds.compute.domain.HardwareBuilder; import org.jclouds.compute.domain.Image; import org.jclouds.compute.domain.NodeMetadataBuilder; import org.jclouds.compute.domain.NodeState; +import org.jclouds.compute.functions.GroupNamingConvention; import org.jclouds.domain.Credentials; import org.jclouds.domain.Location; import org.jclouds.domain.LoginCredentials; @@ -55,8 +56,9 @@ public class AWSRunningInstanceToNodeMetadata extends RunningInstanceToNodeMetad @Inject protected AWSRunningInstanceToNodeMetadata(Map instanceToNodeState, Map credentialStore, Supplier> imageMap, - @Memoized Supplier> locations, @Memoized Supplier> hardware) { - super(instanceToNodeState, credentialStore, imageMap, locations, hardware); + @Memoized Supplier> locations, @Memoized Supplier> hardware, + GroupNamingConvention.Factory namingConvention) { + super(instanceToNodeState, credentialStore, imageMap, locations, hardware, namingConvention); } @Override diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions.java index 4872ffdfbc..bbfec00caa 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions.java @@ -34,6 +34,7 @@ import org.jclouds.aws.ec2.domain.RegionNameAndPublicKeyMaterial; import org.jclouds.aws.ec2.functions.CreatePlacementGroupIfNeeded; import org.jclouds.aws.ec2.options.AWSRunInstancesOptions; import org.jclouds.compute.domain.Template; +import org.jclouds.compute.functions.GroupNamingConvention; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.reference.ComputeServiceConstants; import org.jclouds.ec2.compute.domain.RegionAndName; @@ -72,8 +73,9 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions Provider optionsProvider, @Named("PLACEMENT") LoadingCache placementGroupMap, CreatePlacementGroupIfNeeded createPlacementGroupIfNeeded, - Function importExistingKeyPair) { - super(makeKeyPair, credentialsMap, securityGroupMap, optionsProvider); + Function importExistingKeyPair, + GroupNamingConvention.Factory namingConvention) { + super(makeKeyPair, credentialsMap, securityGroupMap, optionsProvider, namingConvention); this.placementGroupMap = placementGroupMap; this.createPlacementGroupIfNeeded = createPlacementGroupIfNeeded; this.importExistingKeyPair = importExistingKeyPair; diff --git a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeServiceLiveTest.java b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeServiceLiveTest.java index 7ef63ee4aa..6d3163dd73 100644 --- a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeServiceLiveTest.java +++ b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeServiceLiveTest.java @@ -180,12 +180,11 @@ public class AWSEC2ComputeServiceLiveTest extends EC2ComputeServiceLiveTest { } // make sure we made our dummy group and also let in the user's group - assertEquals(newTreeSet(instance.getGroupIds()), ImmutableSortedSet. of("jclouds#" + group + "#" - + instance.getRegion(), group)); + assertEquals(newTreeSet(instance.getGroupIds()), ImmutableSortedSet. of("jclouds#" + group, group)); // make sure our dummy group has no rules SecurityGroup secgroup = getOnlyElement(securityGroupClient.describeSecurityGroupsInRegion(instance - .getRegion(), "jclouds#" + group + "#" + instance.getRegion())); + .getRegion(), "jclouds#" + group)); assert secgroup.getIpPermissions().size() == 0 : secgroup; @@ -258,7 +257,7 @@ public class AWSEC2ComputeServiceLiveTest extends EC2ComputeServiceLiveTest { // Assert the two instances are in the same groups region = instance1.getRegion(); - String expectedSecurityGroupName = "jclouds#" + group + "#" + region; + String expectedSecurityGroupName = "jclouds#" + group; assertEquals(instance1.getRegion(), region); assertNotNull(instance1.getKeyName()); diff --git a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/functions/AWSRunningInstanceToNodeMetadataTest.java b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/functions/AWSRunningInstanceToNodeMetadataTest.java index 00a0c7e2fc..1f7e7d58fd 100644 --- a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/functions/AWSRunningInstanceToNodeMetadataTest.java +++ b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/functions/AWSRunningInstanceToNodeMetadataTest.java @@ -23,12 +23,14 @@ import static org.testng.Assert.assertEquals; import java.util.Map; import java.util.Set; +import org.jclouds.aws.ec2.AWSEC2ApiMetadata; import org.jclouds.aws.ec2.domain.AWSRunningInstance; import org.jclouds.aws.ec2.domain.MonitoringState; import org.jclouds.compute.domain.Hardware; import org.jclouds.compute.domain.Image; import org.jclouds.compute.domain.NodeMetadataBuilder; import org.jclouds.compute.domain.NodeState; +import org.jclouds.compute.functions.GroupNamingConvention; import org.jclouds.date.DateService; import org.jclouds.domain.Credentials; import org.jclouds.domain.Location; @@ -53,7 +55,9 @@ import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.inject.AbstractModule; import com.google.inject.Guice; +import com.google.inject.name.Names; /** * @author Adrian Cole @@ -86,7 +90,7 @@ public class AWSRunningInstanceToNodeMetadataTest { .instanceState(InstanceState.RUNNING) .privateDnsName("ip-10-212-81-7.ec2.internal") .dnsName("ec2-174-129-173-155.compute-1.amazonaws.com") - .keyName("jclouds#zkclustertest#us-east-1#23") + .keyName("jclouds#zkclustertest#23") .amiLaunchIndex("0") .instanceType("t1.micro") .launchTime(dateService.iso8601DateParse("2011-08-16T13:40:50.000Z")) @@ -95,7 +99,7 @@ public class AWSRunningInstanceToNodeMetadataTest { .monitoringState(MonitoringState.DISABLED) .privateIpAddress("10.212.81.7") .ipAddress("174.129.173.155") - .securityGroupIdToNames(ImmutableMap. of("sg-ef052b86", "jclouds#zkclustertest#us-east-1")) + .securityGroupIdToNames(ImmutableMap. of("sg-ef052b86", "jclouds#zkclustertest")) .rootDeviceType(RootDeviceType.EBS) .rootDeviceName("/dev/sda1") .device("/dev/sda1", new BlockDevice("vol-5829fc32", Attachment.Status.ATTACHED, dateService.iso8601DateParse("2011-08-16T13:41:19.000Z"), true)) @@ -111,7 +115,7 @@ public class AWSRunningInstanceToNodeMetadataTest { .instanceState(InstanceState.RUNNING) .privateDnsName("ip-10-212-185-8.ec2.internal") .dnsName("ec2-50-19-207-248.compute-1.amazonaws.com") - .keyName("jclouds#zkclustertest#us-east-1#23") + .keyName("jclouds#zkclustertest#23") .amiLaunchIndex("0") .instanceType("t1.micro") .launchTime(dateService.iso8601DateParse("2011-08-16T13:40:50.000Z")) @@ -120,7 +124,7 @@ public class AWSRunningInstanceToNodeMetadataTest { .monitoringState(MonitoringState.DISABLED) .privateIpAddress("10.212.185.8") .ipAddress("50.19.207.248") - .securityGroupIdToNames(ImmutableMap.of("sg-ef052b86", "jclouds#zkclustertest#us-east-1")) + .securityGroupIdToNames(ImmutableMap.of("sg-ef052b86", "jclouds#zkclustertest")) .rootDeviceType(RootDeviceType.EBS) .rootDeviceName("/dev/sda1") .device("/dev/sda1", new BlockDevice("vol-5029fc3a", Attachment.Status.ATTACHED, dateService.iso8601DateParse("2011-08-16T13:41:19.000Z"), true)) @@ -195,9 +199,19 @@ public class AWSRunningInstanceToNodeMetadataTest { } }; + + GroupNamingConvention.Factory namingConvention = Guice.createInjector(new AbstractModule() { + + @Override + protected void configure() { + Names.bindProperties(binder(),new AWSEC2ApiMetadata().getDefaultProperties()); + } + + }).getInstance(GroupNamingConvention.Factory.class); + AWSRunningInstanceToNodeMetadata parser = new AWSRunningInstanceToNodeMetadata(instanceToNodeState, credentialStore, Suppliers.> ofInstance(instanceToImage), - locationSupplier, hardwareSupplier); + locationSupplier, hardwareSupplier, namingConvention); return parser; } diff --git a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java index 59f1727cc4..5efc2db5cf 100644 --- a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java +++ b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java @@ -41,6 +41,7 @@ import org.jclouds.aws.ec2.functions.CreatePlacementGroupIfNeeded; import org.jclouds.aws.ec2.options.AWSRunInstancesOptions; import org.jclouds.compute.domain.Hardware; import org.jclouds.compute.domain.Template; +import org.jclouds.compute.functions.GroupNamingConvention; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.domain.LoginCredentials; import org.jclouds.ec2.compute.EC2TemplateBuilderTest; @@ -659,7 +660,7 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT // setup constants String region = Region.AP_SOUTHEAST_1; String group = "group"; - String generatedMarkerGroup = "jclouds#group#" + Region.AP_SOUTHEAST_1; + String generatedMarkerGroup = "jclouds#group"; Set groupNames = ImmutableSet. of(); int[] ports = new int[] {}; boolean shouldAuthorizeSelf = true; @@ -693,7 +694,7 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT // setup constants String region = Region.AP_SOUTHEAST_1; String group = "group"; - String generatedMarkerGroup = "jclouds#group#" + Region.AP_SOUTHEAST_1; + String generatedMarkerGroup = "jclouds#group"; Set groupNames = ImmutableSet. of(); int[] ports = new int[] { 22, 80 }; boolean shouldAuthorizeSelf = true; @@ -727,7 +728,7 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT // setup constants String region = Region.AP_SOUTHEAST_1; String group = "group"; - String generatedMarkerGroup = "jclouds#group#" + Region.AP_SOUTHEAST_1; + String generatedMarkerGroup = "jclouds#group"; Set groupNames = ImmutableSet. of(); int[] ports = new int[] {}; boolean shouldAuthorizeSelf = true; @@ -761,7 +762,7 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT // setup constants String region = Region.AP_SOUTHEAST_1; String group = "group"; - String generatedMarkerGroup = "jclouds#group#" + Region.AP_SOUTHEAST_1; + String generatedMarkerGroup = "jclouds#group"; Set groupNames = ImmutableSet. of("group1", "group2"); int[] ports = new int[] {}; boolean shouldAuthorizeSelf = true; @@ -797,7 +798,7 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT // setup constants String region = Region.AP_SOUTHEAST_1; String group = "group"; - String generatedMarkerGroup = "jclouds#group#" + Region.AP_SOUTHEAST_1; + String generatedMarkerGroup = "jclouds#group"; Set groupNames = ImmutableSet. of(); int[] ports = new int[] {}; boolean shouldAuthorizeSelf = true; @@ -938,9 +939,16 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT LoadingCache placementGroupMap = createMock(LoadingCache.class); Function importExistingKeyPair = createMock(Function.class); CreatePlacementGroupIfNeeded createPlacementGroupIfNeeded = createMock(CreatePlacementGroupIfNeeded.class); + GroupNamingConvention.Factory namingConventionFactory = createMock(GroupNamingConvention.Factory.class); + GroupNamingConvention namingConvention = createMock(GroupNamingConvention.class); + expect(namingConventionFactory.create()).andReturn(namingConvention).anyTimes(); + expect(namingConvention.sharedNameForGroup("group")).andReturn("jclouds#group").anyTimes(); + replay(namingConventionFactory); + replay(namingConvention); return new CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions(makeKeyPair, credentialsMap, - securityGroupMap, OPTIONS_PROVIDER, placementGroupMap, createPlacementGroupIfNeeded, importExistingKeyPair); + securityGroupMap, OPTIONS_PROVIDER, placementGroupMap, createPlacementGroupIfNeeded, importExistingKeyPair, + namingConventionFactory); } private void replayStrategy(CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions strategy) {