From ec6ed98cfaca455a663c0e6909b7faeb703a349e Mon Sep 17 00:00:00 2001 From: Swaminathan Vasudevan Date: Mon, 26 Sep 2016 14:08:50 -0700 Subject: [PATCH] DVR: Fix IPtables driver for metering with DVR routers IPTables driver for metering was not handling the DVR router namespaces properly for configuring the metering forward rules. This patch addresses the issue by configuring the iptables manager based on the availability of the namespace and selecting the respective namespaces such as router namespace and snat namespace with the right external device. Change-Id: I6790d6ff42d9f8fa220e1a231ae94cbf8b60506c Closes-Bug: #1506567 --- neutron/db/metering/metering_db.py | 3 + .../drivers/iptables/iptables_driver.py | 248 +++++++++++------- .../metering/drivers/test_iptables.py | 162 ++++++++++++ .../services/metering/test_metering_plugin.py | 9 + 4 files changed, 321 insertions(+), 101 deletions(-) diff --git a/neutron/db/metering/metering_db.py b/neutron/db/metering/metering_db.py index f8e8828dada..6e05fccff3b 100644 --- a/neutron/db/metering/metering_db.py +++ b/neutron/db/metering/metering_db.py @@ -20,6 +20,7 @@ from neutron.api.rpc.agentnotifiers import metering_rpc_agent_api from neutron.common import _deprecate from neutron.common import constants from neutron.db import common_db_mixin as base_db +from neutron.db import l3_dvr_db from neutron.db.models import l3 as l3_models from neutron.db.models import metering as metering_models from neutron.extensions import metering @@ -180,12 +181,14 @@ class MeteringDbMixin(metering.MeteringPluginBase, return rules def _make_router_dict(self, router): + distributed = l3_dvr_db.is_distributed_router(router) res = {'id': router['id'], 'name': router['name'], 'tenant_id': router['tenant_id'], 'admin_state_up': router['admin_state_up'], 'status': router['status'], 'gw_port_id': router['gw_port_id'], + 'distributed': distributed, constants.METERING_LABEL_KEY: []} return res diff --git a/neutron/services/metering/drivers/iptables/iptables_driver.py b/neutron/services/metering/drivers/iptables/iptables_driver.py index 003524c049f..73e8a548d32 100644 --- a/neutron/services/metering/drivers/iptables/iptables_driver.py +++ b/neutron/services/metering/drivers/iptables/iptables_driver.py @@ -20,7 +20,10 @@ import six from neutron._i18n import _, _LE, _LI from neutron.agent.common import config +from neutron.agent.l3 import dvr_snat_ns +from neutron.agent.l3 import namespaces from neutron.agent.linux import interface +from neutron.agent.linux import ip_lib from neutron.agent.linux import iptables_manager from neutron.common import constants as constants from neutron.common import ipv6_utils @@ -31,6 +34,7 @@ LOG = logging.getLogger(__name__) NS_PREFIX = 'qrouter-' WRAP_NAME = 'neutron-meter' EXTERNAL_DEV_PREFIX = 'qg-' +ROUTER_2_FIP_DEV_PREFIX = namespaces.ROUTER_2_FIP_DEV_PREFIX TOP_CHAIN = WRAP_NAME + "-FORWARD" RULE = '-r-' LABEL = '-l-' @@ -70,11 +74,32 @@ class RouterWithMetering(object): self.router = router # TODO(cbrandily): deduplicate ns_name generation in metering/l3 self.ns_name = NS_PREFIX + self.id - self.iptables_manager = iptables_manager.IptablesManager( - namespace=self.ns_name, - binary_name=WRAP_NAME, - state_less=True, - use_ipv6=ipv6_utils.is_enabled_and_bind_by_default()) + self.iptables_manager = None + self.snat_iptables_manager = None + if self.router['distributed']: + # If distributed routers then we need to apply the + # metering agent label rules in the snat namespace as well. + snat_ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name( + self.id) + # Check for namespace existence before we assign the + # snat_iptables_manager + if ip_lib.IPWrapper().netns.exists(snat_ns_name): + self.snat_iptables_manager = iptables_manager.IptablesManager( + namespace=snat_ns_name, + binary_name=WRAP_NAME, + state_less=True, + use_ipv6=ipv6_utils.is_enabled_and_bind_by_default()) + # Check of namespace existence before we assign the iptables_manager + # NOTE(Swami): If distributed routers, all external traffic on a + # compute node will flow through the rfp interface in the router + # namespace. + ip_wrapper = ip_lib.IPWrapper(namespace=self.ns_name) + if ip_wrapper.netns.exists(self.ns_name): + self.iptables_manager = iptables_manager.IptablesManager( + namespace=self.ns_name, + binary_name=WRAP_NAME, + state_less=True, + use_ipv6=ipv6_utils.is_enabled_and_bind_by_default()) self.metering_labels = {} @@ -117,7 +142,12 @@ class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver): if gw_port_id != old_gw_port_id: if old_rm: - with IptablesManagerTransaction(old_rm.iptables_manager): + if router.get('distributed'): + old_rm_im = old_rm.snat_iptables_manager + else: + old_rm_im = old_rm.iptables_manager + + with IptablesManagerTransaction(old_rm_im): self._process_disassociate_metering_label(router) if gw_port_id: self._process_associate_metering_label(router) @@ -129,32 +159,34 @@ class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver): if router_id in self.routers: del self.routers[router_id] - def get_external_device_name(self, port_id): - return (EXTERNAL_DEV_PREFIX + port_id)[:self.driver.DEV_NAME_LEN] + def get_external_device_names(self, rm): + gw_port_id = rm.router.get('gw_port_id') + if not gw_port_id: + return None, None - def _process_metering_label_rules(self, rm, rules, label_chain, - rules_chain): - im = rm.iptables_manager - ex_gw_port = rm.router.get('gw_port_id') - if not ex_gw_port: - return + # NOTE (Swami): External device 'qg' should be used on the + # Router namespace if the router is legacy and should be used on + # SNAT namespace if the router is distributed. + ext_dev = (EXTERNAL_DEV_PREFIX + + gw_port_id)[:self.driver.DEV_NAME_LEN] + ext_snat_dev = (ROUTER_2_FIP_DEV_PREFIX + + rm.id)[:self.driver.DEV_NAME_LEN] + return ext_dev, ext_snat_dev - ext_dev = self.get_external_device_name(ex_gw_port) + def _process_metering_label_rules(self, rules, label_chain, + rules_chain, ext_dev, im): if not ext_dev: return - for rule in rules: self._add_rule_to_chain(ext_dev, rule, im, label_chain, rules_chain) - def _process_metering_label_rule_add(self, rm, rule, ext_dev, - label_chain, rules_chain): - im = rm.iptables_manager + def _process_metering_label_rule_add(self, rule, ext_dev, + label_chain, rules_chain, im): self._add_rule_to_chain(ext_dev, rule, im, label_chain, rules_chain) - def _process_metering_label_rule_delete(self, rm, rule, ext_dev, - label_chain, rules_chain): - im = rm.iptables_manager + def _process_metering_label_rule_delete(self, rule, ext_dev, + label_chain, rules_chain, im): self._remove_rule_from_chain(ext_dev, rule, im, label_chain, rules_chain) @@ -191,67 +223,75 @@ class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver): ipt_rule = '%s -j %s' % (dir_opt, label_chain) return ipt_rule - def _process_associate_metering_label(self, router): - self._update_router(router) + def _process_ns_specific_metering_label(self, router, ext_dev, im): + '''Process metering label based on the associated namespaces.''' rm = self.routers.get(router['id']) - - with IptablesManagerTransaction(rm.iptables_manager): + with IptablesManagerTransaction(im): labels = router.get(constants.METERING_LABEL_KEY, []) for label in labels: label_id = label['id'] - label_chain = iptables_manager.get_chain_name(WRAP_NAME + - LABEL + label_id, - wrap=False) - rm.iptables_manager.ipv4['filter'].add_chain(label_chain, - wrap=False) + label_chain = iptables_manager.get_chain_name( + WRAP_NAME + LABEL + label_id, wrap=False) + im.ipv4['filter'].add_chain(label_chain, wrap=False) - rules_chain = iptables_manager.get_chain_name(WRAP_NAME + - RULE + label_id, - wrap=False) - rm.iptables_manager.ipv4['filter'].add_chain(rules_chain, - wrap=False) - rm.iptables_manager.ipv4['filter'].add_rule(TOP_CHAIN, '-j ' + - rules_chain, - wrap=False) + rules_chain = iptables_manager.get_chain_name( + WRAP_NAME + RULE + label_id, wrap=False) + im.ipv4['filter'].add_chain(rules_chain, wrap=False) + im.ipv4['filter'].add_rule( + TOP_CHAIN, '-j ' + rules_chain, wrap=False) - rm.iptables_manager.ipv4['filter'].add_rule(label_chain, - '', - wrap=False) + im.ipv4['filter'].add_rule( + label_chain, '', wrap=False) rules = label.get('rules') if rules: - self._process_metering_label_rules(rm, rules, - label_chain, - rules_chain) + self._process_metering_label_rules( + rules, label_chain, rules_chain, ext_dev, im) rm.metering_labels[label_id] = label - def _process_disassociate_metering_label(self, router): + def _process_associate_metering_label(self, router): + self._update_router(router) rm = self.routers.get(router['id']) - if not rm: - return - with IptablesManagerTransaction(rm.iptables_manager): + ext_dev, ext_snat_dev = self.get_external_device_names(rm) + for (im, dev) in [(rm.iptables_manager, ext_dev), + (rm.snat_iptables_manager, ext_snat_dev)]: + if im: + self._process_ns_specific_metering_label(router, dev, im) + + def _process_ns_specific_disassociate_metering_label(self, router, im): + '''Disassociate metering label based on specific namespaces.''' + rm = self.routers.get(router['id']) + with IptablesManagerTransaction(im): labels = router.get(constants.METERING_LABEL_KEY, []) for label in labels: label_id = label['id'] if label_id not in rm.metering_labels: continue - label_chain = iptables_manager.get_chain_name(WRAP_NAME + - LABEL + label_id, - wrap=False) - rules_chain = iptables_manager.get_chain_name(WRAP_NAME + - RULE + label_id, - wrap=False) + label_chain = iptables_manager.get_chain_name( + WRAP_NAME + LABEL + label_id, wrap=False) + rules_chain = iptables_manager.get_chain_name( + WRAP_NAME + RULE + label_id, wrap=False) + im.ipv4['filter'].remove_chain(label_chain, wrap=False) + im.ipv4['filter'].remove_chain(rules_chain, wrap=False) - rm.iptables_manager.ipv4['filter'].remove_chain(label_chain, - wrap=False) - rm.iptables_manager.ipv4['filter'].remove_chain(rules_chain, - wrap=False) + def _process_disassociate_metering_label(self, router): + rm = self.routers.get(router['id']) + if not rm: + return - del rm.metering_labels[label_id] + for im in [rm.iptables_manager, rm.snat_iptables_manager]: + if im: + self._process_ns_specific_disassociate_metering_label( + router, im) + + labels = router.get(constants.METERING_LABEL_KEY, []) + for label in labels: + label_id = label['id'] + del rm.metering_labels[label_id] @log_helpers.log_method_call def add_metering_label(self, context, routers): @@ -279,61 +319,67 @@ class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver): def _remove_metering_label_rule(self, router): self._process_metering_rule_action(router, 'delete') + def _process_metering_rule_action_based_on_ns( + self, router, action, ext_dev, im): + '''Process metering rule actions based specific namespaces.''' + with IptablesManagerTransaction(im): + labels = router.get(constants.METERING_LABEL_KEY, []) + for label in labels: + label_id = label['id'] + label_chain = iptables_manager.get_chain_name( + WRAP_NAME + LABEL + label_id, wrap=False) + + rules_chain = iptables_manager.get_chain_name( + WRAP_NAME + RULE + label_id, wrap=False) + rule = label.get('rule') + if rule: + if action == 'create': + self._process_metering_label_rule_add( + rule, ext_dev, label_chain, rules_chain, im) + elif action == 'delete': + self._process_metering_label_rule_delete( + rule, ext_dev, label_chain, rules_chain, im) + def _process_metering_rule_action(self, router, action): rm = self.routers.get(router['id']) if not rm: return - ext_dev = self.get_external_device_name(rm.router['gw_port_id']) - if not ext_dev: - return - with IptablesManagerTransaction(rm.iptables_manager): + + ext_dev, ext_snat_dev = self.get_external_device_names(rm) + for (im, dev) in [(rm.iptables_manager, ext_dev), + (rm.snat_iptables_manager, ext_snat_dev)]: + if im and dev: + self._process_metering_rule_action_based_on_ns( + router, action, dev, im) + + def _update_metering_label_rules_based_on_ns(self, router, ext_dev, im): + '''Update metering lable rules based on namespace.''' + with IptablesManagerTransaction(im): labels = router.get(constants.METERING_LABEL_KEY, []) for label in labels: label_id = label['id'] - label_chain = iptables_manager.get_chain_name(WRAP_NAME + - LABEL + label_id, - wrap=False) - rules_chain = iptables_manager.get_chain_name(WRAP_NAME + - RULE + label_id, - wrap=False) - rule = label.get('rule') - if rule: - if action == 'create': - self._process_metering_label_rule_add(rm, rule, - ext_dev, - label_chain, - rules_chain) - elif action == 'delete': - self._process_metering_label_rule_delete(rm, rule, - ext_dev, - label_chain, - rules_chain) + label_chain = iptables_manager.get_chain_name( + WRAP_NAME + LABEL + label_id, wrap=False) + rules_chain = iptables_manager.get_chain_name( + WRAP_NAME + RULE + label_id, wrap=False) + im.ipv4['filter'].empty_chain(rules_chain, wrap=False) + + rules = label.get('rules') + if rules: + self._process_metering_label_rules( + rules, label_chain, rules_chain, ext_dev, im) def _update_metering_label_rules(self, router): rm = self.routers.get(router['id']) if not rm: return - with IptablesManagerTransaction(rm.iptables_manager): - labels = router.get(constants.METERING_LABEL_KEY, []) - for label in labels: - label_id = label['id'] - - label_chain = iptables_manager.get_chain_name(WRAP_NAME + - LABEL + label_id, - wrap=False) - rules_chain = iptables_manager.get_chain_name(WRAP_NAME + - RULE + label_id, - wrap=False) - rm.iptables_manager.ipv4['filter'].empty_chain(rules_chain, - wrap=False) - - rules = label.get('rules') - if rules: - self._process_metering_label_rules(rm, rules, - label_chain, - rules_chain) + ext_dev, ext_snat_dev = self.get_external_device_names(rm) + for (im, dev) in [(rm.iptables_manager, ext_dev), + (rm.snat_iptables_manager, ext_snat_dev)]: + if im and dev: + self._update_metering_label_rules_based_on_ns(router, dev, im) @log_helpers.log_method_call def remove_metering_label(self, context, routers): diff --git a/neutron/tests/unit/services/metering/drivers/test_iptables.py b/neutron/tests/unit/services/metering/drivers/test_iptables.py index 1bbc708a015..3044a921581 100644 --- a/neutron/tests/unit/services/metering/drivers/test_iptables.py +++ b/neutron/tests/unit/services/metering/drivers/test_iptables.py @@ -34,6 +34,7 @@ TEST_ROUTERS = [ 'gw_port_id': '6d411f48-ecc7-45e0-9ece-3b5bdb54fcee', 'id': '473ec392-1711-44e3-b008-3251ccfc5099', 'name': 'router1', + 'distributed': False, 'status': 'ACTIVE', 'tenant_id': '6c5f5d2a1fa2441e88e35422926f48e8'}, {'_metering_labels': [ @@ -49,9 +50,27 @@ TEST_ROUTERS = [ 'id': '373ec392-1711-44e3-b008-3251ccfc5099', 'name': 'router2', 'status': 'ACTIVE', + 'distributed': False, 'tenant_id': '6c5f5d2a1fa2441e88e35422926f48e8'}, ] +TEST_DVR_ROUTER = [ + {'_metering_labels': [ + {'id': 'c5df2fe5-c610-4a2a-b2f4-c0fb6df73c83', + 'rules': [{ + 'direction': 'ingress', + 'excluded': False, + 'id': '7f1a261f-2600-4ed1-870c-a62754501379', + 'metering_label_id': 'c5df2fe5-c700-4a2a-b2f4-c0fb6df73c83', + 'remote_ip_prefix': '10.0.0.0/24'}]}], + 'admin_state_up': True, + 'gw_port_id': '6d411f48-ecc7-45e0-9ece-3b5bdb54fcee', + 'id': '473ec392-2711-44e3-b008-3251ccfc5099', + 'name': 'router-test', + 'distributed': True, + 'status': 'ACTIVE', + 'tenant_id': '6c5f5d2a1fa2441e88e35422926f48e8'}] + TEST_ROUTERS_WITH_ONE_RULE = [ {'_metering_labels': [ {'id': 'c5df2fe5-c600-4a2a-b2f4-c0fb6df73c83', @@ -66,6 +85,7 @@ TEST_ROUTERS_WITH_ONE_RULE = [ 'id': '473ec392-1711-44e3-b008-3251ccfc5099', 'name': 'router1', 'status': 'ACTIVE', + 'distributed': False, 'tenant_id': '6c5f5d2a1fa2441e88e35422926f48e8'}, {'_metering_labels': [ {'id': 'eeef45da-c600-4a2a-b2f4-c0fb6df73c83', @@ -79,6 +99,7 @@ TEST_ROUTERS_WITH_ONE_RULE = [ 'gw_port_id': '7d411f48-ecc7-45e0-9ece-3b5bdb54fcee', 'id': '373ec392-1711-44e3-b008-3251ccfc5099', 'name': 'router2', + 'distributed': False, 'status': 'ACTIVE', 'tenant_id': '6c5f5d2a1fa2441e88e35422926f48e8'}, ] @@ -96,6 +117,12 @@ class IptablesDriverTestCase(base.BaseTestCase): self.iptables_inst = mock.Mock() self.v4filter_inst = mock.Mock() self.v6filter_inst = mock.Mock() + self.namespace_exists_p = mock.patch( + 'neutron.agent.linux.ip_lib.IpNetnsCommand.exists') + self.namespace_exists = self.namespace_exists_p.start() + self.snat_ns_name_p = mock.patch( + 'neutron.agent.l3.dvr_snat_ns.SnatNamespace.get_snat_ns_name') + self.snat_ns_name = self.snat_ns_name_p.start() self.v4filter_inst.chains = [] self.v6filter_inst.chains = [] self.iptables_inst.ipv4 = {'filter': self.v4filter_inst} @@ -108,16 +135,42 @@ class IptablesDriverTestCase(base.BaseTestCase): def test_create_stateless_iptables_manager(self): routers = TEST_ROUTERS[:1] + self.namespace_exists.return_value = True self.metering.add_metering_label(None, routers) + self.assertEqual(1, self.iptables_cls.call_count) self.iptables_cls.assert_called_with( binary_name=mock.ANY, namespace=mock.ANY, state_less=True, use_ipv6=mock.ANY) + rm = iptables_driver.RouterWithMetering(self.metering.conf, routers[0]) + self.assertTrue(rm.iptables_manager) + self.assertIsNone(rm.snat_iptables_manager) + + def test_iptables_manager_never_create_with_no_valid_namespace(self): + routers = TEST_ROUTERS[:1] + self.namespace_exists.return_value = False + self.metering.add_metering_label(None, routers) + self.assertFalse(self.iptables_cls.called) + rm = iptables_driver.RouterWithMetering(self.metering.conf, routers[0]) + self.assertIsNone(rm.iptables_manager) + self.assertIsNone(rm.snat_iptables_manager) + + def test_create_iptables_manager_for_distributed_routers(self): + routers = TEST_DVR_ROUTER[:1] + self.namespace_exists.return_value = True + snat_ns_name = 'snat-' + routers[0]['id'] + self.snat_ns_name.return_value = snat_ns_name + self.metering.add_metering_label(None, routers) + self.assertEqual(2, self.iptables_cls.call_count) + rm = iptables_driver.RouterWithMetering(self.metering.conf, routers[0]) + self.assertTrue(rm.iptables_manager) + self.assertTrue(rm.snat_iptables_manager) def test_add_metering_label(self): routers = TEST_ROUTERS[:1] + self.namespace_exists.return_value = True self.metering.add_metering_label(None, routers) calls = [mock.call.add_chain('neutron-meter-l-c5df2fe5-c60', wrap=False), @@ -132,7 +185,52 @@ class IptablesDriverTestCase(base.BaseTestCase): self.v4filter_inst.assert_has_calls(calls) + def test_add_metering_label_dvr_routers(self): + routers = TEST_DVR_ROUTER[:1] + + self.namespace_exists.return_value = True + snat_ns_name = 'snat-' + routers[0]['id'] + self.snat_ns_name.return_value = snat_ns_name + self.metering._process_ns_specific_metering_label = mock.Mock() + self.metering.add_metering_label(None, routers) + rm = iptables_driver.RouterWithMetering(self.metering.conf, routers[0]) + ext_dev, ext_snat_dev = self.metering.get_external_device_names(rm) + self.assertEqual( + 2, self.metering._process_ns_specific_metering_label.call_count) + # check and validate the right device being passed based on the + # namespace. + self.assertEqual( + self.metering._process_ns_specific_metering_label.mock_calls, + [mock.call( + routers[0], ext_dev, rm.iptables_manager), + mock.call( + routers[0], ext_snat_dev, rm.snat_iptables_manager)]) + + def test_add_metering_label_legacy_routers(self): + routers = TEST_ROUTERS[:1] + + self.namespace_exists.return_value = True + self.metering._process_ns_specific_metering_label = mock.Mock() + self.metering.add_metering_label(None, routers) + rm = iptables_driver.RouterWithMetering(self.metering.conf, routers[0]) + ext_dev, _ = self.metering.get_external_device_names(rm) + self.assertEqual( + self.metering._process_ns_specific_metering_label.mock_calls, + [mock.call(routers[0], ext_dev, rm.iptables_manager)]) + + def test_add_metering_label_when_no_namespace(self): + routers = TEST_ROUTERS[:1] + + self.namespace_exists.return_value = False + self.metering._process_metering_label = mock.Mock() + self.metering.add_metering_label(None, routers) + rm = iptables_driver.RouterWithMetering(self.metering.conf, routers[0]) + self.assertIsNone(rm.iptables_manager) + self.assertIsNone(rm.snat_iptables_manager) + self.assertFalse(self.metering._process_metering_label.called) + def test_process_metering_label_rules(self): + self.namespace_exists.return_value = True self.metering.add_metering_label(None, TEST_ROUTERS) calls = [mock.call.add_chain('neutron-meter-l-c5df2fe5-c60', @@ -171,6 +269,7 @@ class IptablesDriverTestCase(base.BaseTestCase): for router in routers: router['gw_port_id'] = None + self.namespace_exists.return_value = True self.metering.add_metering_label(None, routers) calls = [mock.call.add_chain('neutron-meter-l-c5df2fe5-c60', @@ -203,6 +302,7 @@ class IptablesDriverTestCase(base.BaseTestCase): 'excluded': True, }) + self.namespace_exists.return_value = True self.metering.add_metering_label(None, routers) calls = [mock.call.add_chain('neutron-meter-l-c5df2fe5-c60', wrap=False), @@ -238,6 +338,7 @@ class IptablesDriverTestCase(base.BaseTestCase): def test_update_metering_label_rules(self): routers = TEST_ROUTERS[:1] + self.namespace_exists.return_value = True self.metering.add_metering_label(None, routers) updates = copy.deepcopy(routers) @@ -292,6 +393,7 @@ class IptablesDriverTestCase(base.BaseTestCase): 'remote_ip_prefix': '20.0.0.0/24', }) + self.namespace_exists.return_value = True self.metering.add_metering_label(None, routers) del routers[0]['_metering_labels'][0]['rules'][1] @@ -327,6 +429,7 @@ class IptablesDriverTestCase(base.BaseTestCase): def test_add_metering_label_rule(self): new_routers_rules = TEST_ROUTERS_WITH_ONE_RULE self.metering.update_routers(None, TEST_ROUTERS) + self.namespace_exists.return_value = True self.metering.add_metering_label_rule(None, new_routers_rules) calls = [ mock.call.add_rule('neutron-meter-r-c5df2fe5-c60', @@ -341,9 +444,53 @@ class IptablesDriverTestCase(base.BaseTestCase): ] self.v4filter_inst.assert_has_calls(calls) + def test_add_metering_label_rule_dvr_router(self): + routers = TEST_DVR_ROUTER + self.metering.update_routers(None, TEST_DVR_ROUTER) + self.namespace_exists.return_value = True + self.metering._process_metering_rule_action_based_on_ns = mock.Mock() + self.metering.add_metering_label_rule(None, routers) + rm = iptables_driver.RouterWithMetering(self.metering.conf, routers[0]) + ext_dev, ext_snat_dev = self.metering.get_external_device_names(rm) + self.assertEqual( + 2, + self.metering._process_metering_rule_action_based_on_ns.call_count) + # check and validate the right device being passed based on the + # namespace. + self.assertEqual( + self.metering._process_metering_rule_action_based_on_ns.mock_calls, + [mock.call( + routers[0], 'create', ext_dev, rm.iptables_manager), + mock.call( + routers[0], 'create', ext_snat_dev, + rm.snat_iptables_manager)]) + + def test_remove_metering_label_rule_dvr_router(self): + routers = TEST_DVR_ROUTER + self.metering.update_routers(None, TEST_DVR_ROUTER) + self.namespace_exists.return_value = True + self.metering.add_metering_label_rule(None, routers) + self.metering._process_metering_rule_action_based_on_ns = mock.Mock() + self.metering.remove_metering_label_rule(None, routers) + rm = iptables_driver.RouterWithMetering(self.metering.conf, routers[0]) + ext_dev, ext_snat_dev = self.metering.get_external_device_names(rm) + self.assertEqual( + 2, + self.metering._process_metering_rule_action_based_on_ns.call_count) + # check and validate the right device being passed based on the + # namespace. + self.assertEqual( + self.metering._process_metering_rule_action_based_on_ns.mock_calls, + [mock.call( + routers[0], 'delete', ext_dev, rm.iptables_manager), + mock.call( + routers[0], 'delete', ext_snat_dev, + rm.snat_iptables_manager)]) + def test_remove_metering_label_rule(self): new_routers_rules = TEST_ROUTERS_WITH_ONE_RULE self.metering.update_routers(None, TEST_ROUTERS) + self.namespace_exists.return_value = True self.metering.add_metering_label_rule(None, new_routers_rules) self.metering.remove_metering_label_rule(None, new_routers_rules) calls = [ @@ -361,6 +508,7 @@ class IptablesDriverTestCase(base.BaseTestCase): def test_remove_metering_label(self): routers = TEST_ROUTERS[:1] + self.namespace_exists.return_value = True self.metering.add_metering_label(None, routers) self.metering.remove_metering_label(None, routers) calls = [mock.call.add_chain('neutron-meter-l-c5df2fe5-c60', @@ -384,6 +532,18 @@ class IptablesDriverTestCase(base.BaseTestCase): self.v4filter_inst.assert_has_calls(calls) + def test_remove_metering_label_with_dvr_routers(self): + routers = TEST_DVR_ROUTER[:1] + + self.namespace_exists.return_value = True + self.metering.add_metering_label(None, routers) + self.metering._process_ns_specific_disassociate_metering_label = ( + mock.Mock()) + self.metering.remove_metering_label(None, routers) + self.assertEqual( + 2, (self.metering. + _process_ns_specific_disassociate_metering_label.call_count)) + def test_update_routers(self): routers = copy.deepcopy(TEST_ROUTERS) routers[1]['_metering_labels'][0]['rules'][0].update({ @@ -391,6 +551,7 @@ class IptablesDriverTestCase(base.BaseTestCase): 'excluded': True, }) + self.namespace_exists.return_value = True self.metering.add_metering_label(None, routers) updates = copy.deepcopy(routers) @@ -449,6 +610,7 @@ class IptablesDriverTestCase(base.BaseTestCase): def test_update_routers_removal(self): routers = TEST_ROUTERS + self.namespace_exists.return_value = True self.metering.add_metering_label(None, routers) # Remove router id '373ec392-1711-44e3-b008-3251ccfc5099' diff --git a/neutron/tests/unit/services/metering/test_metering_plugin.py b/neutron/tests/unit/services/metering/test_metering_plugin.py index 7411a25212a..8caab86684f 100644 --- a/neutron/tests/unit/services/metering/test_metering_plugin.py +++ b/neutron/tests/unit/services/metering/test_metering_plugin.py @@ -138,6 +138,7 @@ class TestMeteringPlugin(test_db_base_plugin_v2.NeutronDbPluginV2TestCase, 'name': 'router1', 'gw_port_id': None, 'admin_state_up': True, + 'distributed': False, 'tenant_id': self.tenant_id, '_metering_labels': [ {'rules': [], @@ -161,6 +162,7 @@ class TestMeteringPlugin(test_db_base_plugin_v2.NeutronDbPluginV2TestCase, 'name': 'router1', 'gw_port_id': None, 'admin_state_up': True, + 'distributed': False, 'tenant_id': self.tenant_id, '_metering_labels': [ {'rules': [], @@ -184,6 +186,7 @@ class TestMeteringPlugin(test_db_base_plugin_v2.NeutronDbPluginV2TestCase, 'name': 'router1', 'gw_port_id': None, 'admin_state_up': True, + 'distributed': False, 'tenant_id': self.tenant_id, '_metering_labels': [ {'rules': [], @@ -204,6 +207,7 @@ class TestMeteringPlugin(test_db_base_plugin_v2.NeutronDbPluginV2TestCase, 'name': 'router1', 'gw_port_id': None, 'admin_state_up': True, + 'distributed': False, 'tenant_id': self.tenant_id, '_metering_labels': [ {'rules': [], @@ -215,6 +219,7 @@ class TestMeteringPlugin(test_db_base_plugin_v2.NeutronDbPluginV2TestCase, 'name': 'router1', 'gw_port_id': None, 'admin_state_up': True, + 'distributed': False, 'tenant_id': self.tenant_id, '_metering_labels': [ {'rules': [], @@ -238,6 +243,7 @@ class TestMeteringPlugin(test_db_base_plugin_v2.NeutronDbPluginV2TestCase, 'name': 'router1', 'gw_port_id': None, 'admin_state_up': True, + 'distributed': False, 'tenant_id': self.tenant_id, '_metering_labels': [ {'rule': { @@ -253,6 +259,7 @@ class TestMeteringPlugin(test_db_base_plugin_v2.NeutronDbPluginV2TestCase, 'name': 'router1', 'gw_port_id': None, 'admin_state_up': True, + 'distributed': False, 'tenant_id': self.tenant_id, '_metering_labels': [ {'rule': { @@ -351,6 +358,7 @@ class TestMeteringPluginL3AgentScheduler( 'name': 'router1', 'gw_port_id': None, 'admin_state_up': True, + 'distributed': False, 'tenant_id': self.tenant_id, '_metering_labels': [ {'rules': [], @@ -360,6 +368,7 @@ class TestMeteringPluginL3AgentScheduler( 'name': 'router2', 'gw_port_id': None, 'admin_state_up': True, + 'distributed': False, 'tenant_id': self.tenant_id, '_metering_labels': [ {'rules': [],