[DVR] Don't populate unbound ports in router's ARP cache

When user is using keepalived on their instances, he often creates
additional port in Neutron to allocate some IP address which will
be then used as VIP in keepalived and will be configured in
allowed_address_pair of other ports plugged to instances with
keepalived.
This is e.g. Octavia's use case.

This together with DVR caused problems with connectivity to such VIP
as it was populated in router's arp cache with MAC address from
Neutron db.

As this port isn't bound, it is only Neutron db entry so there is no
need to set it in arp cache of the router.
This patch is doing exactly that to filter such "unbound" and
"binding_failed" ports from the list.

Change-Id: Ia885ce00dbb5f2968859e8d0850bc511016f0846
Closes-Bug: #1869887

neutron/db/l3_dvr_db.py

@@ -15,6 +15,7 @@ import collections
 
 from neutron_lib.api.definitions import l3 as l3_apidef
 from neutron_lib.api.definitions import portbindings
+from neutron_lib.api.definitions import portbindings_extended
 from neutron_lib.api import validators
 from neutron_lib.callbacks import events
 from neutron_lib.callbacks import exceptions
@@ -66,6 +67,18 @@ def is_admin_state_down_necessary():
     return _IS_ADMIN_STATE_DOWN_NECESSARY
 
 
+# TODO(slaweq): this should be moved to neutron_lib.plugins.utils module
+def is_port_bound(port):
+    active_binding = plugin_utils.get_port_binding_by_status_and_host(
+        port.get("port_bindings", []), const.ACTIVE)
+    if not active_binding:
+        LOG.warning("Binding for port %s was not found.", port)
+        return False
+    return active_binding[portbindings_extended.VIF_TYPE] not in [
+        portbindings.VIF_TYPE_UNBOUND,
+        portbindings.VIF_TYPE_BINDING_FAILED]
+
+
 @registry.has_registry_receivers
 class DVRResourceOperationHandler(object):
     """Contains callbacks for DVR operations.
@@ -1204,10 +1217,11 @@ class L3_NAT_with_dvr_db_mixin(_DVRAgentInterfaceMixin,
 
     def get_ports_under_dvr_connected_subnet(self, context, subnet_id):
         query = dvr_mac_db.get_ports_query_by_subnet_and_ip(context, subnet_id)
+        ports = [p for p in query.all() if is_port_bound(p)]
         return [
             self.l3plugin._core_plugin._make_port_dict(
                 port, process_extensions=False)
-            for port in query.all()
+            for port in ports
         ]
 
 

neutron/tests/unit/db/test_l3_dvr_db.py

@@ -1282,6 +1282,35 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
             self.assertTrue(
                 self.mixin.is_router_distributed(self.ctx, router_id))
 
+    @mock.patch.object(l3_dvr_db, "is_port_bound")
+    def test_get_ports_under_dvr_connected_subnet(self, is_port_bound_mock):
+        router_dict = {'name': 'test_router', 'admin_state_up': True,
+                       'distributed': True}
+        router = self._create_router(router_dict)
+        with self.network() as network,\
+                self.subnet(network=network) as subnet:
+            fake_bound_ports_ids = []
+
+            def fake_is_port_bound(port):
+                return port['id'] in fake_bound_ports_ids
+
+            is_port_bound_mock.side_effect = fake_is_port_bound
+
+            for _ in range(4):
+                port_res = self.create_port(
+                    network['network']['id'],
+                    {'fixed_ips': [{'subnet_id': subnet['subnet']['id']}]})
+                port_id = self.deserialize(self.fmt, port_res)['port']['id']
+                if len(fake_bound_ports_ids) < 2:
+                    fake_bound_ports_ids.append(port_id)
+
+            self.mixin.add_router_interface(self.ctx, router['id'],
+                {'subnet_id': subnet['subnet']['id']})
+            dvr_subnet_ports = self.mixin.get_ports_under_dvr_connected_subnet(
+                self.ctx, subnet['subnet']['id'])
+            dvr_subnet_ports_ids = [p['id'] for p in dvr_subnet_ports]
+            self.assertItemsEqual(fake_bound_ports_ids, dvr_subnet_ports_ids)
+
     @mock.patch.object(plugin_utils, 'can_port_be_bound_to_virtual_bridge',
                        return_value=True)
     def test__get_assoc_data_valid_vnic_type(self, *args):