diff --git a/ironic/common/dhcp_factory.py b/ironic/common/dhcp_factory.py index aaff635df3..fa294cb463 100644 --- a/ironic/common/dhcp_factory.py +++ b/ironic/common/dhcp_factory.py @@ -72,12 +72,10 @@ class DHCPFactory(object): :: - [{'opt_name': 'bootfile-name', + [{'opt_name': '67', 'opt_value': 'pxelinux.0'}, - {'opt_name': 'server-ip-address', - 'opt_value': '123.123.123.456'}, - {'opt_name': 'tftp-server', - 'opt_value': '123.123.123.123'}] + {'opt_name': '66', + 'opt_value': '123.123.123.456'}] :param ports: A dict with keys 'ports' and 'portgroups' and dicts as values. Each dict has key/value pairs of the form diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index 1cb0b0d3d2..4bba8d124f 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -18,6 +18,7 @@ from oslo_utils import uuidutils from ironic.common import exception from ironic.common.i18n import _ from ironic.common import keystone +from ironic.common.pxe_utils import DHCP_CLIENT_ID from ironic.conf import CONF LOG = log.getLogger(__name__) @@ -237,7 +238,8 @@ def add_ports_to_network(task, network_uuid, security_groups=None): body['port']['binding:profile'] = binding_profile client_id = ironic_port.extra.get('client-id') if client_id: - client_id_opt = {'opt_name': 'client-id', 'opt_value': client_id} + client_id_opt = {'opt_name': DHCP_CLIENT_ID, + 'opt_value': client_id} extra_dhcp_opts = body['port'].get('extra_dhcp_opts', []) extra_dhcp_opts.append(client_id_opt) body['port']['extra_dhcp_opts'] = extra_dhcp_opts diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py index 0ed9a21ff7..806a00cf49 100644 --- a/ironic/common/pxe_utils.py +++ b/ironic/common/pxe_utils.py @@ -33,6 +33,13 @@ LOG = logging.getLogger(__name__) PXE_CFG_DIR_NAME = 'pxelinux.cfg' +DHCP_CLIENT_ID = '61' # rfc2132 +DHCP_TFTP_SERVER_NAME = '66' # rfc2132 +DHCP_BOOTFILE_NAME = '67' # rfc2132 +DHCP_TFTP_SERVER_ADDRESS = '150' # rfc5859 +DHCP_IPXE_ENCAP_OPTS = '175' # Tentatively Assigned +DHCP_TFTP_PATH_PREFIX = '210' # rfc5071 + def get_root_dir(): """Returns the directory where the config files and images will live.""" @@ -317,30 +324,47 @@ def dhcp_options_for_instance(task): if dhcp_provider_name == 'neutron': # Neutron use dnsmasq as default DHCP agent, add extra config # to neutron "dhcp-match=set:ipxe,175" and use below option - dhcp_opts.append({'opt_name': 'tag:!ipxe,bootfile-name', + dhcp_opts.append({'opt_name': "tag:!ipxe,%s" % DHCP_BOOTFILE_NAME, 'opt_value': boot_file}) - dhcp_opts.append({'opt_name': 'tag:ipxe,bootfile-name', + dhcp_opts.append({'opt_name': "tag:ipxe,%s" % DHCP_BOOTFILE_NAME, 'opt_value': ipxe_script_url}) else: # !175 == non-iPXE. # http://ipxe.org/howto/dhcpd#ipxe-specific_options - dhcp_opts.append({'opt_name': '!175,bootfile-name', + dhcp_opts.append({'opt_name': "!%s,%s" % (DHCP_IPXE_ENCAP_OPTS, + DHCP_BOOTFILE_NAME), 'opt_value': boot_file}) - dhcp_opts.append({'opt_name': 'bootfile-name', + dhcp_opts.append({'opt_name': DHCP_BOOTFILE_NAME, 'opt_value': ipxe_script_url}) else: - dhcp_opts.append({'opt_name': 'bootfile-name', + dhcp_opts.append({'opt_name': DHCP_BOOTFILE_NAME, 'opt_value': boot_file}) # 210 == tftp server path-prefix or tftp root, will be used to find # pxelinux.cfg directory. The pxelinux.0 loader infers this information # from it's own path, but Petitboot needs it to be specified by this # option since it doesn't use pxelinux.0 loader. - dhcp_opts.append({'opt_name': '210', + dhcp_opts.append({'opt_name': DHCP_TFTP_PATH_PREFIX, 'opt_value': get_tftp_path_prefix()}) - dhcp_opts.append({'opt_name': 'server-ip-address', + dhcp_opts.append({'opt_name': DHCP_TFTP_SERVER_NAME, 'opt_value': CONF.pxe.tftp_server}) - dhcp_opts.append({'opt_name': 'tftp-server', + dhcp_opts.append({'opt_name': DHCP_TFTP_SERVER_ADDRESS, + 'opt_value': CONF.pxe.tftp_server}) + + # NOTE(vsaienko) set this option specially for dnsmasq case as it always + # sets `siaddr` field which is treated by pxe clients as TFTP server + # see page 9 https://tools.ietf.org/html/rfc2131. + # If `server-ip-address` is not provided dnsmasq sets `siaddr` to dnsmasq's + # IP which breaks PXE booting as TFTP server is configured on ironic + # conductor host. + # http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=blob;f=src/dhcp-common.c;h=eae9ae3567fe16eb979a484976c270396322efea;hb=a3303e196e5d304ec955c4d63afb923ade66c6e8#l572 # noqa + # There is an informational RFC which describes how options related to + # tftp 150,66 and siaddr should be used https://tools.ietf.org/html/rfc5859 + # All dhcp servers we've tried: contrail/dnsmasq/isc just silently ignore + # unknown options but potentially it may blow up with others. + # Related bug was opened on Neutron side: + # https://bugs.launchpad.net/neutron/+bug/1723354 + dhcp_opts.append({'opt_name': 'server-ip-address', 'opt_value': CONF.pxe.tftp_server}) # Append the IP version for all the configuration options diff --git a/ironic/dhcp/base.py b/ironic/dhcp/base.py index 94f61fd091..6e694be01e 100644 --- a/ironic/dhcp/base.py +++ b/ironic/dhcp/base.py @@ -36,12 +36,10 @@ class BaseDHCP(object): :: - [{'opt_name': 'bootfile-name', + [{'opt_name': '67', 'opt_value': 'pxelinux.0'}, - {'opt_name': 'server-ip-address', - 'opt_value': '123.123.123.456'}, - {'opt_name': 'tftp-server', - 'opt_value': '123.123.123.123'}] + {'opt_name': '66', + 'opt_value': '123.123.123.456'}] :param token: An optional authentication token. :raises: FailedToUpdateDHCPOptOnPort @@ -56,12 +54,10 @@ class BaseDHCP(object): :: - [{'opt_name': 'bootfile-name', + [{'opt_name': '67', 'opt_value': 'pxelinux.0'}, - {'opt_name': 'server-ip-address', - 'opt_value': '123.123.123.456'}, - {'opt_name': 'tftp-server', - 'opt_value': '123.123.123.123'}] + {'opt_name': '66', + 'opt_value': '123.123.123.456'}] :param vifs: A dict with keys 'ports' and 'portgroups' and dicts as values. Each dict has key/value pairs of the form diff --git a/ironic/dhcp/neutron.py b/ironic/dhcp/neutron.py index a474c501d3..b885c18512 100644 --- a/ironic/dhcp/neutron.py +++ b/ironic/dhcp/neutron.py @@ -47,12 +47,10 @@ class NeutronDHCPApi(base.BaseDHCP): :: - [{'opt_name': 'bootfile-name', + [{'opt_name': '67', 'opt_value': 'pxelinux.0'}, - {'opt_name': 'server-ip-address', - 'opt_value': '123.123.123.456'}, - {'opt_name': 'tftp-server', - 'opt_value': '123.123.123.123'}] + {'opt_name': '66', + 'opt_value': '123.123.123.456'}] :param token: optional auth token. :raises: FailedToUpdateDHCPOptOnPort @@ -72,12 +70,10 @@ class NeutronDHCPApi(base.BaseDHCP): :: - [{'opt_name': 'bootfile-name', + [{'opt_name': '67', 'opt_value': 'pxelinux.0'}, - {'opt_name': 'server-ip-address', - 'opt_value': '123.123.123.456'}, - {'opt_name': 'tftp-server', - 'opt_value': '123.123.123.123'}] + {'opt_name': '66', + 'opt_value': '123.123.123.456'}] :param vifs: a dict of Neutron port/portgroup dicts to update DHCP options on. The port/portgroup dict key should be Ironic port UUIDs, and the values diff --git a/ironic/drivers/modules/network/common.py b/ironic/drivers/modules/network/common.py index 39d9453d66..d01f9cfb3a 100644 --- a/ironic/drivers/modules/network/common.py +++ b/ironic/drivers/modules/network/common.py @@ -24,6 +24,7 @@ from ironic.common import exception from ironic.common.i18n import _ from ironic.common import network from ironic.common import neutron +from ironic.common.pxe_utils import DHCP_CLIENT_ID from ironic.common import states from ironic.common import utils from ironic import objects @@ -245,7 +246,8 @@ def plug_port_to_tenant_network(task, port_like_obj, client=None): local_link_info.append(port_like_obj.local_link_connection) client_id = port_like_obj.extra.get('client-id') if client_id: - client_id_opt = ({'opt_name': 'client-id', 'opt_value': client_id}) + client_id_opt = ({'opt_name': DHCP_CLIENT_ID, + 'opt_value': client_id}) # NOTE(sambetts) Only update required binding: attributes, # because other port attributes may have been set by the user or @@ -415,7 +417,7 @@ class NeutronVIFPortIDMixin(VIFPortIDMixin): # from the neutron port if vif: api = dhcp_factory.DHCPFactory() - client_id_opt = {'opt_name': 'client-id', + client_id_opt = {'opt_name': DHCP_CLIENT_ID, 'opt_value': updated_client_id} api.provider.update_port_dhcp_opts( diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py index 6fa0c14e43..a5b9c8d560 100644 --- a/ironic/tests/unit/common/test_neutron.py +++ b/ironic/tests/unit/common/test_neutron.py @@ -170,7 +170,7 @@ class TestNeutronNetworkActions(db_base.DbTestCase): if is_client_id: expected_body['port']['extra_dhcp_opts'] = ( - [{'opt_name': 'client-id', 'opt_value': self._CLIENT_ID}]) + [{'opt_name': '61', 'opt_value': self._CLIENT_ID}]) # Ensure we can create ports self.client_mock.create_port.return_value = { 'port': self.neutron_port} diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py index 80c9fd4ef8..b3423e17cf 100644 --- a/ironic/tests/unit/common/test_pxe_utils.py +++ b/ironic/tests/unit/common/test_pxe_utils.py @@ -621,18 +621,21 @@ class TestPXEUtils(db_base.DbTestCase): self.config(tftp_server='192.0.2.1', group='pxe') self.config(pxe_bootfile_name='fake-bootfile', group='pxe') self.config(tftp_root='/tftp-path/', group='pxe') - expected_info = [{'opt_name': 'bootfile-name', + expected_info = [{'opt_name': '67', 'opt_value': 'fake-bootfile', 'ip_version': ip_version}, {'opt_name': '210', 'opt_value': '/tftp-path/', 'ip_version': ip_version}, + {'opt_name': '66', + 'opt_value': '192.0.2.1', + 'ip_version': ip_version}, + {'opt_name': '150', + 'opt_value': '192.0.2.1', + 'ip_version': ip_version}, {'opt_name': 'server-ip-address', 'opt_value': '192.0.2.1', - 'ip_version': ip_version}, - {'opt_name': 'tftp-server', - 'opt_value': '192.0.2.1', - 'ip_version': ip_version}, + 'ip_version': ip_version} ] with task_manager.acquire(self.context, self.node.uuid) as task: self.assertEqual(expected_info, @@ -689,17 +692,20 @@ class TestPXEUtils(db_base.DbTestCase): self.config(dhcp_provider='isc', group='dhcp') expected_boot_script_url = 'http://192.0.3.2:1234/boot.ipxe' - expected_info = [{'opt_name': '!175,bootfile-name', + expected_info = [{'opt_name': '!175,67', 'opt_value': boot_file, 'ip_version': 4}, + {'opt_name': '66', + 'opt_value': '192.0.2.1', + 'ip_version': 4}, + {'opt_name': '150', + 'opt_value': '192.0.2.1', + 'ip_version': 4}, + {'opt_name': '67', + 'opt_value': expected_boot_script_url, + 'ip_version': 4}, {'opt_name': 'server-ip-address', 'opt_value': '192.0.2.1', - 'ip_version': 4}, - {'opt_name': 'tftp-server', - 'opt_value': '192.0.2.1', - 'ip_version': 4}, - {'opt_name': 'bootfile-name', - 'opt_value': expected_boot_script_url, 'ip_version': 4}] self.assertItemsEqual(expected_info, @@ -707,17 +713,20 @@ class TestPXEUtils(db_base.DbTestCase): self.config(dhcp_provider='neutron', group='dhcp') expected_boot_script_url = 'http://192.0.3.2:1234/boot.ipxe' - expected_info = [{'opt_name': 'tag:!ipxe,bootfile-name', + expected_info = [{'opt_name': 'tag:!ipxe,67', 'opt_value': boot_file, 'ip_version': 4}, + {'opt_name': '66', + 'opt_value': '192.0.2.1', + 'ip_version': 4}, + {'opt_name': '150', + 'opt_value': '192.0.2.1', + 'ip_version': 4}, + {'opt_name': 'tag:ipxe,67', + 'opt_value': expected_boot_script_url, + 'ip_version': 4}, {'opt_name': 'server-ip-address', 'opt_value': '192.0.2.1', - 'ip_version': 4}, - {'opt_name': 'tftp-server', - 'opt_value': '192.0.2.1', - 'ip_version': 4}, - {'opt_name': 'tag:ipxe,bootfile-name', - 'opt_value': expected_boot_script_url, 'ip_version': 4}] self.assertItemsEqual(expected_info, diff --git a/ironic/tests/unit/drivers/modules/network/test_common.py b/ironic/tests/unit/drivers/modules/network/test_common.py index 35b7a8a94b..6c76540b2d 100644 --- a/ironic/tests/unit/drivers/modules/network/test_common.py +++ b/ironic/tests/unit/drivers/modules/network/test_common.py @@ -970,7 +970,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') def test_port_changed_client_id(self, dhcp_update_mock): expected_extra = {'vif_port_id': 'fake-id', 'client-id': 'fake2'} - expected_dhcp_opts = [{'opt_name': 'client-id', 'opt_value': 'fake2'}] + expected_dhcp_opts = [{'opt_name': '61', 'opt_value': 'fake2'}] self.port.extra = expected_extra with task_manager.acquire(self.context, self.node.id) as task: self.interface.port_changed(task, self.port) diff --git a/ironic/tests/unit/drivers/modules/network/test_neutron.py b/ironic/tests/unit/drivers/modules/network/test_neutron.py index 3708125cc5..65c413b0e8 100644 --- a/ironic/tests/unit/drivers/modules/network/test_neutron.py +++ b/ironic/tests/unit/drivers/modules/network/test_neutron.py @@ -328,9 +328,9 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): } if is_client_id: port1_body['port']['extra_dhcp_opts'] = ( - [{'opt_name': 'client-id', 'opt_value': client_ids[0]}]) + [{'opt_name': '61', 'opt_value': client_ids[0]}]) port2_body['port']['extra_dhcp_opts'] = ( - [{'opt_name': 'client-id', 'opt_value': client_ids[1]}]) + [{'opt_name': '61', 'opt_value': client_ids[1]}]) with task_manager.acquire(self.context, self.node.id) as task: self.interface.configure_tenant_networks(task) client_mock.assert_called_once_with() diff --git a/releasenotes/notes/use-dhcp-option-numbers-8b0b0efae912ff5f.yaml b/releasenotes/notes/use-dhcp-option-numbers-8b0b0efae912ff5f.yaml new file mode 100644 index 0000000000..167d834632 --- /dev/null +++ b/releasenotes/notes/use-dhcp-option-numbers-8b0b0efae912ff5f.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Uses standard DHCP option codes instead of dnsmasq-specific option names, + because different backends use different option names. This fixes the + `compatibility issues with neutron's DHCP backends + `.