[
https://issues.apache.org/jira/browse/BROOKLYN-259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15276549#comment-15276549
]
ASF GitHub Bot commented on BROOKLYN-259:
-----------------------------------------
Github user johnmccabe commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/132#discussion_r62524649
--- Diff:
locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolver.java
---
@@ -61,129 +56,140 @@
* @author aled
*/
@SuppressWarnings({"unchecked","rawtypes"})
-public class JcloudsByonLocationResolver implements LocationResolver {
+public class JcloudsByonLocationResolver extends AbstractLocationResolver
implements LocationResolver {
public static final Logger log =
LoggerFactory.getLogger(JcloudsByonLocationResolver.class);
public static final String BYON = "jcloudsByon";
- private static final Pattern PATTERN =
Pattern.compile("("+BYON+"|"+BYON.toUpperCase()+")" + ":" + "\\((.*)\\)$");
-
- private ManagementContext managementContext;
-
@Override
- public void init(ManagementContext managementContext) {
- this.managementContext = checkNotNull(managementContext,
"managementContext");
+ public boolean isEnabled() {
+ return
LocationConfigUtils.isResolverPrefixEnabled(managementContext, getPrefix());
}
@Override
- public boolean isEnabled() {
- return
LocationConfigUtils.isResolverPrefixEnabled(managementContext, getPrefix());
+ public String getPrefix() {
+ return BYON;
}
- // TODO Remove some duplication from JcloudsResolver; needs more
careful review
@Override
- public LocationSpec<? extends Location>
newLocationSpecFromString(String spec, Map<?, ?> locationFlags,
LocationRegistry registry) {
- Map globalProperties = registry.getProperties();
+ protected Class<? extends Location> getLocationType() {
+ return FixedListMachineProvisioningLocation.class;
+ }
- Matcher matcher = PATTERN.matcher(spec);
- if (!matcher.matches()) {
- throw new IllegalArgumentException("Invalid location
'"+spec+"'; must specify something like
jcloudsByon(provider=\"aws-ec2\",region=\"us-east-1\",hosts=\"i-f2014593,i-d1234567\")");
- }
-
- String argsPart = matcher.group(2);
- Map<String, String> argsMap = KeyValueParser.parseMap(argsPart);
-
- // prefer args map over location flags
-
- String namedLocation = (String)
locationFlags.get(LocationInternal.NAMED_SPEC_NAME.getName());
+ @Override
+ protected SpecParser getSpecParser() {
+ return new
AbstractLocationResolver.SpecParser(getPrefix()).setExampleUsage("\"jcloudsByon(provider='aws-ec2',region='us-east-1',hosts='i-12345678,i-90123456')\"");
+ }
+
+ @Override
+ protected ConfigBag extractConfig(Map<?,?> locationFlags, String spec,
final LocationRegistry registry) {
+ ConfigBag config = super.extractConfig(locationFlags, spec,
registry);
- String providerOrApi = argsMap.containsKey("provider") ?
argsMap.get("provider") : (String)locationFlags.get("provider");
+ String providerOrApi = (String) config.getStringKey("provider");
+ String regionName = (String) config.getStringKey("region");
+ String endpoint = (String) config.getStringKey("endpoint");
+ String namedLocation = (String)
config.get(LocationInternal.NAMED_SPEC_NAME);
+ config.remove(LocationInternal.NAMED_SPEC_NAME.getName());
- String regionName = argsMap.containsKey("region") ?
argsMap.get("region") : (String)locationFlags.get("region");
-
- String endpoint = argsMap.containsKey("endpoint") ?
argsMap.get("endpoint") : (String)locationFlags.get("endpoint");
-
- String name = argsMap.containsKey("name") ? argsMap.get("name") :
(String)locationFlags.get("name");
+ Object hosts = config.getStringKey("hosts");
+ config.remove("hosts");
- String user = argsMap.containsKey("user") ? argsMap.get("user") :
(String)locationFlags.get("user");
+ Map jcloudsProperties = new
JcloudsPropertiesFromBrooklynProperties().getJcloudsProperties(providerOrApi,
regionName, namedLocation, config.getAllConfig());
+ config.putIfAbsent(jcloudsProperties);
- String privateKeyFile = argsMap.containsKey("privateKeyFile") ?
argsMap.get("privateKeyFile") : (String)locationFlags.get("privateKeyFile");
-
- String hosts = argsMap.get("hosts");
-
if (Strings.isEmpty(providerOrApi)) {
throw new IllegalArgumentException("Invalid location
'"+spec+"'; provider must be defined");
}
- if (hosts == null || hosts.isEmpty()) {
+ if (hosts == null || (hosts instanceof String &&
Strings.isBlank((String)hosts))) {
throw new IllegalArgumentException("Invalid location
'"+spec+"'; at least one host must be defined");
}
- if (argsMap.containsKey("name") && (Strings.isEmpty(name))) {
- throw new IllegalArgumentException("Invalid location
'"+spec+"'; if name supplied then value must be non-empty");
+
+ final String jcloudsSpec = "jclouds:"+providerOrApi + (regionName
!= null ? ":"+regionName : "") + (endpoint != null ? ":"+endpoint : "");
+ final Maybe<LocationSpec<? extends Location>> jcloudsLocationSpec
= registry.getLocationSpec(jcloudsSpec, config.getAllConfig());
+ if (jcloudsLocationSpec.isAbsent()) {
+ throw new IllegalArgumentException("Invalid location
'"+spec+"'; referrenced jclouds spec '"+jcloudsSpec+"' cannot be resolved");
+ } else if
(!JcloudsLocation.class.isAssignableFrom(jcloudsLocationSpec.get().getType())) {
+ throw new IllegalArgumentException("Invalid location
'"+spec+"'; referrenced spec '"+jcloudsSpec+"' of type
"+jcloudsLocationSpec.get().getType()+" rather than JcloudsLocation");
}
- // For everything in brooklyn.properties, only use things with
correct prefix (and remove that prefix).
- // But for everything passed in via locationFlags, pass those
as-is.
- // TODO Should revisit the locationFlags: where are these actually
used? Reason accepting properties without
- // full prefix is that the map's context is explicitly this
location, rather than being generic properties.
- Map allProperties = getAllProperties(registry, globalProperties);
- Map jcloudsProperties = new
JcloudsPropertiesFromBrooklynProperties().getJcloudsProperties(providerOrApi,
regionName, namedLocation, allProperties);
- jcloudsProperties.putAll(locationFlags);
- jcloudsProperties.putAll(argsMap);
+ List<?> hostIds;
- String jcloudsSpec = "jclouds:"+providerOrApi + (regionName !=
null ? ":"+regionName : "") + (endpoint != null ? ":"+endpoint : "");
- JcloudsLocation jcloudsLocation = (JcloudsLocation)
registry.resolve(jcloudsSpec, jcloudsProperties);
+ if (hosts instanceof String) {
+ if (((String) hosts).isEmpty()) {
+ hostIds = ImmutableList.of();
+ } else {
+ hostIds =
WildcardGlobs.getGlobsAfterBraceExpansion("{"+hosts+"}",
+ true /* numeric */, /* no quote support though */
PhraseTreatment.NOT_A_SPECIAL_CHAR, PhraseTreatment.NOT_A_SPECIAL_CHAR);
+ }
+ } else if (hosts instanceof Iterable) {
+ hostIds = ImmutableList.copyOf((Iterable<?>)hosts);
+ } else {
+ throw new IllegalArgumentException("Invalid location
'"+spec+"'; at least one host must be defined");
+ }
+ if (hostIds.isEmpty()) {
+ throw new IllegalArgumentException("Invalid location
'"+spec+"'; at least one host must be defined");
+ }
- List<String> hostIdentifiers =
WildcardGlobs.getGlobsAfterBraceExpansion("{"+hosts+"}",
- true /* numeric */, /* no quote support though */
PhraseTreatment.NOT_A_SPECIAL_CHAR, PhraseTreatment.NOT_A_SPECIAL_CHAR);
- List<JcloudsSshMachineLocation> machines = Lists.newArrayList();
-
- for (String hostIdentifier : hostIdentifiers) {
- Map<?, ?> machineFlags = MutableMap.builder()
- .put("id", hostIdentifier)
- .putIfNotNull("user", user)
- .putIfNotNull("privateKeyFile", privateKeyFile)
- .build();
- try {
- // TODO management of these machines may be odd, as it is
passed in as a key as config to a spec
- JcloudsSshMachineLocation machine =
jcloudsLocation.rebindMachine(jcloudsLocation.config().getBag().putAll(machineFlags));
- machines.add(machine);
- } catch (NoMachinesAvailableException e) {
- log.warn("Error rebinding to jclouds machine
"+hostIdentifier+" in "+jcloudsLocation, e);
- Exceptions.propagate(e);
+ final List<Map<?,?>> machinesFlags = Lists.newArrayList();
+ for (Object hostId : hostIds) {
+ Map<?, ?> machineFlags;
+ if (hostId instanceof String) {
+ machineFlags = parseMachineFlags((String)hostId, config);
+ } else if (hostId instanceof Map) {
+ machineFlags = parseMachineFlags((Map<String,?>)hostId,
config);
+ } else {
+ throw new IllegalArgumentException("Invalid host type
'"+(hostId == null ? null : hostId.getClass().getName())+", referrenced in spec
'"+spec);
}
+ machinesFlags.add(machineFlags);
}
+ Supplier<List<JcloudsMachineLocation>> machinesFactory = new
Supplier<List<JcloudsMachineLocation>>() {
+ @Override
+ public List<JcloudsMachineLocation> get() {
+ // FIXME Can't change the parent of the
JcloudsMachineLocation; but the FixedListMachineLocation will expect it to be
the parent?
+ // We'll get errors on rebind. Need to
reproduce/investigate.
+ List<JcloudsMachineLocation> result = Lists.newArrayList();
+ JcloudsLocation jcloudsLocation = (JcloudsLocation)
managementContext.getLocationManager().createLocation(jcloudsLocationSpec.get());
+ for (Map<?,?> machineFlags : machinesFlags) {
+ try {
+ JcloudsMachineLocation machine =
jcloudsLocation.registerMachine(jcloudsLocation.config().getBag().putAll(machineFlags));
+ result.add(machine);
+ } catch (NoMachinesAvailableException e) {
+ Map<?,?> sanitizedMachineFlags =
Sanitizer.sanitize(machineFlags);
+ log.warn("Error rebinding to jclouds machine
"+sanitizedMachineFlags+" in "+jcloudsLocation, e);
+ Exceptions.propagate(e);
+ }
+ }
+
+ log.debug("Created machines for jclouds BYON location:
"+result);
+ return result;
+ }
+ };
+
ConfigBag flags = ConfigBag.newInstance(jcloudsProperties);
--- End diff --
Are we doing anything with this?
> jcloudsByon location spec leaks location instances
> --------------------------------------------------
>
> Key: BROOKLYN-259
> URL: https://issues.apache.org/jira/browse/BROOKLYN-259
> Project: Brooklyn
> Issue Type: Bug
> Affects Versions: 0.9.0
> Reporter: Aled Sage
> Fix For: 0.10.0
>
>
> When declaring in brooklyn.properties a location spec such as
> {noformat}
> jcloudsByon(provider="aws-ec2",region="us-east-1",user="brooklyn",password="pa55w0rd",hosts="i-12345678")
> {noformat}
> It caused new locations to be created and persisted every few seconds, but
> for those locations to be never deleted.
> ---
> This was caused by changes made in 0.9.0 to {{LocationResolver}}
> implementations, so that it creates a {{LocationSpec}} rather than
> instantiating a location directly. Unfortunately,
> {{JcloudsByonLocationResolver}} had not been updated. The REST api would poll
> this regularly to find out about the types of location, and on every poll it
> would create new locations.
> This is similar to the problem encountered for localhost, fixed in time for
> 0.9.0 in https://github.com/apache/brooklyn-server/pull/97.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
