From 82f2a21d1c9e27999d3fd7006a7ecf961039a370 Mon Sep 17 00:00:00 2001 From: yatinkarel Date: Mon, 26 Jun 2023 21:17:48 +0530 Subject: [PATCH] [DHCP agent] Add route to OVN metadata port if exists When DHCP agent is deployed with ml2/ovn for baremetal ports, ovn metadata route is not added. This patch adds route via ovn metadata port if exists so baremetal nodes can fetch metadata. Closes-Bug: #1982569 Related-Bug: https://bugzilla.redhat.com/show_bug.cgi?id=2213862 Change-Id: I12e496d70bb6db707b317d0aeb6e4edd6c43571e --- neutron/agent/linux/dhcp.py | 62 ++++++++++++++----- neutron/tests/unit/agent/dhcp/test_agent.py | 43 ++++++++++++- neutron/tests/unit/agent/linux/test_dhcp.py | 31 ++++++++++ ...nt-ovn-metadata-port-33a654ccb9554c65.yaml | 9 +++ 4 files changed, 127 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/dhcp-agent-ovn-metadata-port-33a654ccb9554c65.yaml diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index dcbe8068b1f..e3b50888ffb 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -1208,6 +1208,14 @@ class Dnsmasq(DhcpLocalProcess): file_utils.replace_file(name, '\n'.join(options)) return name + def _get_ovn_metadata_port_ip(self, subnet): + m_ports = [port for port in self.network.ports if + self._is_ovn_metadata_port(port, self.network.id)] + if m_ports: + for fixed_ip in m_ports[0].fixed_ips: + if fixed_ip.subnet_id == subnet.id: + return fixed_ip.ip_address + def _generate_opts_per_subnet(self): options = [] subnets_without_nameservers = set() @@ -1261,23 +1269,33 @@ class Dnsmasq(DhcpLocalProcess): else: host_routes.append("%s,%s" % (hr.destination, hr.nexthop)) - # Add host routes for isolated network segments + # Determine metadata port route + if subnet.ip_version == constants.IP_VERSION_4: + metadata_route_ip = None + # NOTE: OVN metadata port IP is used in a case when the DHCP + # agent is deployed in the ML2/OVN enviroment where the native + # ovn-controller dhcp is disabled. The ovn metadata route + # takes precedence over native force_metadata and + # enable_isolated_metadata routes settings. + ovn_metadata_port_ip = self._get_ovn_metadata_port_ip(subnet) + if ovn_metadata_port_ip: + metadata_route_ip = ovn_metadata_port_ip - if ((self.conf.force_metadata or - (isolated_subnets[subnet.id] and - self.conf.enable_isolated_metadata)) and - subnet.ip_version == 4): - subnet_dhcp_ip = subnet_to_interface_ip.get(subnet.id) - if subnet_dhcp_ip: + elif (self.conf.force_metadata or + (isolated_subnets[subnet.id] and + self.conf.enable_isolated_metadata)): + subnet_dhcp_ip = subnet_to_interface_ip.get(subnet.id) + if subnet_dhcp_ip: + metadata_route_ip = subnet_dhcp_ip + + if not isolated_subnets[subnet.id] and gateway: + metadata_route_ip = gateway + + if metadata_route_ip: host_routes.append( - '%s,%s' % (constants.METADATA_CIDR, subnet_dhcp_ip) + '%s,%s' % (constants.METADATA_CIDR, metadata_route_ip) ) - elif not isolated_subnets[subnet.id] and gateway: - host_routes.append( - '%s,%s' % (constants.METADATA_CIDR, gateway) - ) - if subnet.ip_version == 4: for s in self._get_all_subnets(self.network): sub_segment_id = getattr(s, 'segment_id', None) if (s.ip_version == 4 and @@ -1442,13 +1460,21 @@ class Dnsmasq(DhcpLocalProcess): return True return False + @staticmethod + def _is_ovn_metadata_port(port, network_id): + return (port.device_id == 'ovnmeta-' + network_id and + port.device_owner == constants.DEVICE_OWNER_DISTRIBUTED) + @classmethod def should_enable_metadata(cls, conf, network): """Determine whether the metadata proxy is needed for a network - This method returns True for truly isolated networks (ie: not attached - to a router) when enable_isolated_metadata is True, or for all the - networks when the force_metadata flags is True. + If the given network contains a ovn metadata port then this method + assumes that the ovn metadata service is in use and this metadata + service is not required, method returns False. For other cases this + method returns True for truly isolated networks (ie: not attached to a + router) when enable_isolated_metadata is True, or for all the networks + when the force_metadata flags is True. This method also returns True when enable_metadata_network is True, and the network passed as a parameter has a subnet in the link-local @@ -1457,6 +1483,10 @@ class Dnsmasq(DhcpLocalProcess): providing access to the metadata service via logical routers built with 3rd party backends. """ + for port in network.ports: + if cls._is_ovn_metadata_port(port, network.id): + return False + all_subnets = cls._get_all_subnets(network) dhcp_subnets = [s for s in all_subnets if s.enable_dhcp] if not dhcp_subnets: diff --git a/neutron/tests/unit/agent/dhcp/test_agent.py b/neutron/tests/unit/agent/dhcp/test_agent.py index 795a5ee3332..f76a213a7ec 100644 --- a/neutron/tests/unit/agent/dhcp/test_agent.py +++ b/neutron/tests/unit/agent/dhcp/test_agent.py @@ -152,10 +152,27 @@ fake_port_subnet_2 = dhcp.DictModel( fake_ipv6_port = dhcp.DictModel(id='12345678-1234-aaaa-123456789000', device_owner='', + device_id='', mac_address='aa:bb:cc:dd:ee:99', network_id=FAKE_NETWORK_UUID, fixed_ips=[fake_fixed_ipv6]) +fake_ovn_port = dhcp.DictModel(id='12345678-1234-aaaa-123456789000', + device_owner='', + device_id='', + mac_address='aa:bb:cc:dd:ee:98', + network_id=FAKE_NETWORK_UUID, + fixed_ips=[fake_fixed_ip2]) + +fake_ovn_metadata_port = dhcp.DictModel(id='12345678-1234-aaaa-123456789000', + device_owner=const. + DEVICE_OWNER_DISTRIBUTED, + device_id='ovnmeta-{}'.format( + FAKE_NETWORK_UUID), + mac_address='aa:bb:cc:dd:ee:99', + network_id=FAKE_NETWORK_UUID, + fixed_ips=[fake_fixed_ip1]) + fake_meta_port = dhcp.DictModel(id='12345678-1234-aaaa-1234567890ab', mac_address='aa:bb:cc:dd:ee:ff', network_id=FAKE_NETWORK_UUID, @@ -191,6 +208,12 @@ fake_network_ipv6 = dhcp.NetModel(id=FAKE_NETWORK_UUID, subnets=[fake_ipv6_subnet], ports=[fake_ipv6_port]) +fake_ovn_network = dhcp.NetModel(id=FAKE_NETWORK_UUID, + project_id=FAKE_PROJECT_ID, + admin_state_up=True, + subnets=[fake_ipv6_subnet], + ports=[fake_ovn_metadata_port, fake_ovn_port]) + fake_network_ipv6_ipv4 = dhcp.NetModel( id=FAKE_NETWORK_UUID, project_id=FAKE_PROJECT_ID, @@ -803,7 +826,7 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): default_cmd_callback=mock.ANY) def _enable_dhcp_helper(self, network, enable_isolated_metadata=False, - is_isolated_network=False): + is_isolated_network=False, is_ovn_network=False): self.dhcp._process_monitor = mock.Mock() if enable_isolated_metadata: cfg.CONF.set_override('enable_isolated_metadata', True) @@ -813,7 +836,8 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): mock.call.get_network_info(network.id)]) self.call_driver.assert_called_once_with('enable', network) self.cache.assert_has_calls([mock.call.put(network)]) - if is_isolated_network and enable_isolated_metadata: + if (is_isolated_network and enable_isolated_metadata and not + is_ovn_network): self.external_process.assert_has_calls([ self._process_manager_constructor_call(), mock.call().enable()], any_order=True) @@ -858,6 +882,21 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): enable_isolated_metadata=True, is_isolated_network=False) + def test_enable_dhcp_helper_enable_metadata_ovn_network(self): + # Metadata should not be enabled when the dhcp agent is used + # in ML2/OVN where the ovn metadata agent is responsible for the + # metadata service. + self._enable_dhcp_helper(fake_ovn_network, is_ovn_network=True) + + def test_enable_dhcp_helper_ovn_network_with_enable_isolated_metadata( + self): + # Metadata should not be enabled when the dhcp agent is used + # in ML2/OVN where the ovn metadata agent is responsible for the + # metadata service. Even if the enable_isolated_metadata is enabled + self._enable_dhcp_helper(fake_ovn_network, + enable_isolated_metadata=True, + is_ovn_network=True) + def test_enable_dhcp_helper_enable_metadata_empty_network(self): self._enable_dhcp_helper(empty_network, enable_isolated_metadata=True, diff --git a/neutron/tests/unit/agent/linux/test_dhcp.py b/neutron/tests/unit/agent/linux/test_dhcp.py index 1ecbc3d7e4d..df84e93ac62 100644 --- a/neutron/tests/unit/agent/linux/test_dhcp.py +++ b/neutron/tests/unit/agent/linux/test_dhcp.py @@ -90,6 +90,19 @@ class FakeDhcpPort(object): self.extra_dhcp_opts = [] +class FakeOvnMetadataPort(object): + def __init__(self): + self.id = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaa' + self.admin_state_up = True + self.device_owner = constants.DEVICE_OWNER_DISTRIBUTED + self.fixed_ips = [ + FakeIPAllocation('192.168.0.10', + 'dddddddd-dddd-dddd-dddd-dddddddddddd')] + self.mac_address = '00:00:80:aa:bb:ee' + self.device_id = 'ovnmeta-aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' + self.extra_dhcp_opts = [] + + class FakeReservedPort(object): def __init__(self, id='reserved-aaaa-aaaa-aaaa-aaaaaaaaaaa'): self.admin_state_up = True @@ -768,6 +781,14 @@ class FakeNetworkDhcpPort(object): self.namespace = 'qdhcp-ns' +class FakeNetworkDhcpandOvnMetadataPort(object): + def __init__(self): + self.id = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' + self.subnets = [FakeV4Subnet()] + self.ports = [FakePort1(), FakeDhcpPort(), FakeOvnMetadataPort()] + self.namespace = 'qdhcp-ns' + + class FakeDualNetworkGatewayRoute(object): def __init__(self): self.id = 'cccccccc-cccc-cccc-cccc-cccccccccccc' @@ -3129,6 +3150,10 @@ class TestDnsmasq(TestBase): self.assertFalse(dhcp.Dnsmasq.has_metadata_subnet( [FakeV4Subnet()])) + def test_should_enable_metadata_ovn_metadata_port_returns_false(self): + self.assertFalse(dhcp.Dnsmasq.should_enable_metadata( + self.conf, FakeNetworkDhcpandOvnMetadataPort())) + def test_should_enable_metadata_isolated_network_returns_true(self): self.assertTrue(dhcp.Dnsmasq.should_enable_metadata( self.conf, FakeV4NetworkNoRouter())) @@ -3177,6 +3202,12 @@ class TestDnsmasq(TestBase): 'force_metadata': False} self._test__generate_opts_per_subnet_helper(config, False) + def test__generate_opts_per_subnet_with_metadata_port(self): + config = {'enable_isolated_metadata': False, + 'force_metadata': False} + self._test__generate_opts_per_subnet_helper(config, True, + network_class=FakeNetworkDhcpandOvnMetadataPort) + def test__generate_opts_per_subnet_isolated_metadata_with_router(self): config = {'enable_isolated_metadata': True, 'force_metadata': False} diff --git a/releasenotes/notes/dhcp-agent-ovn-metadata-port-33a654ccb9554c65.yaml b/releasenotes/notes/dhcp-agent-ovn-metadata-port-33a654ccb9554c65.yaml new file mode 100644 index 00000000000..49f73c68eab --- /dev/null +++ b/releasenotes/notes/dhcp-agent-ovn-metadata-port-33a654ccb9554c65.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixed the scenario where the DHCP agent is deployed in conjunction with + the OVN metadata agent in order to serve metadata for baremetal nodes. + In this scenario, the DHCP agent would not set the route needed for the + OVN metadata agent service resulting in baremetal nodes not being able + to query the metadata service. For more information see + `bug 1982569 `_.