make nwfilter mock more 'realistic' by having it remember which

filters have been defined

2 files changed, 55 insertions, 18 deletions

diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py
index 4bc5fed16..5e85e3a2f 100644
--- a/nova/tests/test_virt.py
+++ b/nova/tests/test_virt.py
@@ -659,16 +659,26 @@ class LibvirtConnTestCase(test.TestCase):
 
 class FakeNWFilter:
     def __init__(self):
-        self.undefine_call_count = 0
+        self.filters = {}
 
-    def undefine(self):
-        self.undefine_call_count += 1
-        pass
-
-    def _nwfilterLookupByName(self, ignore):
-        return self
+    def _nwfilterLookupByName(self, name):
+        if name in self.filters:
+            return self.filters[name]
+        raise libvirt.libvirtError('Filter Not Found')
 
     def _filterDefineXMLMock(self, xml):
+        class FakeNWFilterInternal:
+            def __init__(self, parent, name):
+                self.name = name
+                self.parent = parent
+
+            def undefine(self):
+                del self.parent.filters[self.name]
+                pass
+        tree = xml_to_tree(xml)
+        name = tree.get('name')
+        if name not in self.filters:
+            self.filters[name] = FakeNWFilterInternal(self, name)
         return True
 
 
@@ -689,6 +699,20 @@ class IptablesFirewallTestCase(test.TestCase):
         self.fw = libvirt_conn.IptablesFirewallDriver(
                       get_connection=lambda: self.fake_libvirt_connection)
 
+    def lazy_load_library_exists(self):
+        """check if libvirt is available."""
+        # try to connect libvirt. if fail, skip test.
+        try:
+            import libvirt
+            import libxml2
+        except ImportError:
+            return False
+        global libvirt
+        libvirt = __import__('libvirt')
+        libvirt_conn.libvirt = __import__('libvirt')
+        libvirt_conn.libxml2 = __import__('libxml2')
+        return True
+
     def tearDown(self):
         self.manager.delete_project(self.project)
         self.manager.delete_user(self.user)
@@ -895,6 +919,10 @@ class IptablesFirewallTestCase(test.TestCase):
         self.fw.do_refresh_security_group_rules("fake")
 
     def test_unfilter_instance_undefines_nwfilter(self):
+        # Skip if non-libvirt environment
+        if not self.lazy_load_library_exists():
+            return
+
         admin_ctxt = context.get_admin_context()
 
         fakefilter = FakeNWFilter()
@@ -916,10 +944,11 @@ class IptablesFirewallTestCase(test.TestCase):
         self.fw.setup_basic_filtering(instance)
         self.fw.prepare_instance_filter(instance)
         self.fw.apply_instance_filter(instance)
+        original_filter_count = len(fakefilter.filters)
         self.fw.unfilter_instance(instance)
 
         # should attempt to undefine just the instance filter
-        self.assertEquals(fakefilter.undefine_call_count, 1)
+        self.assertEqual(original_filter_count - len(fakefilter.filters), 1)
 
         db.instance_destroy(admin_ctxt, instance_ref['id'])
 
@@ -1109,6 +1138,12 @@ class NWFilterTestCase(test.TestCase):
 
         instance_ref = self._create_instance()
         inst_id = instance_ref['id']
+
+        self.security_group = self.setup_and_return_security_group()
+
+        db.instance_add_security_group(self.context, inst_id,
+                                       self.security_group.id)
+
         instance = db.instance_get(self.context, inst_id)
 
         ip = '10.11.12.13'
@@ -1120,9 +1155,12 @@ class NWFilterTestCase(test.TestCase):
         self.fw.setup_basic_filtering(instance)
         self.fw.prepare_instance_filter(instance)
         self.fw.apply_instance_filter(instance)
+        original_filter_count = len(fakefilter.filters)
+        print fakefilter.filters.keys()
         self.fw.unfilter_instance(instance)
 
+        print fakefilter.filters.keys()
         # should attempt to undefine 2 filters: instance and instance-secgroup
-        self.assertEquals(fakefilter.undefine_call_count, 2)
+        self.assertEqual(original_filter_count - len(fakefilter.filters), 2)
 
         db.instance_destroy(admin_ctxt, instance_ref['id'])
diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index 9241c1d9e..f27398aa3 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -1838,7 +1838,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, remove_secgroup=True):
+    def unfilter_instance(self, instance):
         """Clear out the nwfilter rules."""
         network_info = _get_network_info(instance)
         instance_name = instance.name
@@ -1856,13 +1856,12 @@ class NWFilterFirewall(FirewallDriver):
         instance_secgroup_filter_name =\
             '%s-secgroup' % (self._instance_filter_name(instance))
 
-        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())
+        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):
         """
@@ -2028,7 +2027,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, False)
+            self.nwfilter.unfilter_instance(instance)
         else:
             LOG.info(_('Attempted to unfilter instance %s which is not '
                      'filtered'), instance['id'])