You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jclouds.apache.org by an...@apache.org on 2017/06/09 10:05:01 UTC
jclouds git commit: [JCLOUDS-1306] Fix SG cache invalidation when
deleting
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 <sv...@cloudsoftcorp.com>
Authored: Fri Jun 9 10:55:20 2017 +0300
Committer: Andrea Turli <an...@gmail.com>
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();
}