brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mikezaccardo <...@git.apache.org>
Subject [GitHub] incubator-brooklyn pull request: [BROOKLYN-99] add support for Nov...
Date Wed, 24 Jun 2015 18:20:40 GMT
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.
---

Mime
View raw message