Retry creating iptables managers and adding metering rules

This change makes the metering agent retry creating the iptables
managers for each router and applying the metering rules.
This is needed in case the metering agent starts before some or all of
the namespaces are created.

Change-Id: Ifc565feb98c7f02df5c2831a3607c3e526a2e703
Closes-Bug: #1807153
This commit is contained in:
bno1 2019-06-23 00:51:02 +03:00
parent 6945fc9f30
commit 9f541521bb
6 changed files with 114 additions and 18 deletions

View File

@ -148,6 +148,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()
@ -214,6 +215,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')

View File

@ -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

View File

@ -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)

View File

@ -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

View File

@ -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)

View File

@ -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)