Metering: sync only active routers hosted on the same host

When syncing data from neutron server, metering-agent may receive
information about routers that are not hosted by the l3-agent on the
same host, because the server didn't filter them out. This could lead to
the following problems:
  * metering-agent tries to setup iptables rules for a router that is not
    on the host
  * metering-agent tries to get get traffic counters for a router that
    was once on the host but is already removed
  * metering-agent not sets up iptables rules for a router that is
    removed then added back to the host, because nothing about the
    router is changed from metering-agent's perspective

This commit fixes the aforementioned problems by making metering-agent
only receive information about routers that are on the same host, and
update metering-agent's knowledge about which routers it should care.

However, there could still be problem if one removes then adds a router
back to the same l3-agent, or just sets the router's admin_state_up
property to False then True in a short time(shorter than the interval
between two syncs). Because the metering-agent sees nothing changed
while during the same time the router's namespace is removed and added
back on the host. Thus metering-agent will fail to get such router's
traffic counters. This commit also make iptables-driver to forget such
routers and leave the metering-agent to reconfigure them later.

Closes-Bug: #1580548

Change-Id: Ia6ae82c676582b06710d6f96b9938c215258182d
Signed-off-by: Hunt Xu <mhuntxu@gmail.com>

neutron/db/metering/metering_db.py

@@ -211,6 +211,8 @@ class MeteringDbMixin(metering.MeteringPluginBase,
                 routers = label.routers
 
             for router in routers:
+                if not router['admin_state_up']:
+                    continue
                 router_dict = routers_dict.get(
                     router['id'],
                     self._make_router_dict(router))
@@ -244,15 +246,12 @@ class MeteringDbMixin(metering.MeteringPluginBase,
 
         return list(routers_dict.values())
 
-    def get_sync_data_metering(self, context, label_id=None, router_ids=None):
+    def get_sync_data_metering(self, context, label_id=None):
         labels = context.session.query(metering_models.MeteringLabel)
 
         if label_id:
             labels = labels.filter(
                 metering_models.MeteringLabel.id == label_id)
-        elif router_ids:
-            labels = (labels.join(metering_models.MeteringLabel.routers).
-                      filter(l3_models.Router.id.in_(router_ids)))
 
         return self._process_sync_metering_data(context, labels)
 

neutron/db/metering/metering_rpc.py

@@ -37,10 +37,11 @@ class MeteringRpcCallbacks(object):
         if not l3_plugin:
             return
 
+        metering_data = self.meter_plugin.get_sync_data_metering(context)
         host = kwargs.get('host')
         if not utils.is_extension_supported(
             l3_plugin, consts.L3_AGENT_SCHEDULER_EXT_ALIAS) or not host:
-            return self.meter_plugin.get_sync_data_metering(context)
+            return metering_data
         else:
             agents = l3_plugin.get_l3_agents(context, filters={'host': [host]})
             if not agents:
@@ -51,6 +52,8 @@ class MeteringRpcCallbacks(object):
             router_ids = [router['id'] for router in routers['routers']]
             if not router_ids:
                 return
-
-        return self.meter_plugin.get_sync_data_metering(context,
-                                                        router_ids=router_ids)
+            else:
+                return [
+                    router for router in metering_data
+                    if router['id'] in router_ids
+                ]

neutron/services/metering/agents/metering_agent.py

@@ -176,6 +176,14 @@ class MeteringAgent(MeteringPluginRpc, manager.Manager):
     @periodic_task.periodic_task(run_immediately=True)
     def _sync_routers_task(self, context):
         routers = self._get_sync_data_metering(self.context)
+
+        routers_on_agent = set(self.routers.keys())
+        routers_on_server = set(
+            [router['id'] for router in routers] if routers else [])
+        for router_id in routers_on_agent - routers_on_server:
+            del self.routers[router_id]
+            self._invoke_driver(context, router_id, 'remove_router')
+
         if not routers:
             return
         self._update_routers(context, routers)

neutron/services/metering/drivers/iptables/iptables_driver.py

@@ -343,6 +343,7 @@ class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver):
     @log_helpers.log_method_call
     def get_traffic_counters(self, context, routers):
         accs = {}
+        routers_to_reconfigure = set()
         for router in routers:
             rm = self.routers.get(router['id'])
             if not rm:
@@ -360,6 +361,7 @@ class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver):
                 except RuntimeError:
                     LOG.exception(_LE('Failed to get traffic counters, '
                                       'router: %s'), router)
+                    routers_to_reconfigure.add(router['id'])
                     continue
 
                 if not chain_acc:
@@ -372,4 +374,7 @@ class IptablesMeteringDriver(abstract_driver.MeteringAbstractDriver):
 
                 accs[label_id] = acc
 
+        for router_id in routers_to_reconfigure:
+            del self.routers[router_id]
+
         return accs

neutron/tests/unit/extensions/test_l3.py

@@ -335,7 +335,7 @@ class L3NatTestCaseMixin(object):
         data = {'router': {'tenant_id': tenant_id}}
         if name:
             data['router']['name'] = name
-        if admin_state_up:
+        if admin_state_up is not None:
             data['router']['admin_state_up'] = admin_state_up
         for arg in (('admin_state_up', 'tenant_id', 'availability_zone_hints')
                     + (arg_list or ())):

neutron/tests/unit/services/metering/test_metering_plugin.py

@@ -490,3 +490,53 @@ class TestMeteringPluginRpcFromL3Agent(
                     routers = [router['name'] for router in data]
 
                     self.assertEqual([], routers)
+
+    def test_get_sync_data_metering_with_unscheduled_router(self):
+        with self.subnet() as subnet:
+            s = subnet['subnet']
+            self._set_net_external(s['network_id'])
+            with self.router(
+                name='router1', tenant_id=self.tenant_id
+            ) as router1:
+                self._add_external_gateway_to_router(
+                    router1['router']['id'], s['network_id'])
+                with self.router(name='router2', tenant_id=self.tenant_id):
+                    with self.metering_label(tenant_id=self.tenant_id):
+                        callbacks = metering_rpc.MeteringRpcCallbacks(
+                            self.meter_plugin)
+                        data = callbacks.get_sync_data_metering(
+                            self.adminContext, host='agent1')
+                        self.assertEqual(
+                            set(['router1']), set([r['name'] for r in data]))
+
+                self._remove_external_gateway_from_router(
+                    router1['router']['id'], s['network_id'])
+
+    def test_get_sync_data_metering_with_inactive_router(self):
+        with self.subnet() as subnet:
+            s = subnet['subnet']
+            self._set_net_external(s['network_id'])
+            with self.router(
+                name='router1', tenant_id=self.tenant_id
+            ) as router1:
+                self._add_external_gateway_to_router(
+                    router1['router']['id'], s['network_id'])
+                with self.router(
+                    name='router2', tenant_id=self.tenant_id,
+                    admin_state_up=False
+                ) as router2:
+                    self._add_external_gateway_to_router(
+                        router2['router']['id'], s['network_id'])
+                    with self.metering_label(tenant_id=self.tenant_id):
+                        callbacks = metering_rpc.MeteringRpcCallbacks(
+                            self.meter_plugin)
+                        data = callbacks.get_sync_data_metering(
+                            self.adminContext, host='agent1')
+                        self.assertEqual(
+                            set(['router1']), set([r['name'] for r in data]))
+
+                    self._remove_external_gateway_from_router(
+                        router2['router']['id'], s['network_id'])
+
+                self._remove_external_gateway_from_router(
+                    router1['router']['id'], s['network_id'])