From 367cdcd66521a569965fc59cbce431039dbeb3ac Mon Sep 17 00:00:00 2001 From: Mohammed Naser Date: Thu, 25 Feb 2021 16:53:55 -0500 Subject: [PATCH] Add both IPv4 and IPv6 DHCP options if interface has both It is possible that an interface has both IPv4 and IPv6 addresses, primarily when using SLAAC with OpenStack Neutron. When this is the case, it is very likely that the first fixed IP would be a SLAAC assigned port and the second IP is the IPv4 address. In an environment where you are looking to boot via IPv4, no DHCPv6 infrastructure exists as IPv6 connectivity is provided via SLAAC, you would not be able to use this network to boot off of. This patch instead grabs all the fixed IP addresses, then inserts the options that match the IP versions which are attached to the interface, potentially resulting in both IPv4 and IPv6 options being included (though the IPv6 ones would be largely omitted). In environments where only IPv4 or IPv6 is in use on the port, it will still only insert the options for those specific IP versions. Story #2008660 Task #41933 Change-Id: I52e4ee022b17cb7f007534cb368136567b139a34 --- ironic/dhcp/neutron.py | 40 ++++++------- ironic/tests/unit/dhcp/test_neutron.py | 60 +++++++++++++++++++ ...dual-stack-dhcp-opts-6dc18ae10aeb599a.yaml | 5 ++ 3 files changed, 85 insertions(+), 20 deletions(-) create mode 100644 releasenotes/notes/add-dual-stack-dhcp-opts-6dc18ae10aeb599a.yaml diff --git a/ironic/dhcp/neutron.py b/ironic/dhcp/neutron.py index 13317cd2eb..18ccae05c2 100644 --- a/ironic/dhcp/neutron.py +++ b/ironic/dhcp/neutron.py @@ -64,28 +64,28 @@ class NeutronDHCPApi(base.BaseDHCP): try: neutron_client = neutron.get_client(token=token, context=context) - fip = None + fips = [] port = neutron_client.get_port(port_id) - try: - if port: - # TODO(TheJulia): We need to retool this down the - # road so that we handle ports and allow preferences - # for multi-address ports with different IP versions - # and enable operators to possibly select preferences - # for provisionioning operations. - # This is compounded by v6 mainly only being available - # with UEFI machines, so the support matrix also gets - # a little "weird". - # Ideally, we should work on this in Victoria. - fip = port.get('fixed_ips')[0] - except (TypeError, IndexError): - fip = None + if port: + # TODO(TheJulia): We need to retool this down the + # road so that we handle ports and allow preferences + # for multi-address ports with different IP versions + # and enable operators to possibly select preferences + # for provisionioning operations. + # This is compounded by v6 mainly only being available + # with UEFI machines, so the support matrix also gets + # a little "weird". + # Ideally, we should work on this in Victoria. + fips = port.get('fixed_ips') + update_opts = [] - if fip: - ip_version = ipaddress.ip_address(fip['ip_address']).version - for option in dhcp_options: - if option.get('ip_version', 4) == ip_version: - update_opts.append(option) + if len(fips) != 0: + for fip in fips: + ip_version = \ + ipaddress.ip_address(fip['ip_address']).version + for option in dhcp_options: + if option.get('ip_version', 4) == ip_version: + update_opts.append(option) else: LOG.error('Requested to update port for port %s, ' 'however port lacks an IP address.', port_id) diff --git a/ironic/tests/unit/dhcp/test_neutron.py b/ironic/tests/unit/dhcp/test_neutron.py index c0921874e9..97c552fbe8 100644 --- a/ironic/tests/unit/dhcp/test_neutron.py +++ b/ironic/tests/unit/dhcp/test_neutron.py @@ -119,6 +119,66 @@ class TestNeutron(db_base.DbTestCase): update_mock.assert_called_once_with( task.context, port_id, expected) + @mock.patch('ironic.common.neutron.get_client', autospec=True) + @mock.patch('ironic.common.neutron.update_neutron_port', autospec=True) + def test_update_port_dhcp_opts_v4_and_v6(self, update_mock, client_mock): + opts = [{'opt_name': 'bootfile-name', + 'opt_value': 'pxelinux.0', + 'ip_version': 4}, + {'opt_name': 'tftp-server', + 'opt_value': '1.1.1.1', + 'ip_version': 4}, + {'opt_name': 'server-ip-address', + 'opt_value': '1.1.1.1', + 'ip_version': 4}, + {'opt_name': 'bootfile-url', + 'opt_value': 'tftp://::1/file.name', + 'ip_version': 6}] + port_id = 'fake-port-id' + expected = { + 'extra_dhcp_opts': [ + { + 'opt_name': 'bootfile-name', + 'opt_value': 'pxelinux.0', + 'ip_version': 4 + }, + { + 'opt_name': 'tftp-server', + 'opt_value': '1.1.1.1', + 'ip_version': 4 + }, + { + 'opt_name': 'server-ip-address', + 'opt_value': '1.1.1.1', + 'ip_version': 4 + }, + { + 'opt_name': 'bootfile-url', + 'opt_value': 'tftp://::1/file.name', + 'ip_version': 6 + } + ] + } + port_data = { + "id": port_id, + "fixed_ips": [ + { + "ip_address": "192.168.1.3", + }, + { + "ip_address": "2001:db8::201", + } + ], + } + client_mock.return_value.get_port.return_value = port_data + + api = dhcp_factory.DHCPFactory() + with task_manager.acquire(self.context, self.node.uuid) as task: + api.provider.update_port_dhcp_opts(port_id, opts, + context=task.context) + update_mock.assert_called_once_with( + task.context, port_id, expected) + @mock.patch('ironic.common.neutron.get_client', autospec=True) @mock.patch('ironic.common.neutron.update_neutron_port', autospec=True) def test_update_port_dhcp_opts_with_exception(self, update_mock, diff --git a/releasenotes/notes/add-dual-stack-dhcp-opts-6dc18ae10aeb599a.yaml b/releasenotes/notes/add-dual-stack-dhcp-opts-6dc18ae10aeb599a.yaml new file mode 100644 index 0000000000..d5c65f9e1c --- /dev/null +++ b/releasenotes/notes/add-dual-stack-dhcp-opts-6dc18ae10aeb599a.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - When using the Neutron DHCP driver, Ironic would only use the first fixed + IP address to determine what IP versions are use on the port. Now, it + checks for all the IP addresses and adds DHCP options for all IP versions.