From f6ec99d1b8d90010ed1bf445d6a127c900ab1892 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Mon, 17 Jun 2024 13:53:04 +0100 Subject: [PATCH] [OVN] Sanitize the classless-static-route DHCP option This patch ensures that the "classless-static-route" is wrapped in {} as expected by OVN and also merges the default routes with the user inputted ones so everything works as expected. Closes-Bug: #2069625 Change-Id: I302a872161c55df447a05b31d99c702537502a2f Signed-off-by: Lucas Alvares Gomes (cherry picked from commit ceee380a1835d706579aa0f3597ad9e0ce1a37ee) --- neutron/common/ovn/constants.py | 4 ++ neutron/common/ovn/utils.py | 7 ++++ .../ovn/mech_driver/ovsdb/ovn_client.py | 18 +++++++++ neutron/tests/unit/common/ovn/test_utils.py | 14 +++++++ .../ovn/mech_driver/test_mech_driver.py | 38 +++++++++++++++++++ 5 files changed, 81 insertions(+) diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index da01809ed9d..7f3e033cb99 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -216,6 +216,10 @@ OVN_STR_TYPE_DHCP_OPTS = [ 'wpad', 'tftp_server'] +OVN_MAP_TYPE_DHCP_OPTS = [ + 'classless_static_route', +] + # Special option for disabling DHCP via extra DHCP options DHCP_DISABLED_OPT = 'dhcp_disabled' diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 35aa9a3d068..5958414eaf9 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -250,6 +250,10 @@ def is_dhcp_option_quoted(opt_value): return opt_value.startswith('"') and opt_value.endswith('"') +def is_dhcp_option_a_map(opt_value): + return opt_value.startswith('{') and opt_value.endswith('}') + + def get_lsp_dhcp_opts(port, ip_version): # Get dhcp options from Neutron port, for setting DHCP_Options row # in OVN. @@ -288,6 +292,9 @@ def get_lsp_dhcp_opts(port, ip_version): if (opt in constants.OVN_STR_TYPE_DHCP_OPTS and not is_dhcp_option_quoted(edo['opt_value'])): edo['opt_value'] = '"%s"' % edo['opt_value'] + elif (opt in constants.OVN_MAP_TYPE_DHCP_OPTS and + not is_dhcp_option_a_map(edo['opt_value'])): + edo['opt_value'] = '{%s}' % edo['opt_value'] lsp_dhcp_opts[opt] = edo['opt_value'] return (lsp_dhcp_disabled, lsp_dhcp_opts) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index d906c234dee..8ef4028211d 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -191,6 +191,18 @@ class OVNClient(object): return opts return get_opts[0] + def _merge_map_dhcp_option(self, opt, port_opts, subnet_opts): + """Merge a port and subnet map DHCP option. + + If a DHCP option exists in both port and subnet, the port + should inherit the values from the subnet. + """ + port_opt = port_opts[opt] + subnet_opt = subnet_opts.get(opt) + if not subnet_opt: + return port_opt + return '{%s, %s}' % (subnet_opt[1:-1], port_opt[1:-1]) + def _get_port_dhcp_options(self, port, ip_version): """Return dhcp options for port. @@ -223,6 +235,12 @@ class OVNClient(object): if not lsp_dhcp_opts: return subnet_dhcp_options + # Check for map DHCP options + for opt in ovn_const.OVN_MAP_TYPE_DHCP_OPTS: + if opt in lsp_dhcp_opts: + lsp_dhcp_opts[opt] = self._merge_map_dhcp_option( + opt, lsp_dhcp_opts, subnet_dhcp_options['options']) + # This port has extra DHCP options defined, so we will create a new # row in DHCP_Options table for it. subnet_dhcp_options['options'].update(lsp_dhcp_opts) diff --git a/neutron/tests/unit/common/ovn/test_utils.py b/neutron/tests/unit/common/ovn/test_utils.py index 3d720ade2aa..7e9e1d1ff11 100644 --- a/neutron/tests/unit/common/ovn/test_utils.py +++ b/neutron/tests/unit/common/ovn/test_utils.py @@ -490,6 +490,20 @@ class TestDHCPUtils(base.BaseTestCase): expected_options = {'domain_search_list': '"openstack.org,ovn.org"'} self.assertEqual(expected_options, options) + def test_get_lsp_dhcp_opts_sanitize_map(self): + opt = {'opt_name': 'classless-static-route', + 'opt_value': '128.128.128.128/32,22.2.0.2', + 'ip_version': 4} + port = {portbindings.VNIC_TYPE: portbindings.VNIC_NORMAL, + edo_ext.EXTRADHCPOPTS: [opt]} + dhcp_disabled, options = utils.get_lsp_dhcp_opts(port, 4) + self.assertFalse(dhcp_disabled) + # Assert option got translated to "classless_static_route" and + # the value is a map (wrapped with {}) + expected_options = { + 'classless_static_route': '{128.128.128.128/32,22.2.0.2}'} + self.assertEqual(expected_options, options) + class TestGetDhcpDnsServers(base.BaseTestCase): diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 58b7d3506d7..0423a5b4f82 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -4028,6 +4028,44 @@ class TestOVNMechanismDriverDHCPOptions(OVNMechanismDriverTestCase): self._test__get_subnet_dhcp_options_for_port(ip_version=6, enable_dhcp=False) + def test_get_port_dhcp_options_classless_static_route(self): + port = { + 'id': 'foo-port', + 'device_owner': 'compute:None', + 'fixed_ips': [{'subnet_id': 'foo-subnet', + 'ip_address': '10.0.0.11'}], + 'extra_dhcp_opts': [ + {'ip_version': 4, 'opt_name': 'classless-static-route', + 'opt_value': '128.128.128.128/32,22.2.0.2'}]} + + self.mech_driver._ovn_client._get_subnet_dhcp_options_for_port = ( + mock.Mock( + return_value=({ + 'cidr': '10.0.0.0/24', + 'external_ids': {'subnet_id': 'foo-subnet'}, + 'options': { + 'classless_static_route': + '{169.254.169.254/32,10.0.0.2}'}, + 'uuid': 'foo-uuid'}))) + + # Expect both the subnet and port classless_static_route + # to be merged + expected_routes = ('{169.254.169.254/32,10.0.0.2, ' + '128.128.128.128/32,22.2.0.2}') + expected_dhcp_options = { + 'cidr': '10.0.0.0/24', + 'external_ids': {'subnet_id': 'foo-subnet', + 'port_id': 'foo-port'}, + 'options': {'classless_static_route': expected_routes} + } + + self.mech_driver.nb_ovn.add_dhcp_options.return_value = 'foo-val' + dhcp_options = self.mech_driver._ovn_client._get_port_dhcp_options( + port, 4) + self.assertEqual({'cmd': 'foo-val'}, dhcp_options) + self.mech_driver.nb_ovn.add_dhcp_options.assert_called_once_with( + 'foo-subnet', port_id='foo-port', **expected_dhcp_options) + class TestOVNMechanismDriverSecurityGroup(MechDriverSetupBase, test_security_group.Ml2SecurityGroupsTestCase):