From 2e34279ec3f4e93a7eca1e89b307fdbeb15b211e Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Fri, 4 May 2018 16:43:45 -0400 Subject: [PATCH] Fix lack of routes for neighbour IPv4 subnets According to [1], when a network contains more that one IPv4 subnet, they are returned in the 'classless-static-routes' DHCP option, regardless of whether DHCP is enabled for them or not. However, the get_active_networks_info() method used for synchronizing networks after the dhcp agent restarts filters subnets with "enable_dhcp=True", which differs from the get_network_info() method. This will block VM access to other VMs in the dhcp disabled subnets, even though they are in the same network. This is visible by looking at the "opts" file before and after a restart. Change the dhcp agent to ask for all subnets in its get_active_networks_info() RPC call by adding an enable_dhcp_filter argument to toggle the behavior, with the default being True to not break backwards compatibility. Based on https://review.openstack.org/#/c/352530/ by Quan Tian. [1] https://review.openstack.org/#/c/125043/ Change-Id: I11ca1d1a603d02587f3b8d4a5a52a96b0587d61f Closes-Bug: #1652654 --- neutron/agent/dhcp/agent.py | 7 ++++--- neutron/api/rpc/handlers/dhcp_rpc.py | 4 +++- .../unit/api/rpc/handlers/test_dhcp_rpc.py | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/neutron/agent/dhcp/agent.py b/neutron/agent/dhcp/agent.py index 790a60dee5a..5d7edc3043c 100644 --- a/neutron/agent/dhcp/agent.py +++ b/neutron/agent/dhcp/agent.py @@ -186,7 +186,8 @@ class DhcpAgent(manager.Manager): known_network_ids = set(self.cache.get_network_ids()) try: - active_networks = self.plugin_rpc.get_active_networks_info() + active_networks = self.plugin_rpc.get_active_networks_info( + enable_dhcp_filter=False) LOG.info('All active networks have been fetched through RPC.') active_network_ids = set(network.id for network in active_networks) for deleted_id in known_network_ids - active_network_ids: @@ -567,11 +568,11 @@ class DhcpPluginApi(object): # can be independently tracked server side. return context.get_admin_context_without_session() - def get_active_networks_info(self): + def get_active_networks_info(self, **kwargs): """Make a remote process call to retrieve all network info.""" cctxt = self.client.prepare(version='1.1') networks = cctxt.call(self.context, 'get_active_networks_info', - host=self.host) + host=self.host, **kwargs) return [dhcp.NetModel(n) for n in networks] def get_network_info(self, network_id): diff --git a/neutron/api/rpc/handlers/dhcp_rpc.py b/neutron/api/rpc/handlers/dhcp_rpc.py index 14269ace0e2..acffd213125 100644 --- a/neutron/api/rpc/handlers/dhcp_rpc.py +++ b/neutron/api/rpc/handlers/dhcp_rpc.py @@ -144,7 +144,9 @@ class DhcpRpcCallback(object): plugin = directory.get_plugin() filters = {'network_id': [network['id'] for network in networks]} ports = plugin.get_ports(context, filters=filters) - filters['enable_dhcp'] = [True] + # default is to filter subnets based on 'enable_dhcp' flag + if kwargs.get('enable_dhcp_filter', True): + filters['enable_dhcp'] = [True] # NOTE(kevinbenton): we sort these because the agent builds tags # based on position in the list and has to restart the process if # the order changes. diff --git a/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py b/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py index 3056f5b2e9b..946cd5ae0ac 100644 --- a/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py +++ b/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py @@ -97,6 +97,24 @@ class TestDhcpRpcCallback(base.BaseTestCase): 'ports': []}] self.assertEqual(expected, networks) + def _test_get_active_networks_info_enable_dhcp_filter(self, + enable_dhcp_filter): + plugin_retval = [{'id': 'a'}, {'id': 'b'}] + self.plugin.get_networks.return_value = plugin_retval + self.callbacks.get_active_networks_info(mock.Mock(), host='host', + enable_dhcp_filter=enable_dhcp_filter) + filters = {'network_id': ['a', 'b']} + if enable_dhcp_filter: + filters['enable_dhcp'] = [True] + self.plugin.get_subnets.assert_called_once_with(mock.ANY, + filters=filters) + + def test_get_active_networks_info_enable_dhcp_filter_false(self): + self._test_get_active_networks_info_enable_dhcp_filter(False) + + def test_get_active_networks_info_enable_dhcp_filter_true(self): + self._test_get_active_networks_info_enable_dhcp_filter(True) + def _test__port_action_with_failures(self, exc=None, action=None): port = { 'network_id': 'foo_network_id',