diff --git a/neutron/services/metering/agents/metering_agent.py b/neutron/services/metering/agents/metering_agent.py index 03540f8feb9..a5da6dfc50e 100644 --- a/neutron/services/metering/agents/metering_agent.py +++ b/neutron/services/metering/agents/metering_agent.py @@ -149,6 +149,7 @@ class MeteringAgent(MeteringPluginRpc, manager.Manager): self._add_metering_info(label_id, acc['pkts'], acc['bytes']) def _metering_loop(self): + self._sync_router_namespaces(self.context, self.routers.values()) self._add_metering_infos() ts = timeutils.utcnow_ts() @@ -215,6 +216,10 @@ class MeteringAgent(MeteringPluginRpc, manager.Manager): LOG.debug("Get router traffic counters") return self._invoke_driver(context, routers, 'get_traffic_counters') + def _sync_router_namespaces(self, context, routers): + LOG.debug("Sync router namespaces") + return self._invoke_driver(context, routers, 'sync_router_namespaces') + def add_metering_label_rule(self, context, routers): return self._invoke_driver(context, routers, 'add_metering_label_rule') diff --git a/neutron/services/metering/drivers/abstract_driver.py b/neutron/services/metering/drivers/abstract_driver.py index 5169bdd97ba..ce7e8561d63 100644 --- a/neutron/services/metering/drivers/abstract_driver.py +++ b/neutron/services/metering/drivers/abstract_driver.py @@ -47,3 +47,7 @@ class MeteringAbstractDriver(object): @abc.abstractmethod def get_traffic_counters(self, context, routers): pass + + @abc.abstractmethod + def sync_router_namespaces(self, context, routers): + pass diff --git a/neutron/services/metering/drivers/iptables/iptables_driver.py b/neutron/services/metering/drivers/iptables/iptables_driver.py index 69c3955a421..9c601efff0f 100644 --- a/neutron/services/metering/drivers/iptables/iptables_driver.py +++ b/neutron/services/metering/drivers/iptables/iptables_driver.py @@ -73,7 +73,19 @@ class RouterWithMetering(object): self.ns_name = NS_PREFIX + self.id self.iptables_manager = None self.snat_iptables_manager = None - if self.router['distributed']: + self.metering_labels = {} + + self.create_iptables_managers() + + def create_iptables_managers(self): + """Creates iptables managers if the are not already created + + Returns True if any manager is created + """ + + created = False + + if self.router['distributed'] and self.snat_iptables_manager is None: # 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( @@ -86,17 +98,25 @@ class RouterWithMetering(object): 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. - if ip_lib.network_namespace_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 = {} + + created = True + + if self.iptables_manager is None: + # 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. + if ip_lib.network_namespace_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()) + + created = True + + return created class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver): @@ -109,8 +129,11 @@ class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver): self.driver = common_utils.load_interface_driver(self.conf) def _update_router(self, router): - r = self.routers.get(router['id'], - RouterWithMetering(self.conf, router)) + r = self.routers.get(router['id']) + + if r is None: + r = RouterWithMetering(self.conf, router) + r.router = router self.routers[r.id] = r @@ -138,10 +161,17 @@ class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver): 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) + # In case the selected manager is None pick another one. + # This is not ideal sometimes. + old_rm_im = (old_rm_im or + old_rm.snat_iptables_manager or + old_rm.iptables_manager) + + if old_rm_im: + with IptablesManagerTransaction(old_rm_im): + self._process_disassociate_metering_label(router) + if gw_port_id: + self._process_associate_metering_label(router) elif gw_port_id: self._process_associate_metering_label(router) @@ -430,3 +460,18 @@ class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver): del self.routers[router_id] return accs + + @log_helpers.log_method_call + def sync_router_namespaces(self, context, routers): + for router in routers: + rm = self.routers.get(router['id']) + if not rm: + continue + + # NOTE(bno1): Sometimes a router is added before its namespaces are + # created. The metering agent has to periodically check if the + # namespaces for the missing iptables managers have appearead and + # create the managers for them. When a new manager is created, the + # metering rules have to be added to it. + if rm.create_iptables_managers(): + self._process_associate_metering_label(router) diff --git a/neutron/services/metering/drivers/noop/noop_driver.py b/neutron/services/metering/drivers/noop/noop_driver.py index 25df069fa72..2bacbe3543e 100644 --- a/neutron/services/metering/drivers/noop/noop_driver.py +++ b/neutron/services/metering/drivers/noop/noop_driver.py @@ -50,3 +50,7 @@ class NoopMeteringDriver(abstract_driver.MeteringAbstractDriver): @log_helpers.log_method_call def get_traffic_counters(self, context, routers): pass + + @log_helpers.log_method_call + def sync_router_namespaces(self, context, routers): + pass diff --git a/neutron/tests/unit/services/metering/agents/test_metering_agent.py b/neutron/tests/unit/services/metering/agents/test_metering_agent.py index f657f9ffd28..f7001f50544 100644 --- a/neutron/tests/unit/services/metering/agents/test_metering_agent.py +++ b/neutron/tests/unit/services/metering/agents/test_metering_agent.py @@ -103,6 +103,10 @@ class TestMeteringOperations(base.BaseTestCase): self.agent._get_traffic_counters(None, ROUTERS) self.assertEqual(1, self.driver.get_traffic_counters.call_count) + def test_sync_router_namespaces(self): + self.agent._sync_router_namespaces(None, ROUTERS) + self.assertEqual(1, self.driver.sync_router_namespaces.call_count) + def test_notification_report(self): self.agent.routers_updated(None, ROUTERS) diff --git a/neutron/tests/unit/services/metering/drivers/test_iptables.py b/neutron/tests/unit/services/metering/drivers/test_iptables.py index aaaf16780d3..dcbdfe53fae 100644 --- a/neutron/tests/unit/services/metering/drivers/test_iptables.py +++ b/neutron/tests/unit/services/metering/drivers/test_iptables.py @@ -683,3 +683,37 @@ class IptablesDriverTestCase(base.BaseTestCase): self.assertIn(expected_label_id, counters) self.assertEqual(1, counters[expected_label_id]['pkts']) self.assertEqual(8, counters[expected_label_id]['bytes']) + + def test_sync_router_namespaces(self): + routers = TEST_DVR_ROUTER[:1] + + self.metering._process_ns_specific_metering_label = mock.Mock() + self.namespace_exists.return_value = False + self.metering.add_metering_label(None, routers) + rm = self.metering.routers[routers[0]['id']] + self.assertEqual( + 0, self.metering._process_ns_specific_metering_label.call_count) + self.assertIsNone(rm.snat_iptables_manager) + self.assertIsNone(rm.iptables_manager) + + self.namespace_exists.side_effect = [True, False] + self.metering.sync_router_namespaces(None, routers) + self.assertIsNotNone(rm.snat_iptables_manager) + self.assertIsNone(rm.iptables_manager) + self.assertEqual( + 1, self.metering._process_ns_specific_metering_label.call_count) + + self.namespace_exists.side_effect = [True] + self.metering.sync_router_namespaces(None, routers) + self.assertIsNotNone(rm.snat_iptables_manager) + self.assertIsNotNone(rm.iptables_manager) + self.assertEqual( + 3, self.metering._process_ns_specific_metering_label.call_count) + + # syncing again should have no effect + self.namespace_exists.side_effect = [RuntimeError('Unexpected call')] + self.metering.sync_router_namespaces(None, routers) + self.assertIsNotNone(rm.snat_iptables_manager) + self.assertIsNotNone(rm.iptables_manager) + self.assertEqual( + 3, self.metering._process_ns_specific_metering_label.call_count)