You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by sj...@apache.org on 2015/09/16 12:52:16 UTC

[4/9] incubator-brooklyn git commit: Add synchronization and checks for duplicate rules in addPermission security group customizer

Add synchronization and checks for duplicate rules in addPermission security group customizer


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/4096358f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/4096358f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/4096358f

Branch: refs/heads/master
Commit: 4096358f5fcc3058f8185d2587086d93fb023c9a
Parents: 7ca2781
Author: Andrew Kennedy <gr...@apache.org>
Authored: Wed Sep 16 04:11:06 2015 +0100
Committer: Andrew Kennedy <gr...@apache.org>
Committed: Wed Sep 16 04:12:09 2015 +0100

----------------------------------------------------------------------
 .../JcloudsLocationSecurityGroupCustomizer.java | 27 +++++++++++++++-----
 1 file changed, 21 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4096358f/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
index cd4e45f..60a5271 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
@@ -33,6 +33,7 @@ import org.apache.brooklyn.location.jclouds.JcloudsLocation;
 import org.apache.brooklyn.location.jclouds.JcloudsLocationCustomizer;
 import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
 import org.apache.brooklyn.location.jclouds.JcloudsSshMachineLocation;
+
 import org.jclouds.aws.AWSResponseException;
 import org.jclouds.compute.ComputeService;
 import org.jclouds.compute.domain.SecurityGroup;
@@ -43,9 +44,12 @@ import org.jclouds.net.domain.IpPermission;
 import org.jclouds.net.domain.IpProtocol;
 import org.jclouds.providers.ProviderMetadata;
 import org.jclouds.providers.Providers;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
 import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.net.Cidr;
@@ -179,14 +183,23 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation
      * Applies the given security group permissions to the given location.
      * <p>
      * Takes no action if the location's compute service does not have a security group extension.
-     * @param permissions The set of permissions to be applied to the location
+     * <p>
+     * The {@code synchronized} block is to serialize the permission changes, preventing race
+     * conditions in some clouds. If multiple customizations of the same group are done in parallel
+     * the changes may not be picked up by later customizations, meaning the same rule could possibly be
+     * added twice, which would fail. A finer grained mechanism would be preferable here, but
+     * we have no access to the information required, so this brute force serializing is required.
+     *
      * @param location Location to gain permissions
+     * @param permissions The set of permissions to be applied to the location
      */
     public JcloudsLocationSecurityGroupCustomizer addPermissionsToLocation(final JcloudsMachineLocation location, final Iterable<IpPermission> permissions) {
-        ComputeService computeService = location.getParent().getComputeService();
-        String nodeId = location.getNode().getId();
-        addPermissionsToLocation(permissions, nodeId, computeService);
-        return this;
+        synchronized (JcloudsLocationSecurityGroupCustomizer.class) {
+            ComputeService computeService = location.getParent().getComputeService();
+            String nodeId = location.getNode().getId();
+            addPermissionsToLocation(permissions, nodeId, computeService);
+            return this;
+        }
     }
 
     /**
@@ -228,7 +241,9 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation
         } finally {
             Tasks.resetBlockingDetails();
         }
-        for (IpPermission permission : permissions) {
+        MutableList<IpPermission> newPermissions = MutableList.copyOf(permissions);
+        Iterables.removeAll(newPermissions, machineUniqueSecurityGroup.getIpPermissions());
+        for (IpPermission permission : newPermissions) {
             addPermission(permission, machineUniqueSecurityGroup, securityApi);
         }
     }