Mailing-List: contact dev-help@brooklyn.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@brooklyn.incubator.apache.org
From: mikezaccardo <git@git.apache.org>
To: dev@brooklyn.incubator.apache.org
Reply-To: dev@brooklyn.incubator.apache.org
References: <git-pr-694-incubator-brooklyn@git.apache.org>
In-Reply-To: <git-pr-694-incubator-brooklyn@git.apache.org>
Subject: [GitHub] incubator-brooklyn pull request: [BROOKLYN-99] add support
 for Nov...
Content-Type: text/plain
Message-Id: <20150624182040.DCBDAE05D1@git1-us-west.apache.org>
Date: Wed, 24 Jun 2015 18:20:40 +0000 (UTC)

Github user mikezaccardo commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/694#discussion_r33178718
  
    --- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java ---
    @@ -225,41 +238,47 @@ void addPermissionsToLocation(Iterable<IpPermission> permissions, final String n
          * {@link #sharedGroupCache} if no mapping for the shared group's location previously
          * existed (e.g. Brooklyn was restarted and rebound to an existing application).
          *
    +     * Notice that jclouds will attach 2 securityGroups to the node if the locationId is `aws-ec2` so it needs to
    +     * look for the uniqueSecurityGroup rather than the shared securityGroup.
    +     *
          * @param nodeId The id of the node in question
    +     * @param locationId The id of the location in question
          * @param securityApi The API to use to list security groups
          * @return the security group unique to the given node, or null if one could not be determined.
          */
    -    private SecurityGroup getUniqueSecurityGroupForNodeCachingSharedGroupIfPreviouslyUnknown(
    -            String nodeId, SecurityGroupExtension securityApi) {
    +    private SecurityGroup getUniqueSecurityGroupForNodeCachingSharedGroupIfPreviouslyUnknown(String nodeId, String locationId, SecurityGroupExtension securityApi) {
             Set<SecurityGroup> groupsOnNode = securityApi.listSecurityGroupsForNode(nodeId);
    -        if (groupsOnNode.size() != 2) {
    -            LOG.warn("Expected to find two security groups on node {} in app {} (one shared, one unique). Found {}: {}",
    -                    new Object[]{nodeId, applicationId, groupsOnNode.size(), groupsOnNode});
    -            return null;
    -        }
    -        String expectedSharedName = getNameForSharedSecurityGroup();
    -        Iterator<SecurityGroup> it = groupsOnNode.iterator();
    -        SecurityGroup shared = it.next();
             SecurityGroup unique;
    -        if (shared.getName().endsWith(expectedSharedName)) {
    -            unique = it.next();
    -        } else {
    -            unique = shared;
    -            shared = it.next();
    -        }
    -        if (!shared.getName().endsWith(expectedSharedName)) {
    -            LOG.warn("Couldn't determine which security group is shared between instances in app {}. Expected={}, found={}",
    -                    new Object[]{applicationId, expectedSharedName, groupsOnNode});
    -            return null;
    -        }
    -        // Shared entry might be missing if Brooklyn has rebound to an application
    -        SecurityGroup old = sharedGroupCache.asMap().putIfAbsent(shared.getLocation(), shared);
    -        LOG.info("Loaded unique security group for node {} (in {}): {}",
    -                new Object[]{nodeId, applicationId, unique});
    -        if (old == null) {
    -            LOG.info("Proactively set shared group for app {} to: {}", applicationId, shared);
    +        if (locationId.equals("aws-ec2")) {
    --- End diff --
    
    It may be nice to extract this AWS-specific functionality into a new method.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---