Separate out tests for when unfilter is called from iptables vs. nwfilter

driver.  Re: lp783705

2 files changed, 58 insertions, 29 deletions

diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py
index babb5de9b..3b5a3867d 100644
--- a/nova/tests/test_virt.py
+++ b/nova/tests/test_virt.py
@@ -657,6 +657,21 @@ class LibvirtConnTestCase(test.TestCase):
         super(LibvirtConnTestCase, self).tearDown()
 
 
+class FakeNWFilter:
+    def __init__(self):
+        self.undefine_call_count = 0
+
+    def undefine(self):
+        self.undefine_call_count += 1
+        pass
+
+    def _nwfilterLookupByName(self, ignore):
+        return self
+
+    def _filterDefineXMLMock(self, xml):
+        return True
+
+
 class IptablesFirewallTestCase(test.TestCase):
     def setUp(self):
         super(IptablesFirewallTestCase, self).setUp()
@@ -869,6 +884,35 @@ class IptablesFirewallTestCase(test.TestCase):
         self.assertEquals(ipv6_network_rules,
                           ipv6_rules_per_network * networks_count)
 
+    def test_unfilter_instance_undefines_nwfilters(self):
+        admin_ctxt = context.get_admin_context()
+
+        fakefilter = FakeNWFilter()
+        self.fw.nwfilter._conn.nwfilterDefineXML =\
+                               fakefilter._filterDefineXMLMock
+        self.fw.nwfilter._conn.nwfilterLookupByName =\
+                               fakefilter._nwfilterLookupByName
+
+        instance_ref = self._create_instance_ref()
+        inst_id = instance_ref['id']
+        instance = db.instance_get(self.context, inst_id)
+
+        ip = '10.11.12.13'
+        network_ref = db.project_get_network(self.context, 'fake')
+        fixed_ip = {'address': ip, 'network_id': network_ref['id']}
+        db.fixed_ip_create(admin_ctxt, fixed_ip)
+        db.fixed_ip_update(admin_ctxt, ip, {'allocated': True,
+                                            'instance_id': inst_id})
+        self.fw.setup_basic_filtering(instance)
+        self.fw.prepare_instance_filter(instance)
+        self.fw.apply_instance_filter(instance)
+        self.fw.unfilter_instance(instance)
+
+        # should attempt to undefine just the instance filter
+        self.assertEquals(fakefilter.undefine_call_count, 1)
+
+        db.instance_destroy(admin_ctxt, instance_ref['id'])
+
 
 class NWFilterTestCase(test.TestCase):
     def setUp(self):
@@ -1047,26 +1091,11 @@ class NWFilterTestCase(test.TestCase):
         self.assertEquals(len(result), 3)
 
     def test_unfilter_instance_undefines_nwfilters(self):
-        class FakeNWFilter:
-            def __init__(self):
-                self.undefine_call_count = 0
-
-            def undefine(self):
-                self.undefine_call_count += 1
-                pass
-
-        fakefilter = FakeNWFilter()
-
-        def _nwfilterLookupByName(ignore):
-            return fakefilter
-
-        def _filterDefineXMLMock(xml):
-            return True
-
         admin_ctxt = context.get_admin_context()
 
-        self.fw._conn.nwfilterDefineXML = _filterDefineXMLMock
-        self.fw._conn.nwfilterLookupByName = _nwfilterLookupByName
+        fakefilter = FakeNWFilter()
+        self.fw._conn.nwfilterDefineXML = fakefilter._filterDefineXMLMock
+        self.fw._conn.nwfilterLookupByName = fakefilter._nwfilterLookupByName
 
         instance_ref = self._create_instance()
         inst_id = instance_ref['id']
diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index 706973176..f808a4b7b 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -1834,7 +1834,7 @@ class NWFilterFirewall(FirewallDriver):
         # execute in a native thread and block current greenthread until done
         tpool.execute(self._conn.nwfilterDefineXML, xml)
 
-    def unfilter_instance(self, instance):
+    def unfilter_instance(self, instance, remove_secgroup=True):
         """Clear out the nwfilter rules."""
         network_info = _get_network_info(instance)
         instance_name = instance.name
@@ -1846,19 +1846,19 @@ class NWFilterFirewall(FirewallDriver):
                 self._conn.nwfilterLookupByName(instance_filter_name).\
                                                     undefine()
             except libvirt.libvirtError:
-                LOG.debug(_('The nwfilter(%(instance_filter_name)s) for '
-                            '%(instance_name)s is not found.') % locals())
+                LOG.debug(_('The nwfilter(%(instance_filter_name)s) '
+                            'for %(instance_name)s is not found.') % locals())
 
         instance_secgroup_filter_name =\
             '%s-secgroup' % (self._instance_filter_name(instance))
 
-        try:
-            self._conn.nwfilterLookupByName(instance_secgroup_filter_name).\
-                                                undefine()
-        except libvirt.libvirtError:
-            # This will happen if called by IptablesFirewallDriver
-            LOG.debug(_('The nwfilter(%(instance_secgroup_filter_name)s) for '
-                            '%(instance_name)s is not found.') % locals())
+        if remove_secgroup:
+            try:
+                self._conn.nwfilterLookupByName(instance_secgroup_filter_name)\
+                                                .undefine()
+            except libvirt.libvirtError:
+                LOG.debug(_('The nwfilter(%(instance_secgroup_filter_name)s) '
+                            'for %(instance_name)s is not found.') % locals())
 
     def prepare_instance_filter(self, instance, network_info=None):
         """
@@ -2022,7 +2022,7 @@ class IptablesFirewallDriver(FirewallDriver):
         if self.instances.pop(instance['id'], None):
             self.remove_filters_for_instance(instance)
             self.iptables.apply()
-            self.nwfilter.unfilter_instance(instance)
+            self.nwfilter.unfilter_instance(instance, False)
         else:
             LOG.info(_('Attempted to unfilter instance %s which is not '
                      'filtered'), instance['id'])