From 09aaa520f00af4498610d91a64e8b37988168312 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Tue, 31 Mar 2020 05:33:06 +0200 Subject: [PATCH] [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. Conflicts: neutron/tests/unit/db/test_l3_dvr_db.py Change-Id: Ia885ce00dbb5f2968859e8d0850bc511016f0846 Closes-Bug: #1869887 (cherry picked from commit eb775458c6da57426703289c7b969caddb83d677) --- neutron/db/l3_dvr_db.py | 14 +++++++++++- neutron/tests/unit/db/test_l3_dvr_db.py | 29 +++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index 9de97d4371f..ee699cdcd5d 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -16,6 +16,7 @@ import collections import netaddr 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 @@ -56,6 +57,16 @@ LOG = logging.getLogger(__name__) l3_dvr_db.register_db_l3_dvr_opts() +# TODO(slaweq): this should be moved to neutron_lib.plugins.utils module +def is_port_bound(port): + if port.port_binding: + LOG.warning("Binding for port %s was not found.", port) + return False + return port.port_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. @@ -1239,10 +1250,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 ] diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index a1f854932d5..66311921f23 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -1070,6 +1070,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(l3_db, 'can_port_be_bound_to_virtual_bridge', return_value=True) def test__get_assoc_data_valid_vnic_type(self, *args):