From b0a93df47637958fc3fc172f15e8593d6e9b181a Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 27 Aug 2019 09:43:26 +0000 Subject: [PATCH] Update DHCP port information during setup When setting up the DHCP agent of a network, the DHCP namespace external port is configured. If this port already exists and the fixed IP addresses are correctly configured (in the DHCP subnets range), the port is used as is. Sometimes, because of 1627480 or 1841636, the port information is not correctly retrieved. This patch does not solve it but mitigates the process of resynchronizing the network DHCP. If the stored DHCP port does not have the correct information, the agent calls the RPC plugin to retrieve from the server the DHCP port updated information, including the fixed IP address and the subnets. Change-Id: Iff40e7bba645ee12c2001d7ce735a36e0ddc81e9 Related-Bug: #1627480 Related-Bug: #1841636 --- neutron/agent/dhcp/agent.py | 9 +++- neutron/agent/linux/dhcp.py | 46 +++++++++++++++------ neutron/api/rpc/handlers/dhcp_rpc.py | 10 ++++- neutron/tests/unit/agent/dhcp/test_agent.py | 17 +++++++- 4 files changed, 66 insertions(+), 16 deletions(-) diff --git a/neutron/agent/dhcp/agent.py b/neutron/agent/dhcp/agent.py index 7ded9ed4b9d..7fe073a1703 100644 --- a/neutron/agent/dhcp/agent.py +++ b/neutron/agent/dhcp/agent.py @@ -718,7 +718,7 @@ class DhcpPluginApi(object): and update_dhcp_port methods. 1.5 - Added dhcp_ready_on_ports 1.7 - Added get_networks - + 1.8 - Added get_dhcp_port """ def __init__(self, topic, host): @@ -776,6 +776,13 @@ class DhcpPluginApi(object): network_id=network_id, device_id=device_id, host=self.host) + def get_dhcp_port(self, port_id): + """Make a remote process call to retrieve the dhcp port.""" + cctxt = self.client.prepare(version='1.8') + port = cctxt.call(self.context, 'get_dhcp_port', port_id=port_id) + if port: + return dhcp.DictModel(port) + def dhcp_ready_on_ports(self, port_ids): """Notify the server that DHCP is configured for the port.""" cctxt = self.client.prepare(version='1.5') diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index e663bec5079..a53b34ae3c1 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -1391,6 +1391,37 @@ class DeviceManager(object): fixed_ips=unique_ip_subnets) return self.plugin.create_dhcp_port({'port': port_dict}) + def _check_dhcp_port_subnet(self, dhcp_port, dhcp_subnets, network): + """Check if DHCP port IPs are in the range of the DHCP subnets + + FIXME(kevinbenton): ensure we have the IPs we actually need. + can be removed once bug/1627480 is fixed + """ + if self.driver.use_gateway_ips: + return + + expected = set(dhcp_subnets) + actual = {fip.subnet_id for fip in dhcp_port.fixed_ips} + missing = expected - actual + if not missing: + return + + LOG.debug('Requested DHCP port with IPs on subnets %(expected)s ' + 'but only got IPs on subnets %(actual)s.', + {'expected': expected, 'actual': actual}) + updated_dhcp_port = self.plugin.get_dhcp_port(dhcp_port.id) + actual = {fip.subnet_id for fip in updated_dhcp_port.fixed_ips} + missing = expected - actual + if missing: + raise exceptions.SubnetMismatchForPort( + port_id=updated_dhcp_port.id, subnet_id=list(missing)[0]) + + self._update_dhcp_port(network, updated_dhcp_port) + LOG.debug('Previous DHCP port information: %(dhcp_port)s. Updated ' + 'DHCP port information: %(updated_dhcp_port)s.', + {'dhcp_port': dhcp_port, + 'updated_dhcp_port': updated_dhcp_port}) + def setup_dhcp_port(self, network): """Create/update DHCP port for the host if needed and return port.""" @@ -1416,19 +1447,8 @@ class DeviceManager(object): else: raise exceptions.Conflict() - # FIXME(kevinbenton): ensure we have the IPs we actually need. - # can be removed once bug/1627480 is fixed - if not self.driver.use_gateway_ips: - expected = set(dhcp_subnets) - actual = {fip.subnet_id for fip in dhcp_port.fixed_ips} - missing = expected - actual - if missing: - LOG.debug("Requested DHCP port with IPs on subnets " - "%(expected)s but only got IPs on subnets " - "%(actual)s.", {'expected': expected, - 'actual': actual}) - raise exceptions.SubnetMismatchForPort( - port_id=dhcp_port.id, subnet_id=list(missing)[0]) + self._check_dhcp_port_subnet(dhcp_port, dhcp_subnets, network) + # Convert subnet_id to subnet dict fixed_ips = [dict(subnet_id=fixed_ip.subnet_id, ip_address=fixed_ip.ip_address, diff --git a/neutron/api/rpc/handlers/dhcp_rpc.py b/neutron/api/rpc/handlers/dhcp_rpc.py index c109d44cbb4..232d6f71eb5 100644 --- a/neutron/api/rpc/handlers/dhcp_rpc.py +++ b/neutron/api/rpc/handlers/dhcp_rpc.py @@ -71,10 +71,11 @@ class DhcpRpcCallback(object): # DHCP agent since Havana, so similar rationale for not bumping # the major version as above applies here too. # 1.7 - Add get_networks + # 1.8 - Add get_dhcp_port target = oslo_messaging.Target( namespace=constants.RPC_NAMESPACE_DHCP_PLUGIN, - version='1.7') + version='1.8') def _get_active_networks(self, context, **kwargs): """Retrieve and return a list of the active networks.""" @@ -313,6 +314,13 @@ class DhcpRpcCallback(object): '%(port_id)s which no longer exists.', {'host': host, 'port_id': port['id']}) + @db_api.retry_db_errors + def get_dhcp_port(self, context, **kwargs): + """Retrieve the DHCP port""" + port_id = kwargs.get('port_id') + plugin = directory.get_plugin() + return plugin.get_port(context, port_id) + @db_api.retry_db_errors def dhcp_ready_on_ports(self, context, port_ids): for port_id in port_ids: diff --git a/neutron/tests/unit/agent/dhcp/test_agent.py b/neutron/tests/unit/agent/dhcp/test_agent.py index 1af4a29946a..49e261e36c4 100644 --- a/neutron/tests/unit/agent/dhcp/test_agent.py +++ b/neutron/tests/unit/agent/dhcp/test_agent.py @@ -1906,7 +1906,21 @@ class TestDeviceManager(base.BaseTestCase): [{'subnet_id': fake_fixed_ip1.subnet_id}], 'device_id': mock.ANY}})]) - def test_create_dhcp_port_update_add_subnet_bug_1627480(self): + def test__check_dhcp_port_subnet(self): + # this can go away once bug/1627480 is fixed + plugin = mock.Mock() + fake_port_copy = copy.deepcopy(fake_port1) + fake_port_copy.fixed_ips = [fake_fixed_ip1, fake_fixed_ip_subnet2] + plugin.get_dhcp_port.return_value = fake_port_copy + dh = dhcp.DeviceManager(cfg.CONF, plugin) + fake_network_copy = copy.deepcopy(fake_network) + fake_network_copy.ports[0].device_id = dh.get_device_id(fake_network) + fake_network_copy.subnets[1].enable_dhcp = True + plugin.update_dhcp_port.return_value = fake_network.ports[0] + dh.setup_dhcp_port(fake_network_copy) + self.assertEqual(fake_port_copy, fake_network_copy.ports[0]) + + def test__check_dhcp_port_subnet_port_missing_subnet(self): # this can go away once bug/1627480 is fixed plugin = mock.Mock() dh = dhcp.DeviceManager(cfg.CONF, plugin) @@ -1914,6 +1928,7 @@ class TestDeviceManager(base.BaseTestCase): fake_network_copy.ports[0].device_id = dh.get_device_id(fake_network) fake_network_copy.subnets[1].enable_dhcp = True plugin.update_dhcp_port.return_value = fake_network.ports[0] + plugin.get_dhcp_port.return_value = fake_network_copy.ports[0] with testtools.ExpectedException(exceptions.SubnetMismatchForPort): dh.setup_dhcp_port(fake_network_copy)