brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (BROOKLYN-259) jcloudsByon location spec leaks location instances
Date Mon, 09 May 2016 14:20:12 GMT

    [ 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)

Mime
View raw message