From a8d186d212ffbc628fc2b2672eca1e0557c57414 Mon Sep 17 00:00:00 2001 From: Eldar Nugaev Date: Sun, 3 Apr 2011 03:45:33 +0400 Subject: [PATCH 1/7] split up to_xml to creation xml_info and filling the template --- nova/tests/test_virt.py | 20 ++++++-------------- nova/virt/libvirt_conn.py | 12 +++++++----- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py index 958c8e3e2299..62afcd1f1edc 100644 --- a/nova/tests/test_virt.py +++ b/nova/tests/test_virt.py @@ -31,9 +31,7 @@ from nova import test from nova import utils from nova.api.ec2 import cloud from nova.auth import manager -from nova.compute import manager as compute_manager from nova.compute import power_state -from nova.db.sqlalchemy import models from nova.virt import libvirt_conn libvirt = None @@ -269,7 +267,7 @@ class LibvirtConnTestCase(test.TestCase): self.assertTrue(len(target) > 0) def _check_xml_and_uri(self, instance, expect_ramdisk, expect_kernel, - rescue=False): + rescue=False, network_info=None): user_context = context.RequestContext(project=self.project, user=self.user) instance_ref = db.instance_create(user_context, instance) @@ -327,19 +325,13 @@ class LibvirtConnTestCase(test.TestCase): check = (lambda t: t.find('./os/initrd'), None) check_list.append(check) + parameter = './devices/interface/filterref/parameter' common_checks = [ (lambda t: t.find('.').tag, 'domain'), - (lambda t: t.find( - './devices/interface/filterref/parameter').get('name'), 'IP'), - (lambda t: t.find( - './devices/interface/filterref/parameter').get( - 'value'), '10.11.12.13'), - (lambda t: t.findall( - './devices/interface/filterref/parameter')[1].get( - 'name'), 'DHCPSERVER'), - (lambda t: t.findall( - './devices/interface/filterref/parameter')[1].get( - 'value'), '10.0.0.1'), + (lambda t: t.find(parameter).get('name'), 'IP'), + (lambda t: t.find(parameter).get('value'), '10.11.12.13'), + (lambda t: t.findall(parameter)[1].get('name'), 'DHCPSERVER'), + (lambda t: t.findall(parameter)[1].get('value'), '10.0.0.1'), (lambda t: t.find('./devices/serial/source').get( 'path').split('/')[1], 'console.log'), (lambda t: t.find('./memory').text, '2097152')] diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index f34ea72255db..910d8a634861 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -936,7 +936,7 @@ class LibvirtConnection(driver.ComputeDriver): return result - def to_xml(self, instance, rescue=False, network_info=None): + def _prepare_xml_info(self, instance, rescue=False, network_info=None): # TODO(termie): cache? LOG.debug(_('instance %s: starting toXML method'), instance['name']) @@ -947,8 +947,7 @@ class LibvirtConnection(driver.ComputeDriver): nics = [] for (network, mapping) in network_info: - nics.append(self._get_nic_for_xml(network, - mapping)) + nics.append(self._get_nic_for_xml(network, mapping)) # FIXME(vish): stick this in db instance_type_name = instance['instance_type'] instance_type = instance_types.get_instance_type(instance_type_name) @@ -979,10 +978,13 @@ class LibvirtConnection(driver.ComputeDriver): xml_info['ramdisk'] = xml_info['basepath'] + "/ramdisk" xml_info['disk'] = xml_info['basepath'] + "/disk" + + return xml_info + def to_xml(self, instance, rescue=False, network_info=None): + xml_info = self._prepare_xml_info(instance, rescue, network_info) xml = str(Template(self.libvirt_xml, searchList=[xml_info])) - LOG.debug(_('instance %s: finished toXML method'), - instance['name']) + LOG.debug(_('instance %s: finished toXML method'), instance['name']) return xml def get_info(self, instance_name): From 74d9a325a452fb927e5edddca3f1b7edd35d1496 Mon Sep 17 00:00:00 2001 From: Eldar Nugaev Date: Sun, 3 Apr 2011 21:18:35 +0400 Subject: [PATCH 2/7] added preparing_xml test --- nova/tests/test_virt.py | 41 +++++++++++++++++++++++++++++++++++++++ nova/virt/libvirt_conn.py | 24 +++++++---------------- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py index 62afcd1f1edc..319544099d50 100644 --- a/nova/tests/test_virt.py +++ b/nova/tests/test_virt.py @@ -192,6 +192,47 @@ class LibvirtConnTestCase(test.TestCase): return db.service_create(context.get_admin_context(), service_ref) + + def _create_network_info(self, count=1): + fake = 'fake' + fake_ip = '0.0.0.0/0' + network = {'gateway': fake, + 'gateway_v6': fake, + 'bridge': fake, + 'cidr': fake_ip, + 'cidr_v6': fake_ip} + mapping = {'mac': fake, + 'ips': [{'ip': fake_ip}]} + + return [(network, mapping) for x in xrange(0, count)] + + def test_preparing_xml_info(self): + conn = libvirt_conn.LibvirtConnection(True) + instance_ref = db.instance_create(self.context, self.test_instance) + + result = conn._prepare_xml_info(instance_ref, False) + self.assertFalse(result['nics']) + + result = conn._prepare_xml_info(instance_ref, False, + self._create_network_info()) + self.assertTrue(len(result['nics']) == 1) + + result = conn._prepare_xml_info(instance_ref, False, + self._create_network_info(2)) + self.assertTrue(len(result['nics']) == 2) + + def test_get_nic_for_xml(self): + conn = libvirt_conn.LibvirtConnection(True) + network, mapping = self._create_network_info()[0] + FLAGS.use_ipv6 = False + params_1 = conn._get_nic_for_xml(network, mapping)['extra_params'] + FLAGS.use_ipv6 = True + params_2 = conn._get_nic_for_xml(network, mapping)['extra_params'] + self.assertTrue(params_1.find('PROJNETV6') == -1) + self.assertTrue(params_1.find('PROJMASKV6') == -1) + self.assertTrue(params_2.find('PROJNETV6') > -1) + self.assertTrue(params_2.find('PROJMASKV6') > -1) + def test_xml_and_uri_no_ramdisk_no_kernel(self): instance_data = dict(self.test_instance) self._check_xml_and_uri(instance_data, diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 910d8a634861..8af5eb025af0 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -899,26 +899,16 @@ class LibvirtConnection(driver.ComputeDriver): mac_id = mapping['mac'].replace(':', '') if FLAGS.allow_project_net_traffic: + template = "\n" + net, mask = _get_net_and_mask(network['cidr']) + values = [("PROJNET", net), ("PROJMASK", mask)] if FLAGS.use_ipv6: - net, mask = _get_net_and_mask(network['cidr']) net_v6, prefixlen_v6 = _get_net_and_prefixlen( network['cidr_v6']) - extra_params = ("\n" - "\n" - "\n" - "\n") % \ - (net, mask, net_v6, prefixlen_v6) - else: - net, mask = _get_net_and_mask(network['cidr']) - extra_params = ("\n" - "\n") % \ - (net, mask) + values.extend([("PROJNETV6", net_v6), + ("PROJMASKV6", prefixlen_v6)]) + + extra_params = "".join([template % value for value in values]) else: extra_params = "\n" From 8969cb1f22a7760dc7e17c578a686f088b1a8d89 Mon Sep 17 00:00:00 2001 From: Eldar Nugaev Date: Sun, 3 Apr 2011 22:50:38 +0400 Subject: [PATCH 3/7] add multi_nic_test --- nova/tests/test_virt.py | 14 ++++++++++++-- nova/virt/libvirt_conn.py | 3 +-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py index 319544099d50..b6482503e013 100644 --- a/nova/tests/test_virt.py +++ b/nova/tests/test_virt.py @@ -192,7 +192,6 @@ class LibvirtConnTestCase(test.TestCase): return db.service_create(context.get_admin_context(), service_ref) - def _create_network_info(self, count=1): fake = 'fake' fake_ip = '0.0.0.0/0' @@ -224,6 +223,7 @@ class LibvirtConnTestCase(test.TestCase): def test_get_nic_for_xml(self): conn = libvirt_conn.LibvirtConnection(True) network, mapping = self._create_network_info()[0] + backup = FLAGS.use_ipv6 FLAGS.use_ipv6 = False params_1 = conn._get_nic_for_xml(network, mapping)['extra_params'] FLAGS.use_ipv6 = True @@ -232,6 +232,7 @@ class LibvirtConnTestCase(test.TestCase): self.assertTrue(params_1.find('PROJMASKV6') == -1) self.assertTrue(params_2.find('PROJNETV6') > -1) self.assertTrue(params_2.find('PROJMASKV6') > -1) + FLAGS.use_ipv6 = backup def test_xml_and_uri_no_ramdisk_no_kernel(self): instance_data = dict(self.test_instance) @@ -268,6 +269,15 @@ class LibvirtConnTestCase(test.TestCase): instance_data = dict(self.test_instance) self._check_xml_and_container(instance_data) + def test_multi_nic(self): + instance_data = dict(self.test_instance) + network_info = self._create_network_info(2) + conn = libvirt_conn.LibvirtConnection(True) + instance_ref = db.instance_create(self.context, instance_data) + xml = conn.to_xml(instance_ref, False, network_info) + tree = xml_to_tree(xml) + self.assertEquals(len(tree.findall("./devices/interface")), 2) + def _check_xml_and_container(self, instance): user_context = context.RequestContext(project=self.project, user=self.user) @@ -308,7 +318,7 @@ class LibvirtConnTestCase(test.TestCase): self.assertTrue(len(target) > 0) def _check_xml_and_uri(self, instance, expect_ramdisk, expect_kernel, - rescue=False, network_info=None): + rescue=False): user_context = context.RequestContext(project=self.project, user=self.user) instance_ref = db.instance_create(user_context, instance) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 8af5eb025af0..5c7540927cd9 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -908,7 +908,7 @@ class LibvirtConnection(driver.ComputeDriver): values.extend([("PROJNETV6", net_v6), ("PROJMASKV6", prefixlen_v6)]) - extra_params = "".join([template % value for value in values]) + extra_params = "".join([template % value for value in values]) else: extra_params = "\n" @@ -968,7 +968,6 @@ class LibvirtConnection(driver.ComputeDriver): xml_info['ramdisk'] = xml_info['basepath'] + "/ramdisk" xml_info['disk'] = xml_info['basepath'] + "/disk" - return xml_info def to_xml(self, instance, rescue=False, network_info=None): From 80549a0085e7c3a90b117b4c9df5a77b4ecd0843 Mon Sep 17 00:00:00 2001 From: Eldar Nugaev Date: Mon, 4 Apr 2011 18:33:50 +0400 Subject: [PATCH 4/7] improving tests --- nova/tests/test_virt.py | 76 +++++++++++++++++++++++++++------------ nova/virt/libvirt_conn.py | 44 +++++++++++------------ 2 files changed, 75 insertions(+), 45 deletions(-) diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py index b6482503e013..ae813cb80211 100644 --- a/nova/tests/test_virt.py +++ b/nova/tests/test_virt.py @@ -44,6 +44,22 @@ def _concurrency(wait, done, target): done.send() +def _create_network_info(count=1): + fake = 'fake' + fake_ip = '0.0.0.0/0' + fake_ip_2 = '0.0.0.1/0' + fake_ip_3 = '0.0.0.1/0' + network = {'gateway': fake, + 'gateway_v6': fake, + 'bridge': fake, + 'cidr': fake_ip, + 'cidr_v6': fake_ip} + mapping = {'mac': fake, + 'ips': [{'ip': fake_ip}, {'ip': fake_ip}], + 'ip6s': [{'ip': fake_ip}, {'ip': fake_ip_2}, {'ip': fake_ip_3}]} + return [(network, mapping) for x in xrange(0, count)] + + class CacheConcurrencyTestCase(test.TestCase): def setUp(self): super(CacheConcurrencyTestCase, self).setUp() @@ -192,19 +208,6 @@ class LibvirtConnTestCase(test.TestCase): return db.service_create(context.get_admin_context(), service_ref) - def _create_network_info(self, count=1): - fake = 'fake' - fake_ip = '0.0.0.0/0' - network = {'gateway': fake, - 'gateway_v6': fake, - 'bridge': fake, - 'cidr': fake_ip, - 'cidr_v6': fake_ip} - mapping = {'mac': fake, - 'ips': [{'ip': fake_ip}]} - - return [(network, mapping) for x in xrange(0, count)] - def test_preparing_xml_info(self): conn = libvirt_conn.LibvirtConnection(True) instance_ref = db.instance_create(self.context, self.test_instance) @@ -213,16 +216,16 @@ class LibvirtConnTestCase(test.TestCase): self.assertFalse(result['nics']) result = conn._prepare_xml_info(instance_ref, False, - self._create_network_info()) + _create_network_info()) self.assertTrue(len(result['nics']) == 1) result = conn._prepare_xml_info(instance_ref, False, - self._create_network_info(2)) + _create_network_info(2)) self.assertTrue(len(result['nics']) == 2) def test_get_nic_for_xml(self): conn = libvirt_conn.LibvirtConnection(True) - network, mapping = self._create_network_info()[0] + network, mapping = _create_network_info()[0] backup = FLAGS.use_ipv6 FLAGS.use_ipv6 = False params_1 = conn._get_nic_for_xml(network, mapping)['extra_params'] @@ -271,12 +274,19 @@ class LibvirtConnTestCase(test.TestCase): def test_multi_nic(self): instance_data = dict(self.test_instance) - network_info = self._create_network_info(2) + network_info = _create_network_info(2) conn = libvirt_conn.LibvirtConnection(True) instance_ref = db.instance_create(self.context, instance_data) xml = conn.to_xml(instance_ref, False, network_info) tree = xml_to_tree(xml) - self.assertEquals(len(tree.findall("./devices/interface")), 2) + interfaces = tree.findall("./devices/interface") + self.assertEquals(len(interfaces), 2) + parameters = interfaces[0].findall('./filterref/parameter') + self.assertEquals(interfaces[0].get('type'), 'bridge') + self.assertEquals(parameters[0].get('name'), 'IP') + self.assertEquals(parameters[0].get('value'), '0.0.0.0/0') + self.assertEquals(parameters[1].get('name'), 'DHCPSERVER') + self.assertEquals(parameters[1].get('value'), 'fake') def _check_xml_and_container(self, instance): user_context = context.RequestContext(project=self.project, @@ -656,11 +666,14 @@ class IptablesFirewallTestCase(test.TestCase): '# Completed on Tue Jan 18 23:47:56 2011', ] + def _create_instance_ref(self): + return db.instance_create(self.context, + {'user_id': 'fake', + 'project_id': 'fake', + 'mac_address': '56:12:12:12:12:12'}) + def test_static_filters(self): - instance_ref = db.instance_create(self.context, - {'user_id': 'fake', - 'project_id': 'fake', - 'mac_address': '56:12:12:12:12:12'}) + instance_ref = self._create_instance_ref() ip = '10.11.12.13' network_ref = db.project_get_network(self.context, @@ -771,6 +784,25 @@ class IptablesFirewallTestCase(test.TestCase): "TCP port 80/81 acceptance rule wasn't added") db.instance_destroy(admin_ctxt, instance_ref['id']) + def test_filters_for_instance(self): + network_info = _create_network_info() + rulesv4, rulesv6 = self.fw._filters_for_instance("fake", network_info) + self.assertEquals(len(rulesv4), 2) + self.assertEquals(len(rulesv6), 3) + + def multinic_iptables_test(self): + instance_ref = self._create_instance_ref() + network_info = _create_network_info() + ipv4_len = len(self.fw.iptables.ipv4['filter'].rules) + ipv6_len = len(self.fw.iptables.ipv6['filter'].rules) + inst_ipv4, inst_ipv6 = self.fw.instance_rules(instance_ref, + network_info) + self.fw.add_filters_for_instance(instance_ref, network_info) + ipv4 = self.fw.iptables.ipv4['filter'].rules + ipv6 = self.fw.iptables.ipv6['filter'].rules + self.assertEquals(len(ipv4) - len(inst_ipv4) - ipv4_len, 2) + self.assertEquals(len(ipv6) - len(inst_ipv6) - ipv6_len, 3) + class NWFilterTestCase(test.TestCase): def setUp(self): diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 5c7540927cd9..92519da65578 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -1877,34 +1877,21 @@ class IptablesFirewallDriver(FirewallDriver): self.add_filters_for_instance(instance, network_info) self.iptables.apply() - def add_filters_for_instance(self, instance, network_info=None): - if not network_info: - network_info = _get_network_info(instance) - chain_name = self._instance_chain_name(instance) - - self.iptables.ipv4['filter'].add_chain(chain_name) + def _create_filter(self, ips, chain_name): + return ['-d %s -j $%s' % (ip, chain_name) for ip in ips] + def _filters_for_instance(self, chain_name, network_info): ips_v4 = [ip['ip'] for (_, mapping) in network_info - for ip in mapping['ips']] + for ip in mapping['ips']] + ipv4_rules = self._create_filter(ips_v4, chain_name) - for ipv4_address in ips_v4: - self.iptables.ipv4['filter'].add_rule('local', - '-d %s -j $%s' % - (ipv4_address, chain_name)) + ips_v6 = [ip['ip'] for (_, mapping) in network_info + for ip in mapping['ip6s']] - if FLAGS.use_ipv6: - self.iptables.ipv6['filter'].add_chain(chain_name) - ips_v6 = [ip['ip'] for (_, mapping) in network_info - for ip in mapping['ip6s']] - - for ipv6_address in ips_v6: - self.iptables.ipv6['filter'].add_rule('local', - '-d %s -j $%s' % - (ipv6_address, - chain_name)) - - ipv4_rules, ipv6_rules = self.instance_rules(instance, network_info) + ipv6_rules = self._create_filter(ips_v6, chain_name) + return ipv4_rules, ipv6_rules + def _add_filters(self, chain_name, ipv4_rules, ipv6_rules): for rule in ipv4_rules: self.iptables.ipv4['filter'].add_rule(chain_name, rule) @@ -1912,6 +1899,17 @@ class IptablesFirewallDriver(FirewallDriver): for rule in ipv6_rules: self.iptables.ipv6['filter'].add_rule(chain_name, rule) + def add_filters_for_instance(self, instance, network_info=None): + chain_name = self._instance_chain_name(instance) + if FLAGS.use_ipv6: + self.iptables.ipv6['filter'].add_chain(chain_name) + self.iptables.ipv4['filter'].add_chain(chain_name) + ipv4_rules, ipv6_rules = self._filters_for_instance(chain_name, + network_info) + self._add_filters('local', ipv4_rules, ipv6_rules) + ipv4_rules, ipv6_rules = self.instance_rules(instance, network_info) + self._add_filters(chain_name, ipv4_rules, ipv6_rules) + def remove_filters_for_instance(self, instance): chain_name = self._instance_chain_name(instance) From 917f7aafbfa0a797687d10a600a218517f9b75e0 Mon Sep 17 00:00:00 2001 From: Eldar Nugaev Date: Mon, 4 Apr 2011 22:22:27 +0400 Subject: [PATCH 5/7] add test for NWFilterFirewall --- nova/tests/test_virt.py | 19 ++++++++++--- nova/virt/libvirt_conn.py | 60 +++++++++++++++++++++++++-------------- 2 files changed, 53 insertions(+), 26 deletions(-) diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py index ae813cb80211..b3d701efe4ad 100644 --- a/nova/tests/test_virt.py +++ b/nova/tests/test_virt.py @@ -884,6 +884,12 @@ class NWFilterTestCase(test.TestCase): return db.security_group_get_by_name(self.context, 'fake', 'testgroup') + def _create_instance(self): + return db.instance_create(self.context, + {'user_id': 'fake', + 'project_id': 'fake', + 'mac_address': '00:A0:C9:14:C8:29'}) + def test_creates_base_rule_first(self): # These come pre-defined by libvirt self.defined_filters = ['no-mac-spoofing', @@ -912,10 +918,7 @@ class NWFilterTestCase(test.TestCase): self.fake_libvirt_connection.nwfilterDefineXML = _filterDefineXMLMock - instance_ref = db.instance_create(self.context, - {'user_id': 'fake', - 'project_id': 'fake', - 'mac_address': '00:A0:C9:14:C8:29'}) + instance_ref = self._create_instance() inst_id = instance_ref['id'] ip = '10.11.12.13' @@ -955,3 +958,11 @@ class NWFilterTestCase(test.TestCase): _ensure_all_called() self.teardown_security_group() db.instance_destroy(admin_ctxt, instance_ref['id']) + + + def test_create_network_filters(self): + instance_ref = self._create_instance() + network_info = _create_network_info(3) + result = \ + self.fw._create_network_filters(instance_ref, network_info, "fake") + self.assertEquals(len(result), 3) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 6c99e54489b6..57d0f4355511 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -1740,10 +1740,7 @@ class NWFilterFirewall(FirewallDriver): """ if not network_info: network_info = _get_network_info(instance) - if instance['image_id'] == FLAGS.vpn_image_id: - base_filter = 'nova-vpn' - else: - base_filter = 'nova-base' + ctxt = context.get_admin_context() @@ -1755,41 +1752,60 @@ class NWFilterFirewall(FirewallDriver): 'nova-base-ipv6', 'nova-allow-dhcp-server'] + if FLAGS.use_ipv6: + networks = [network for (network, _) in network_info if + network['gateway_v6']] + + if networks: + instance_secgroup_filter_children.\ + append('nova-allow-ra-server') + for security_group in \ db.security_group_get_by_instance(ctxt, instance['id']): self.refresh_security_group_rules(security_group['id']) - instance_secgroup_filter_children += [('nova-secgroup-%s' % - security_group['id'])] + instance_secgroup_filter_children.append('nova-secgroup-%s' % + security_group['id']) self._define_filter( self._filter_container(instance_secgroup_filter_name, instance_secgroup_filter_children)) - for (network, mapping) in network_info: + network_filters = self.\ + _create_network_filters(instance, network_info, + instance_secgroup_filter_name) + + for (name, children) in network_filters: + self._define_filters(name, children) + + + def _create_network_filters(self, instance, network_info, + instance_secgroup_filter_name): + if instance['image_id'] == FLAGS.vpn_image_id: + base_filter = 'nova-vpn' + else: + base_filter = 'nova-base' + + result = [] + for (_, mapping) in network_info: nic_id = mapping['mac'].replace(':', '') instance_filter_name = self._instance_filter_name(instance, nic_id) - instance_filter_children = \ - [base_filter, instance_secgroup_filter_name] - - if FLAGS.use_ipv6: - gateway_v6 = network['gateway_v6'] - - if gateway_v6: - instance_secgroup_filter_children += \ - ['nova-allow-ra-server'] + instance_filter_children = [base_filter, + instance_secgroup_filter_name] if FLAGS.allow_project_net_traffic: - instance_filter_children += ['nova-project'] + instance_filter_children.append('nova-project') if FLAGS.use_ipv6: - instance_filter_children += ['nova-project-v6'] + instance_filter_children.append('nova-project-v6') - self._define_filter( - self._filter_container(instance_filter_name, - instance_filter_children)) + result.append((instance_filter_name, instance_filter_children)) - return + return result + + def _define_filters(self, filter_name, filter_children): + self._define_filter(self._filter_container(filter_name, + filter_children)) def refresh_security_group_rules(self, security_group_id): return self._define_filter( From 5e74b5a5f121c9f0be2c529b76878615812d9483 Mon Sep 17 00:00:00 2001 From: Eldar Nugaev Date: Mon, 4 Apr 2011 23:43:26 +0400 Subject: [PATCH 6/7] splitting test_get_nic_for_xml into two functions --- nova/tests/test_virt.py | 23 ++++++++++++----------- nova/virt/libvirt_conn.py | 2 -- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py index b3d701efe4ad..061797b04e38 100644 --- a/nova/tests/test_virt.py +++ b/nova/tests/test_virt.py @@ -226,16 +226,18 @@ class LibvirtConnTestCase(test.TestCase): def test_get_nic_for_xml(self): conn = libvirt_conn.LibvirtConnection(True) network, mapping = _create_network_info()[0] - backup = FLAGS.use_ipv6 - FLAGS.use_ipv6 = False - params_1 = conn._get_nic_for_xml(network, mapping)['extra_params'] - FLAGS.use_ipv6 = True - params_2 = conn._get_nic_for_xml(network, mapping)['extra_params'] - self.assertTrue(params_1.find('PROJNETV6') == -1) - self.assertTrue(params_1.find('PROJMASKV6') == -1) - self.assertTrue(params_2.find('PROJNETV6') > -1) - self.assertTrue(params_2.find('PROJMASKV6') > -1) - FLAGS.use_ipv6 = backup + self.flags(use_ipv6=False) + params = conn._get_nic_for_xml(network, mapping)['extra_params'] + self.assertTrue(params.find('PROJNETV6') == -1) + self.assertTrue(params.find('PROJMASKV6') == -1) + + def test_get_nic_for_xml_v6(self): + conn = libvirt_conn.LibvirtConnection(True) + network, mapping = _create_network_info()[0] + self.flags(use_ipv6=True) + params = conn._get_nic_for_xml(network, mapping)['extra_params'] + self.assertTrue(params.find('PROJNETV6') > -1) + self.assertTrue(params.find('PROJMASKV6') > -1) def test_xml_and_uri_no_ramdisk_no_kernel(self): instance_data = dict(self.test_instance) @@ -959,7 +961,6 @@ class NWFilterTestCase(test.TestCase): self.teardown_security_group() db.instance_destroy(admin_ctxt, instance_ref['id']) - def test_create_network_filters(self): instance_ref = self._create_instance() network_info = _create_network_info(3) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 57d0f4355511..0ca2cce9a50c 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -1741,7 +1741,6 @@ class NWFilterFirewall(FirewallDriver): if not network_info: network_info = _get_network_info(instance) - ctxt = context.get_admin_context() instance_secgroup_filter_name = \ @@ -1779,7 +1778,6 @@ class NWFilterFirewall(FirewallDriver): for (name, children) in network_filters: self._define_filters(name, children) - def _create_network_filters(self, instance, network_info, instance_secgroup_filter_name): if instance['image_id'] == FLAGS.vpn_image_id: From a4b78306d31e1ef84d5dc9550ef2dcb1ed030fa2 Mon Sep 17 00:00:00 2001 From: Eldar Nugaev Date: Wed, 20 Apr 2011 21:34:55 +0400 Subject: [PATCH 7/7] fix after review: style, improving tests, replacing underscore --- nova/tests/test_virt.py | 20 ++++++++++++++------ nova/virt/libvirt_conn.py | 14 +++++++------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py index 19e4d54286b1..2e6fae6c7f2c 100644 --- a/nova/tests/test_virt.py +++ b/nova/tests/test_virt.py @@ -223,7 +223,7 @@ class LibvirtConnTestCase(test.TestCase): _create_network_info(2)) self.assertTrue(len(result['nics']) == 2) - def test_get_nic_for_xml(self): + def test_get_nic_for_xml_v4(self): conn = libvirt_conn.LibvirtConnection(True) network, mapping = _create_network_info()[0] self.flags(use_ipv6=False) @@ -794,8 +794,11 @@ class IptablesFirewallTestCase(test.TestCase): self.assertEquals(len(rulesv6), 3) def multinic_iptables_test(self): + ipv4_rules_per_network = 2 + ipv6_rules_per_network = 3 + networks_count = 5 instance_ref = self._create_instance_ref() - network_info = _create_network_info() + network_info = _create_network_info(networks_count) ipv4_len = len(self.fw.iptables.ipv4['filter'].rules) ipv6_len = len(self.fw.iptables.ipv6['filter'].rules) inst_ipv4, inst_ipv6 = self.fw.instance_rules(instance_ref, @@ -803,8 +806,12 @@ class IptablesFirewallTestCase(test.TestCase): self.fw.add_filters_for_instance(instance_ref, network_info) ipv4 = self.fw.iptables.ipv4['filter'].rules ipv6 = self.fw.iptables.ipv6['filter'].rules - self.assertEquals(len(ipv4) - len(inst_ipv4) - ipv4_len, 2) - self.assertEquals(len(ipv6) - len(inst_ipv6) - ipv6_len, 3) + ipv4_network_rules = len(ipv4) - len(inst_ipv4) - ipv4_len + ipv6_network_rules = len(ipv6) - len(inst_ipv6) - ipv6_len + self.assertEquals(ipv4_network_rules, + ipv4_rules_per_network * networks_count) + self.assertEquals(ipv6_network_rules, + ipv6_rules_per_network * networks_count) class NWFilterTestCase(test.TestCase): @@ -965,6 +972,7 @@ class NWFilterTestCase(test.TestCase): def test_create_network_filters(self): instance_ref = self._create_instance() network_info = _create_network_info(3) - result = \ - self.fw._create_network_filters(instance_ref, network_info, "fake") + result = self.fw._create_network_filters(instance_ref, + network_info, + "fake") self.assertEquals(len(result), 3) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 73a804014501..7e8ff409afdc 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -1839,12 +1839,12 @@ class NWFilterFirewall(FirewallDriver): 'nova-allow-dhcp-server'] if FLAGS.use_ipv6: - networks = [network for (network, _) in network_info if + networks = [network for (network, _m) in network_info if network['gateway_v6']] if networks: instance_secgroup_filter_children.\ - append('nova-allow-ra-server') + append('nova-allow-ra-server') for security_group in \ db.security_group_get_by_instance(ctxt, instance['id']): @@ -1859,8 +1859,8 @@ class NWFilterFirewall(FirewallDriver): instance_secgroup_filter_children)) network_filters = self.\ - _create_network_filters(instance, network_info, - instance_secgroup_filter_name) + _create_network_filters(instance, network_info, + instance_secgroup_filter_name) for (name, children) in network_filters: self._define_filters(name, children) @@ -1873,7 +1873,7 @@ class NWFilterFirewall(FirewallDriver): base_filter = 'nova-base' result = [] - for (_, mapping) in network_info: + for (_n, mapping) in network_info: nic_id = mapping['mac'].replace(':', '') instance_filter_name = self._instance_filter_name(instance, nic_id) instance_filter_children = [base_filter, @@ -1996,11 +1996,11 @@ class IptablesFirewallDriver(FirewallDriver): return ['-d %s -j $%s' % (ip, chain_name) for ip in ips] def _filters_for_instance(self, chain_name, network_info): - ips_v4 = [ip['ip'] for (_, mapping) in network_info + ips_v4 = [ip['ip'] for (_n, mapping) in network_info for ip in mapping['ips']] ipv4_rules = self._create_filter(ips_v4, chain_name) - ips_v6 = [ip['ip'] for (_, mapping) in network_info + ips_v6 = [ip['ip'] for (_n, mapping) in network_info for ip in mapping['ip6s']] ipv6_rules = self._create_filter(ips_v6, chain_name)