From cf6309f35db6c22721b9a3f4a659756be2e072cc Mon Sep 17 00:00:00 2001 From: Anthony Young Date: Mon, 16 May 2011 15:59:01 -0700 Subject: [PATCH 1/4] fix for lp783705 - remove nwfilters when instance is terminated --- nova/tests/test_virt.py | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py index 1311ba36..babb5de9 100644 --- a/nova/tests/test_virt.py +++ b/nova/tests/test_virt.py @@ -1045,3 +1045,45 @@ class NWFilterTestCase(test.TestCase): network_info, "fake") 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 + + instance_ref = self._create_instance() + 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 2 filters: instance and instance-secgroup + self.assertEquals(fakefilter.undefine_call_count, 2) + + db.instance_destroy(admin_ctxt, instance_ref['id']) From 9235e2ecf599483b063acbcf3d76b28087ea7581 Mon Sep 17 00:00:00 2001 From: Anthony Young Date: Thu, 19 May 2011 13:50:11 -0700 Subject: [PATCH 2/4] Separate out tests for when unfilter is called from iptables vs. nwfilter driver. Re: lp783705 --- nova/tests/test_virt.py | 65 +++++++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py index babb5de9..3b5a3867 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'] From b685a461ca2b3ad86db3110bdf8509a5f9558ce4 Mon Sep 17 00:00:00 2001 From: Anthony Young Date: Fri, 20 May 2011 08:59:07 -0700 Subject: [PATCH 3/4] make nwfilter mock more 'realistic' by having it remember which filters have been defined --- nova/tests/test_virt.py | 56 ++++++++++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py index 4bc5fed1..5e85e3a2 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']) From 7865074fc54d9daf085cee1917bd214c46fafc0a Mon Sep 17 00:00:00 2001 From: Anthony Young Date: Fri, 20 May 2011 09:09:03 -0700 Subject: [PATCH 4/4] fix comments --- nova/tests/test_virt.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py index 5e85e3a2..90c6de5a 100644 --- a/nova/tests/test_virt.py +++ b/nova/tests/test_virt.py @@ -659,6 +659,7 @@ class LibvirtConnTestCase(test.TestCase): class FakeNWFilter: def __init__(self): + self.filters = {} def _nwfilterLookupByName(self, name): @@ -947,7 +948,7 @@ class IptablesFirewallTestCase(test.TestCase): original_filter_count = len(fakefilter.filters) self.fw.unfilter_instance(instance) - # should attempt to undefine just the instance filter + # should undefine just the instance filter self.assertEqual(original_filter_count - len(fakefilter.filters), 1) db.instance_destroy(admin_ctxt, instance_ref['id']) @@ -1160,7 +1161,7 @@ class NWFilterTestCase(test.TestCase): self.fw.unfilter_instance(instance) print fakefilter.filters.keys() - # should attempt to undefine 2 filters: instance and instance-secgroup + # should undefine 2 filters: instance and instance-secgroup self.assertEqual(original_filter_count - len(fakefilter.filters), 2) db.instance_destroy(admin_ctxt, instance_ref['id'])