[
https://issues.apache.org/jira/browse/BROOKLYN-259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15276416#comment-15276416
]
ASF GitHub Bot commented on BROOKLYN-259:
-----------------------------------------
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/132#discussion_r62505309
--- 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()) {
--- End diff --
Check `!isPresentAndNonNull` instead?
> 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)
