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 <[email protected]> Authored: Wed Sep 16 04:11:06 2015 +0100 Committer: Andrew Kennedy <[email protected]> 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); } }
