Repository: jclouds Updated Branches: refs/heads/master 5113be22d -> 1d4cb6c39
[JCLOUDS-1306] Fix SG cache invalidation when deleting Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/1d4cb6c3 Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/1d4cb6c3 Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/1d4cb6c3 Branch: refs/heads/master Commit: 1d4cb6c39294bf8e455d90d0d4a6cae651d58d28 Parents: 5113be2 Author: Svetoslav Neykov <[email protected]> Authored: Fri Jun 9 10:55:20 2017 +0300 Committer: Andrea Turli <[email protected]> Committed: Fri Jun 9 12:04:29 2017 +0200 ---------------------------------------------------------------------- .../extensions/NovaSecurityGroupExtension.java | 18 ++++-- .../BaseSecurityGroupExtensionLiveTest.java | 66 ++++++++++++++++++++ 2 files changed, 78 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/jclouds/blob/1d4cb6c3/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtension.java ---------------------------------------------------------------------- diff --git a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtension.java b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtension.java index 88ab63c..4f992b9 100644 --- a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtension.java +++ b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtension.java @@ -191,14 +191,20 @@ public class NovaSecurityGroupExtension implements SecurityGroupExtension { return false; } - if (sgApi.get().get(groupId) == null) { - return false; + // Would be nice to delete the group and invalidate the cache atomically - i.e. use a mutex. + // Will make sure that a create operation in parallel won't see inconsistent state. + + boolean deleted = sgApi.get().delete(groupId); + + for (SecurityGroupInRegion cachedSg : groupCreator.asMap().values()) { + if (groupId.equals(cachedSg.getSecurityGroup().getId())) { + String groupName = cachedSg.getName(); + groupCreator.invalidate(new RegionSecurityGroupNameAndPorts(region, groupName, ImmutableSet.<Integer>of())); + break; + } } - sgApi.get().delete(groupId); - // TODO: test this clear happens - groupCreator.invalidate(new RegionSecurityGroupNameAndPorts(region, groupId, ImmutableSet.<Integer> of())); - return true; + return deleted; } @Override http://git-wip-us.apache.org/repos/asf/jclouds/blob/1d4cb6c3/compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java ---------------------------------------------------------------------- diff --git a/compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java b/compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java index 0804799..2aa7e66 100644 --- a/compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java +++ b/compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java @@ -27,6 +27,7 @@ import javax.annotation.Resource; import javax.inject.Named; import org.jclouds.compute.ComputeService; +import org.jclouds.compute.ComputeServiceContext; import org.jclouds.compute.RunNodesException; import org.jclouds.compute.domain.SecurityGroup; import org.jclouds.compute.domain.Template; @@ -60,6 +61,7 @@ public abstract class BaseSecurityGroupExtensionLiveTest extends BaseComputeServ protected Logger logger = Logger.NULL; protected final String secGroupName = "test-create-security-group"; + protected final String secGroupNameToDelete = "test-create-security-group-to-delete"; protected final String nodeGroup = "test-create-node-with-group"; protected String groupId; @@ -379,6 +381,70 @@ public abstract class BaseSecurityGroupExtensionLiveTest extends BaseComputeServ assertTrue(securityGroupExtension.get().removeSecurityGroup(group.getId())); } + @Test(groups = {"integration", "live"}, singleThreaded = true) + public void testSecurityGroupCacheInvalidated() throws Exception { + ComputeService computeService = view.getComputeService(); + Optional<SecurityGroupExtension> securityGroupExtension = computeService.getSecurityGroupExtension(); + assertTrue(securityGroupExtension.isPresent(), "security extension was not present"); + final SecurityGroupExtension security = securityGroupExtension.get(); + final SecurityGroup seedGroup = security.createSecurityGroup(secGroupNameToDelete, getNodeTemplate().getLocation()); + boolean deleted = security.removeSecurityGroup(seedGroup.getId()); + assertTrue(deleted, "just created security group failed deletion"); + final SecurityGroup recreatedGroup = security.createSecurityGroup(secGroupNameToDelete, getNodeTemplate().getLocation()); + + // Makes sure the security group exists and is re-created and is not just returned from cache + security.addIpPermission(IpPermission.builder() + .fromPort(1000) + .toPort(1000) + .cidrBlock("1.1.1.1/32") + .ipProtocol(IpProtocol.TCP) + .build(), + recreatedGroup); + boolean deleted2 = security.removeSecurityGroup(recreatedGroup.getId()); + assertTrue(deleted2, "just created security group failed deletion"); + } + + @Test(groups = {"integration", "live"}, singleThreaded = true) + public void testSecurityGroupCacheInvalidatedWhenDeletedExternally() throws Exception { + ComputeService computeService = view.getComputeService(); + Optional<SecurityGroupExtension> securityGroupExtension = computeService.getSecurityGroupExtension(); + assertTrue(securityGroupExtension.isPresent(), "security extension was not present"); + final SecurityGroupExtension security = securityGroupExtension.get(); + final SecurityGroup seedGroup = security.createSecurityGroup(secGroupNameToDelete, getNodeTemplate().getLocation()); + + deleteSecurityGroupFromAnotherView(seedGroup); + + boolean deleted = security.removeSecurityGroup(seedGroup.getId()); + assertFalse(deleted, "SG deleted externally so should've failed deletion"); + final SecurityGroup recreatedGroup = security.createSecurityGroup(secGroupNameToDelete, getNodeTemplate().getLocation()); + + // Makes sure the security group exists and is re-created and is not just returned from cache + security.addIpPermission(IpPermission.builder() + .fromPort(1000) + .toPort(1000) + .cidrBlock("1.1.1.1/32") + .ipProtocol(IpProtocol.TCP) + .build(), + recreatedGroup); + boolean deleted2 = security.removeSecurityGroup(recreatedGroup.getId()); + assertTrue(deleted2, "just created security group failed deletion"); + } + + private void deleteSecurityGroupFromAnotherView(SecurityGroup seedGroup) { + ComputeServiceContext localView = createView(setupProperties(), setupModules()); + try { + ComputeService localComputeService = localView.getComputeService(); + Optional<SecurityGroupExtension> securityGroupExtension = localComputeService.getSecurityGroupExtension(); + assertTrue(securityGroupExtension.isPresent(), "security extension was not present"); + final SecurityGroupExtension security = securityGroupExtension.get(); + + boolean deleted = security.removeSecurityGroup(seedGroup.getId()); + assertTrue(deleted, "just created security group failed deletion"); + } finally { + localView.close(); + } + } + private Multimap<String, String> emptyMultimap() { return LinkedHashMultimap.create(); }
