From e7f85abae6a46a115582b80d1909e3565d859e9b Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Mon, 15 May 2023 12:29:42 -0400 Subject: [PATCH] Start metadata proxy even if IPv6 DAD fails A recent change suppressed the IPv6 DAD failure and removed the address when multiple DHCP agents were configured on the same network, https://review.opendev.org/c/openstack/neutron/+/880957 But it also changed the behavior to not enable IPv4 metadata in this case. Restore the old behavior by not returning early in the DAD failure case. The callback that builds the config file was moved until after the address was bound to make the two steps more obvious. Conflicts: neutron/tests/unit/agent/metadata/test_driver.py Related-bug: #1953165 Change-Id: I8436c6c9da9a2533ca27ff7312f5b2c7ea41e94f (cherry picked from commit 846003c4379124de6ffb3628ef1feb12a62a9cfa) --- neutron/agent/metadata/driver.py | 21 ++++++----- .../tests/unit/agent/metadata/test_driver.py | 37 ++++++++++--------- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/neutron/agent/metadata/driver.py b/neutron/agent/metadata/driver.py index a4a62444e23..7d1b45370b3 100644 --- a/neutron/agent/metadata/driver.py +++ b/neutron/agent/metadata/driver.py @@ -230,14 +230,6 @@ class MetadataDriver(object): bind_address="0.0.0.0", network_id=None, router_id=None, bind_address_v6=None, bind_interface=None): - uuid = network_id or router_id - callback = cls._get_metadata_proxy_callback( - bind_address, port, conf, - network_id=network_id, router_id=router_id, - bind_address_v6=bind_address_v6, bind_interface=bind_interface) - pm = cls._get_metadata_proxy_process_manager(uuid, conf, - ns_name=ns_name, - callback=callback) if bind_interface is not None and bind_address_v6 is not None: # HAProxy cannot bind() until IPv6 Duplicate Address Detection # completes. We must wait until the address leaves its 'tentative' @@ -265,7 +257,18 @@ class MetadataDriver(object): except Exception as exc: # do not re-raise a delete failure, just log LOG.info('Address deletion failure: %s', str(exc)) - return + + # Do not use the address or interface when DAD fails + bind_address_v6 = bind_interface = None + + uuid = network_id or router_id + callback = cls._get_metadata_proxy_callback( + bind_address, port, conf, + network_id=network_id, router_id=router_id, + bind_address_v6=bind_address_v6, bind_interface=bind_interface) + pm = cls._get_metadata_proxy_process_manager(uuid, conf, + ns_name=ns_name, + callback=callback) pm.enable() monitor.register(uuid, METADATA_SERVICE_NAME, pm) cls.monitors[router_id] = pm diff --git a/neutron/tests/unit/agent/metadata/test_driver.py b/neutron/tests/unit/agent/metadata/test_driver.py index e3b0b8ef6e3..605e50bdced 100644 --- a/neutron/tests/unit/agent/metadata/test_driver.py +++ b/neutron/tests/unit/agent/metadata/test_driver.py @@ -179,9 +179,12 @@ class TestMetadataDriverProcess(base.BaseTestCase): "%s.conf" % router_id) mock_open = self.useFixture( lib_fixtures.OpenFixture(cfg_file)).mock_open + bind_v6_line = 'bind %s:%s interface %s' % ( + self.METADATA_DEFAULT_IPV6, self.METADATA_PORT, 'fake-if') if dad_failed: mock_wait.side_effect = ip_lib.DADFailed( - address=self.METADATA_DEFAULT_IP, reason='DAD failed') + address=self.METADATA_DEFAULT_IPV6, reason='DAD failed') + bind_v6_line = '' else: mock_wait.return_value = True agent.metadata_driver.spawn_monitored_metadata_proxy( @@ -200,8 +203,6 @@ class TestMetadataDriverProcess(base.BaseTestCase): log_tag = ("haproxy-" + metadata_driver.METADATA_SERVICE_NAME + "-" + router_id) - bind_v6_line = 'bind %s:%s interface %s' % ( - self.METADATA_DEFAULT_IPV6, self.METADATA_PORT, 'fake-if') cfg_contents = metadata_driver._HAPROXY_CONFIG_TEMPLATE % { 'user': self.EUNAME, 'group': self.EGNAME, @@ -217,27 +218,27 @@ class TestMetadataDriverProcess(base.BaseTestCase): 'bind_v6_line': bind_v6_line} if dad_failed: - agent.process_monitor.register.assert_not_called() mock_del.assert_called_once_with(self.METADATA_DEFAULT_IPV6, 'fake-if', namespace=router_ns) else: - mock_open.assert_has_calls([ - mock.call(cfg_file, 'w'), - mock.call().write(cfg_contents)], any_order=True) - - env = {ep.PROCESS_TAG: service_name + '-' + router_id} - ip_mock.assert_has_calls([ - mock.call(namespace=router_ns), - mock.call().netns.execute(netns_execute_args, addl_env=env, - run_as_root=True) - ]) - - agent.process_monitor.register.assert_called_once_with( - router_id, metadata_driver.METADATA_SERVICE_NAME, - mock.ANY) mock_del.assert_not_called() + mock_open.assert_has_calls([ + mock.call(cfg_file, 'w'), + mock.call().write(cfg_contents)], any_order=True) + + env = {ep.PROCESS_TAG: service_name + '-' + router_id} + ip_mock.assert_has_calls([ + mock.call(namespace=router_ns), + mock.call().netns.execute(netns_execute_args, addl_env=env, + run_as_root=True) + ]) + + agent.process_monitor.register.assert_called_once_with( + router_id, metadata_driver.METADATA_SERVICE_NAME, + mock.ANY) + def test_spawn_metadata_proxy(self): self._test_spawn_metadata_proxy()