Merge "Making security group refresh more specific"

10 files changed, 196 insertions, 19 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 078b17106..4c02dcd5a 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -2164,20 +2164,16 @@ class SecurityGroupAPI(base.Base):
 
         security_group = self.db.security_group_get(context, id)
 
-        hosts = set()
         for instance in security_group['instances']:
             if instance['host'] is not None:
-                hosts.add(instance['host'])
-
-        for host in hosts:
-            self.security_group_rpcapi.refresh_security_group_rules(context,
-                    security_group.id, host=host)
+                self.security_group_rpcapi.refresh_instance_security_rules(
+                        context, instance['host'], instance)
 
     def trigger_members_refresh(self, context, group_ids):
         """Called when a security group gains a new or loses a member.
 
-        Sends an update request to each compute node for whom this is
-        relevant.
+        Sends an update request to each compute node for each instance for
+        which this is relevant.
         """
         # First, we get the security group rules that reference these groups as
         # the grantee..
@@ -2188,7 +2184,7 @@ class SecurityGroupAPI(base.Base):
                                                                      context,
                                                                      group_id))
 
-        # ..then we distill the security groups to which they belong..
+        # ..then we distill the rules into the groups to which they belong..
         security_groups = set()
         for rule in security_group_rules:
             security_group = self.db.security_group_get(
@@ -2202,17 +2198,11 @@ class SecurityGroupAPI(base.Base):
             for instance in security_group['instances']:
                 instances.add(instance)
 
-        # ...then we find the hosts where they live...
-        hosts = set()
+        # ..then we send a request to refresh the rules for each instance.
         for instance in instances:
             if instance['host']:
-                hosts.add(instance['host'])
-
-        # ...and finally we tell these nodes to refresh their view of this
-        # particular security group.
-        for host in hosts:
-            self.security_group_rpcapi.refresh_security_group_members(context,
-                    group_id, host=host)
+                self.security_group_rpcapi.refresh_instance_security_rules(
+                        context, instance['host'], instance)
 
     def parse_cidr(self, cidr):
         if cidr:
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 9971be012..eb1e4cda7 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -221,7 +221,7 @@ def _get_image_meta(context, image_ref):
 class ComputeManager(manager.SchedulerDependentManager):
     """Manages the running instances from creation to destruction."""
 
-    RPC_API_VERSION = '1.40'
+    RPC_API_VERSION = '1.41'
 
     def __init__(self, compute_driver=None, *args, **kwargs):
         """Load configuration options and connect to the hypervisor."""
@@ -356,6 +356,16 @@ class ComputeManager(manager.SchedulerDependentManager):
         return self.driver.refresh_security_group_members(security_group_id)
 
     @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
+    def refresh_instance_security_rules(self, context, instance):
+        """Tell the virtualization driver to refresh security rules for
+        an instance.
+
+        Passes straight through to the virtualization driver.
+
+        """
+        return self.driver.refresh_instance_security_rules(instance)
+
+    @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
     def refresh_provider_fw_rules(self, context, **kwargs):
         """This call passes straight through to the virtualization driver."""
         return self.driver.refresh_provider_fw_rules(**kwargs)
diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py
index 9b98f2ef5..9cee4f28d 100644
--- a/nova/compute/rpcapi.py
+++ b/nova/compute/rpcapi.py
@@ -119,6 +119,7 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy):
                 - remove topic, it was unused
         1.39 - Remove instance_uuid, add instance argument to run_instance()
         1.40 - Remove instance_id, add instance argument to live_migration()
+        1.41 - Adds refresh_instance_security_rules()
     '''
 
     BASE_RPC_API_VERSION = '1.0'
@@ -521,6 +522,7 @@ class SecurityGroupAPI(nova.openstack.common.rpc.proxy.RpcProxy):
     API version history:
 
         1.0 - Initial version.
+        1.41 - Adds refresh_instance_security_rules()
     '''
 
     BASE_RPC_API_VERSION = '1.0'
@@ -540,3 +542,11 @@ class SecurityGroupAPI(nova.openstack.common.rpc.proxy.RpcProxy):
         self.cast(ctxt, self.make_msg('refresh_security_group_members',
                 security_group_id=security_group_id),
                 topic=_compute_topic(self.topic, ctxt, host, None))
+
+    def refresh_instance_security_rules(self, ctxt, host, instance):
+        instance_p = jsonutils.to_primitive(instance)
+        self.cast(ctxt, self.make_msg('refresh_instance_security_rules',
+                instance=instance_p),
+                topic=_compute_topic(self.topic, ctxt, instance['host'],
+                instance),
+                version='1.41')
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index 08242c2f5..da3f8c1e1 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -52,6 +52,7 @@ import nova.policy
 from nova import quota
 from nova.scheduler import driver as scheduler_driver
 from nova import test
+from nova.tests.db.fakes import FakeModel
 from nova.tests import fake_network
 from nova.tests.image import fake as fake_image
 from nova import utils
@@ -3918,6 +3919,134 @@ class ComputeAPITestCase(BaseTestCase):
                                      "/tmp/test", "File Contents")
         db.instance_destroy(self.context, instance['uuid'])
 
+    def test_secgroup_refresh(self):
+        instance = self._create_fake_instance()
+
+        def rule_get(*args, **kwargs):
+            mock_rule = FakeModel({'parent_group_id': 1})
+            return [mock_rule]
+
+        def group_get(*args, **kwargs):
+            mock_group = FakeModel({'instances': [instance]})
+            return mock_group
+
+        self.stubs.Set(
+                   self.compute_api.db,
+                   'security_group_rule_get_by_security_group_grantee',
+                   rule_get)
+        self.stubs.Set(self.compute_api.db, 'security_group_get', group_get)
+
+        self.mox.StubOutWithMock(rpc, 'cast')
+        topic = rpc.queue_get_for(self.context, FLAGS.compute_topic,
+                                  instance['host'])
+        rpc.cast(self.context, topic,
+                {"method": "refresh_instance_security_rules",
+                 "args": {'instance': jsonutils.to_primitive(instance)},
+                 "version": '1.41'})
+        self.mox.ReplayAll()
+
+        self.security_group_api.trigger_members_refresh(self.context, [1])
+
+    def test_secgroup_refresh_once(self):
+        instance = self._create_fake_instance()
+
+        def rule_get(*args, **kwargs):
+            mock_rule = FakeModel({'parent_group_id': 1})
+            return [mock_rule]
+
+        def group_get(*args, **kwargs):
+            mock_group = FakeModel({'instances': [instance]})
+            return mock_group
+
+        self.stubs.Set(
+                   self.compute_api.db,
+                   'security_group_rule_get_by_security_group_grantee',
+                   rule_get)
+        self.stubs.Set(self.compute_api.db, 'security_group_get', group_get)
+
+        self.mox.StubOutWithMock(rpc, 'cast')
+        topic = rpc.queue_get_for(self.context, FLAGS.compute_topic,
+                                  instance['host'])
+        rpc.cast(self.context, topic,
+                {"method": "refresh_instance_security_rules",
+                 "args": {'instance': jsonutils.to_primitive(instance)},
+                 "version": '1.41'})
+        self.mox.ReplayAll()
+
+        self.security_group_api.trigger_members_refresh(self.context, [1, 2])
+
+    def test_secgroup_refresh_none(self):
+        def rule_get(*args, **kwargs):
+            mock_rule = FakeModel({'parent_group_id': 1})
+            return [mock_rule]
+
+        def group_get(*args, **kwargs):
+            mock_group = FakeModel({'instances': []})
+            return mock_group
+
+        self.stubs.Set(
+                   self.compute_api.db,
+                   'security_group_rule_get_by_security_group_grantee',
+                   rule_get)
+        self.stubs.Set(self.compute_api.db, 'security_group_get', group_get)
+
+        self.mox.StubOutWithMock(rpc, 'cast')
+        self.mox.ReplayAll()
+
+        self.security_group_api.trigger_members_refresh(self.context, [1])
+
+    def test_secrule_refresh(self):
+        instance = self._create_fake_instance()
+
+        def group_get(*args, **kwargs):
+            mock_group = FakeModel({'instances': [instance]})
+            return mock_group
+
+        self.stubs.Set(self.compute_api.db, 'security_group_get', group_get)
+
+        self.mox.StubOutWithMock(rpc, 'cast')
+        topic = rpc.queue_get_for(self.context, FLAGS.compute_topic,
+                                  instance['host'])
+        rpc.cast(self.context, topic,
+                {"method": "refresh_instance_security_rules",
+                 "args": {'instance': jsonutils.to_primitive(instance)},
+                 "version": '1.41'})
+        self.mox.ReplayAll()
+
+        self.security_group_api.trigger_rules_refresh(self.context, [1])
+
+    def test_secrule_refresh_once(self):
+        instance = self._create_fake_instance()
+
+        def group_get(*args, **kwargs):
+            mock_group = FakeModel({'instances': [instance]})
+            return mock_group
+
+        self.stubs.Set(self.compute_api.db, 'security_group_get', group_get)
+
+        self.mox.StubOutWithMock(rpc, 'cast')
+        topic = rpc.queue_get_for(self.context, FLAGS.compute_topic,
+                                  instance['host'])
+        rpc.cast(self.context, topic,
+                {"method": "refresh_instance_security_rules",
+                 "args": {'instance': jsonutils.to_primitive(instance)},
+                 "version": '1.41'})
+        self.mox.ReplayAll()
+
+        self.security_group_api.trigger_rules_refresh(self.context, [1, 2])
+
+    def test_secrule_refresh_none(self):
+        def group_get(*args, **kwargs):
+            mock_group = FakeModel({'instances': []})
+            return mock_group
+
+        self.stubs.Set(self.compute_api.db, 'security_group_get', group_get)
+
+        self.mox.StubOutWithMock(rpc, 'cast')
+        self.mox.ReplayAll()
+
+        self.security_group_api.trigger_rules_refresh(self.context, [1, 2])
+
 
 def fake_rpc_method(context, topic, msg, do_cast=True):
     pass
diff --git a/nova/virt/baremetal/driver.py b/nova/virt/baremetal/driver.py
index 5bbb23995..3da59e929 100644
--- a/nova/virt/baremetal/driver.py
+++ b/nova/virt/baremetal/driver.py
@@ -655,6 +655,10 @@ class BareMetalDriver(driver.ComputeDriver):
         # Bare metal doesn't currently support security groups
         pass
 
+    def refresh_instance_security_rules(self, instance):
+        # Bare metal doesn't currently support security groups
+        pass
+
     def update_available_resource(self, ctxt, host):
         """Updates compute manager resource info on ComputeNode table.
 
diff --git a/nova/virt/fake.py b/nova/virt/fake.py
index f87e78af4..178bfb066 100644
--- a/nova/virt/fake.py
+++ b/nova/virt/fake.py
@@ -213,6 +213,9 @@ class FakeDriver(driver.ComputeDriver):
     def refresh_security_group_members(self, security_group_id):
         return True
 
+    def refresh_instance_security_rules(self, instance):
+        return True
+
     def refresh_provider_fw_rules(self):
         pass
 
diff --git a/nova/virt/firewall.py b/nova/virt/firewall.py
index 2afb8b6cf..20f23906f 100644
--- a/nova/virt/firewall.py
+++ b/nova/virt/firewall.py
@@ -75,6 +75,14 @@ class FirewallDriver(object):
         the security group."""
         raise NotImplementedError()
 
+    def refresh_instance_security_rules(self, instance):
+        """Refresh security group rules from data store
+
+        Gets called when an instance gets added to or removed from
+        the security group the instance is a member of or if the
+        group gains or looses a rule."""
+        raise NotImplementedError()
+
     def refresh_provider_fw_rules(self):
         """Refresh common rules for all hosts/instances from data store.
 
@@ -391,12 +399,21 @@ class IptablesFirewallDriver(FirewallDriver):
         self.do_refresh_security_group_rules(security_group)
         self.iptables.apply()
 
+    def refresh_instance_security_rules(self, instance):
+        self.do_refresh_instance_rules(instance)
+        self.iptables.apply()
+
     @utils.synchronized('iptables', external=True)
     def do_refresh_security_group_rules(self, security_group):
         for instance in self.instances.values():
             self.remove_filters_for_instance(instance)
             self.add_filters_for_instance(instance)
 
+    @utils.synchronized('iptables', external=True)
+    def do_refresh_instance_rules(self, instance):
+        self.remove_filters_for_instance(instance)
+        self.add_filters_for_instance(instance)
+
     def refresh_provider_fw_rules(self):
         """See :class:`FirewallDriver` docs."""
         self._do_refresh_provider_fw_rules()
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 57459b0c0..ba04dd70d 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -2173,6 +2173,9 @@ class LibvirtDriver(driver.ComputeDriver):
     def refresh_security_group_members(self, security_group_id):
         self.firewall_driver.refresh_security_group_members(security_group_id)
 
+    def refresh_instance_security_rules(self, instance):
+        self.firewall_driver.refresh_instance_security_rules(instance)
+
     def refresh_provider_fw_rules(self):
         self.firewall_driver.refresh_provider_fw_rules()
 
diff --git a/nova/virt/xenapi/driver.py b/nova/virt/xenapi/driver.py
index ac4547166..8891e9106 100644
--- a/nova/virt/xenapi/driver.py
+++ b/nova/virt/xenapi/driver.py
@@ -516,6 +516,13 @@ class XenAPIDriver(driver.ComputeDriver):
         """
         return self._vmops.refresh_security_group_members(security_group_id)
 
+    def refresh_instance_security_rules(self, instance):
+        """ Updates security group rules for specified instance
+            Invoked when instances are added/removed to a security group
+            or when a rule is added/removed to a security group
+        """
+        return self._vmops.refresh_instance_security_rules(instance)
+
     def refresh_provider_fw_rules(self):
         return self._vmops.refresh_provider_fw_rules()
 
diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py
index 39c0f994e..9f00dc75e 100644
--- a/nova/virt/xenapi/vmops.py
+++ b/nova/virt/xenapi/vmops.py
@@ -1456,6 +1456,10 @@ class VMOps(object):
         """ recreates security group rules for every instance """
         self.firewall_driver.refresh_security_group_members(security_group_id)
 
+    def refresh_instance_security_rules(self, instance):
+        """ recreates security group rules for specified instance """
+        self.firewall_driver.refresh_instance_security_rules(instance)
+
     def refresh_provider_fw_rules(self):
         self.firewall_driver.refresh_provider_fw_rules()