openstack/ironic
Code Issues Proposed changes

Change pxe dhcp options name to codes.

Browse Source 
There is difference between dhcp option names in different backends.
This patch changes options name to code according to [0].

[0] https://www.iana.org/assignments/bootp-dhcp-parameters/bootp-dhcp-parameters.xhtml

Closes-Bug: 1717236

This is an updated version of c377f5cbbd
which problems with PXE and dnsmasq due to buggy dnsmasq code which uses
siaddr field to specify tftp server. They are addressed now by always
sending server-ip-address to make sure that dnsmasq works.

More information about siaddr and option 150,66 can be found in
informational RFC https://tools.ietf.org/html/rfc5859

Change-Id: I55487d867979bf6bb4cf228fcf6408beae955d2b
This commit is contained in:
Vasyl Saienko 2017-09-21 13:39:28 +00:00
parent 2d486ca8c0
commit 228a2a7885
11 changed files with 93 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
ironic/common/dhcp_factory.py
View File

@ -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

4
ironic/common/neutron.py
View File

@ -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

40
ironic/common/pxe_utils.py
View File

@ -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

16
ironic/dhcp/base.py
View File

@ -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

16
ironic/dhcp/neutron.py
View File

@ -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

6
ironic/drivers/modules/network/common.py
View File

@ -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(

2
ironic/tests/unit/common/test_neutron.py
View File

@ -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}

47
ironic/tests/unit/common/test_pxe_utils.py
View File

@ -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,

2
ironic/tests/unit/drivers/modules/network/test_common.py
View File

@ -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)

4
ironic/tests/unit/drivers/modules/network/test_neutron.py
View File

@ -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()

7
releasenotes/notes/use-dhcp-option-numbers-8b0b0efae912ff5f.yaml Normal file
View File

@ -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
<https://bugs.launchpad.net/ironic/+bug/1717236>`.