openstack/puppet-ironic
Code Issues Proposed changes

Clean up ipxe support

Browse Source 
The puppet-ironic module's ipxe support does not align with current
versions of ironic which delineated ipxe and pxe support quite some
time ago.

This is an attempt to reconcile this and also change the default
ipxe binary for EFI from ipxe.efi which is intended to contain
firmware, and snponly.efi which only has the EFI networking stack.
In the vast majority of cases, short of very broken EFI firmware,
operators need snponly.efi.

Change-Id: Ia30aff290ec24972f387612851f8f630ddc9403b
This commit is contained in:
Julia Kreger 2021-06-23 08:32:56 -07:00
parent 12bc9b1d5e
commit 3864e15998
5 changed files with 122 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

98
manifests/drivers/pxe.pp
View File

@ -19,25 +19,31 @@
#
# === Parameters
#
# [*ipxe_enabled*]
# (optional) Enable ipxe support
# Defaults to false.
#
# [*pxe_append_params*]
# (optional) Additional append parameters for baremetal PXE boot.
# Should be valid pxe parameters
# Defaults to $::os_service_default
# Defaults to $::os_service_default.
#
# [*pxe_bootfile_name*]
# (optional) Bootfile DHCP parameter.
# If not set, its value is detected based on ipxe_enabled.
# Defaults to undef.
# If not set, its value is detected.
# Defaults to $::os_service_default.
#
# [*pxe_config_template*]
# (optional) Template file for PXE configuration.
# If set, should be an valid template file. Otherwise, its value is detected
# based on ipxe_enabled.
# Defaults to undef.
# If set, should be an valid template file. Otherwise, its value is detected.
# Defaults to $::os_service_default.
#
# [*ipxe_bootfile_name*]
# (optional) Bootfile DHCP parameter when the ipxe boot interface is set
# for a baremetal node. If not set, its value is detected.
# Defaults to $::os_service_default.
#
# [*ipxe_config_template*]
# (optional) Template file for PXE configuration with the iPXE boot
# interface. If set, should be an valid template file. Otherwise,
# its value is detected.
# Defaults to $::os_service_default.
#
# [*tftp_server*]
# (optional) IP address of Ironic compute node's tftp server.
@ -47,7 +53,7 @@
# [*tftp_root*]
# (optional) Ironic compute node's tftp root path.
# Should be an valid path
# Defaults to '/tftpboot'.
# Defaults to $::os_service_default.
#
# [*images_path*]
# (optional) Directory where images are stored on disk.
@ -57,7 +63,7 @@
# [*tftp_master_path*]
# (optional) Directory where master tftp images are stored on disk.
# Should be an valid directory
# Defaults to '/tftpboot/master_images'.
# Defaults to $::os_service_default.
#
# [*instance_master_path*]
# (optional) Directory where master tftp images are stored on disk.
@ -72,6 +78,13 @@
# (optional) Template file for PXE configuration for UEFI boot loader.
# Defaults to $::os_service_default.
#
# [*uefi_ipxe_bootfile_name*]
# (optional) Bootfile DHCP parameter for UEFI boot mode for the
# ipxe boot interface. No separate configuration template is required
# when using ipxe.
# Defaults to snponly.efi, which supports UEFI firmware with network
# enablement, which is a standard feature in UEFI.
#
# [*ipxe_timeout*]
# (optional) ipxe timeout in second.
# Should be an valid integer
@ -89,59 +102,73 @@
# (optional) How often (in seconds) to check for PXE boot status.
# Defaults to $::os_service_default.
#
# DEPRECATED PARAMETERS
#
# [*ipxe_enabled*]
# DEPRECATED: This option is no longer used as support for the option was
# deprecated during Ironic's Stein development cycle and removed during
# Ironic's Train development cycle.
# If this setting is populated, a warning will be indicated.
#
# [*ip_version*]
# (optional) The IP version that will be used for PXE booting.
# DEPRECATED: (optional) The IP version that will be used for PXE booting.
# Ironic presently attempts both IPv4 and IPv6, this option is effectively
# ignored by ironic, and should anticipate being removed in a future
# release.
# Defaults to $::os_service_default.
#
class ironic::drivers::pxe (
$ipxe_enabled = false,
$ipxe_enabled = undef,
$pxe_append_params = $::os_service_default,
$pxe_bootfile_name = undef,
$pxe_config_template = undef,
$pxe_bootfile_name = $::os_service_default,
$pxe_config_template = $::os_service_default,
$ipxe_bootfile_name = $::os_service_default,
$ipxe_config_template = $::os_service_default,
$tftp_server = $::os_service_default,
$tftp_root = '/tftpboot',
$tftp_root = $::os_service_default,
$images_path = $::os_service_default,
$tftp_master_path = '/tftpboot/master_images',
$tftp_master_path = $::os_service_default,
$instance_master_path = $::os_service_default,
$uefi_pxe_bootfile_name = $::os_service_default,
$uefi_pxe_config_template = $::os_service_default,
$uefi_ipxe_bootfile_name = 'snponly.efi',
$ipxe_timeout = $::os_service_default,
$enable_ppc64le = false,
$boot_retry_timeout = $::os_service_default,
$boot_retry_check_interval = $::os_service_default,
$ip_version = $::os_service_default,
$ip_version = undef,
) {
include ironic::deps
include ironic::pxe::common
$tftp_root_real = pick($::ironic::pxe::common::tftp_root, $tftp_root)
$ipxe_timeout_real = pick($::ironic::pxe::common::ipxe_timeout, $ipxe_timeout)
if $ipxe_enabled {
$pxe_bootfile_name_real = pick($pxe_bootfile_name, 'undionly.kpxe')
$pxe_config_template_real = pick($pxe_config_template, '$pybasedir/drivers/modules/ipxe_config.template')
} else {
$pxe_bootfile_name_real = pick($pxe_bootfile_name, 'pxelinux.0')
$pxe_config_template_real = pick($pxe_config_template, '$pybasedir/drivers/modules/pxe_config.template')
if $ipxe_enabled != undef {
warning('The ironic::drivers::pxe::ipxe_enabled parameter is deprecated and has no effect.')
}
if $ip_version != undef {
warning('The ironic::drivers::pxe:ip_version parameter is deprecated and will be removed in the future.')
}
# Configure ironic.conf
ironic_config {
'pxe/pxe_append_params': value => $pxe_append_params;
'pxe/pxe_bootfile_name': value => $pxe_bootfile_name_real;
'pxe/pxe_config_template': value => $pxe_config_template_real;
'pxe/pxe_bootfile_name': value => $pxe_bootfile_name;
'pxe/pxe_config_template': value => $pxe_config_template;
'pxe/ipxe_bootfile_name': value => $ipxe_bootfile_name;
'pxe/ipxe_config_template': value => $ipxe_config_template;
'pxe/tftp_server': value => $tftp_server;
'pxe/tftp_root': value => $tftp_root_real;
'pxe/tftp_root': value => $tftp_root;
'pxe/images_path': value => $images_path;
'pxe/tftp_master_path': value => $tftp_master_path;
'pxe/instance_master_path': value => $instance_master_path;
'pxe/uefi_pxe_bootfile_name': value => $uefi_pxe_bootfile_name;
'pxe/uefi_pxe_config_template': value => $uefi_pxe_config_template;
'pxe/ipxe_timeout': value => $ipxe_timeout_real;
'pxe/uefi_ipxe_bootfile_name': value => $uefi_ipxe_bootfile_name;
'pxe/ipxe_timeout': value => $ipxe_timeout;
'pxe/boot_retry_timeout': value => $boot_retry_timeout;
'pxe/boot_retry_check_interval': value => $boot_retry_check_interval;
'pxe/ip_version': value => $ip_version;
'pxe/ipxe_enabled': ensure => absent;
}
if $enable_ppc64le {
@ -150,7 +177,12 @@ class ironic::drivers::pxe (
# architecture
ironic_config {
# NOTE(tonyb): This first value shouldn't be needed but seems to be?
'pxe/pxe_config_template_by_arch': value => "ppc64le:${pxe_config_template_real}";
# NOTE(TheJulia): Likely not needed as this just points to the default,
# and when the explicit pxe driver is used everything should fall to
# it but in the interest of minimizing impact, the output result
# is preserved as we now just allow the default for normal template
# operation to be used.
'pxe/pxe_config_template_by_arch': value => 'ppc64le:$pybasedir/drivers/modules/pxe_config.template';
'pxe/pxe_bootfile_name_by_arch': value => 'ppc64le:config';
}
}

4
manifests/pxe.pp
View File

@ -66,7 +66,7 @@ class ironic::pxe (
$syslinux_files = $::ironic::params::syslinux_files,
$tftp_bind_host = undef,
$enable_ppc64le = false,
$ipxe_name_base = 'ipxe',
$ipxe_name_base = 'ipxe-snponly',
) inherits ironic::params {
include ironic::deps
@ -181,7 +181,7 @@ class ironic::pxe (
require => Anchor['ironic-inspector::install::end'],
}
file { "${tftp_root_real}/ipxe.efi":
file { "${tftp_root_real}/snponly.efi":
ensure => 'file',
seltype => 'tftpdir_t',
owner => 'ironic',

44
releasenotes/notes/pxe-cleanup-692c78cad322893d.yaml Normal file
View File

@ -0,0 +1,44 @@
---
features:
- Adds support for the ``ironic.conf`` parameters ``[pxe]ipxe_bootfile_name``
and ``[pxe]ipxe_config_template`` which are utilized by the ``ipxe`` boot
interface in Ironic. These settings use the manifest parameters
``ironic::drivers::pxe::ipxe_bootfile_name`` and
``ironic::drivers::pxe::ipxe_config_template`` respectively.
- Adds support for the ``ironic.conf`` parameter
``[pxe]uefi_ipxe_bootfile_name`` to be explicitly set using the
``ironic::drivers::pxe::uefi_ipxe_bootfile_name`` manifest parameter,
however this value is defaulted to ``snponly.efi`` which is anticipated
to become the default in Ironic sometime in the Xena development cycle.
upgrade:
- |
The Ironic project has in elevated support for ``ipxe`` to a top level
node boot_interface, and removed support for it's ``[pxe]ipxe_enabled``
option as this is settable per node. Removal of this option has allowed
for the manifest to be cleaned up.
- The manifest now no longer applies override defaults for a number of
``ironic.conf`` settings, as they match the default settings the project
utilizes. These manifest parameters are
``ironic::drivers::pxe::pxe_bootfile_name``,
``ironic::drivers::pxe::pxe_config_template``,
``ironic::drivers::pxe::tftp_root``,
``ironic::drivers::pxe::tftp_master_path``.
deprecations:
- |
The ``ironic::drivers::pxe::ipxe_enabled`` parameter has been deprecated
and has no effect moving forward. This is a result of the underlying
Ironic project deprecating and removing. Use of this option will raise
a warning.
- The ``ironic::drivers::pxe::ip_version`` parameter has been deprecated
for removal. Please anticipate this option to be removed in a future
release. This option has been redundant since the Ussuri release of
Ironic. Use of this option raises a warning.
fixes:
- |
Fixes a potential issue where use of the
``ironic::drivers::pxe::ipxe_enabled`` parameter would
cause settings specific to ``ipxe`` to be set overriding the ``pxe`` boot
interface's defaults in Ironic. This resulted in operators being unable to
choose to boot a node from ``pxe`` and not ``ipxe``. Now the settings are
not overriden as the ``ironic::drivers::pxe::ipxe_enabled`` parameter for
the manifest has no effect, and raises a warning if used.

41
spec/classes/ironic_drivers_pxe_spec.rb
View File

@ -23,10 +23,7 @@ require 'spec_helper'
describe 'ironic::drivers::pxe' do
let :default_params do
{ :pxe_bootfile_name => 'pxelinux.0',
:pxe_config_template => '$pybasedir/drivers/modules/pxe_config.template',
:tftp_root => '/tftpboot',
:tftp_master_path => '/tftpboot/master_images',
{ :uefi_ipxe_bootfile_name => 'snponly.efi',
}
end
@ -41,37 +38,18 @@ describe 'ironic::drivers::pxe' do
it 'configures ironic.conf' do
is_expected.to contain_ironic_config('pxe/pxe_append_params').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/pxe_bootfile_name').with_value(p[:pxe_bootfile_name])
is_expected.to contain_ironic_config('pxe/pxe_config_template').with_value(p[:pxe_config_template])
is_expected.to contain_ironic_config('pxe/pxe_bootfile_name').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/pxe_config_template').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/ipxe_bootfile_name').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/ipxe_config_template').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/tftp_server').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/tftp_root').with_value(p[:tftp_root])
is_expected.to contain_ironic_config('pxe/tftp_root').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/images_path').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/tftp_master_path').with_value(p[:tftp_master_path])
is_expected.to contain_ironic_config('pxe/tftp_master_path').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/instance_master_path').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/uefi_pxe_bootfile_name').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/uefi_pxe_config_template').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/ipxe_enabled').with_ensure('absent')
end
context 'when overriding only ipxe_enabled' do
before do
params.merge!(
:ipxe_enabled => true,
)
end
it 'detects correct boot parameters' do
is_expected.to contain_ironic_config('pxe/pxe_append_params').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/pxe_bootfile_name').with_value('undionly.kpxe')
is_expected.to contain_ironic_config('pxe/pxe_config_template').with_value('$pybasedir/drivers/modules/ipxe_config.template')
is_expected.to contain_ironic_config('pxe/tftp_server').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/tftp_root').with_value(p[:tftp_root])
is_expected.to contain_ironic_config('pxe/images_path').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/tftp_master_path').with_value(p[:tftp_master_path])
is_expected.to contain_ironic_config('pxe/instance_master_path').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/uefi_pxe_bootfile_name').with_value('<SERVICE DEFAULT>')
is_expected.to contain_ironic_config('pxe/uefi_pxe_config_template').with_value('<SERVICE DEFAULT>')
end
is_expected.to contain_ironic_config('pxe/uefi_ipxe_bootfile_name').with_value('snponly.efi')
end
context 'when overriding only enable_ppc64le' do
@ -98,9 +76,9 @@ describe 'ironic::drivers::pxe' do
:tftp_master_path => '/mnt/master_images',
:instance_master_path => '/mnt/ironic/master_images',
:uefi_pxe_bootfile_name => 'bootx64.efi',
:uefi_ipxe_bootfile_name => 'ipxe.efi',
:uefi_pxe_config_template => 'foo-uefi',
:ipxe_timeout => '60',
:ipxe_enabled => true,
:pxe_bootfile_name => 'bootx64',
:boot_retry_timeout => 600,
:boot_retry_check_interval => 120,
@ -118,6 +96,7 @@ describe 'ironic::drivers::pxe' do
is_expected.to contain_ironic_config('pxe/instance_master_path').with_value(p[:instance_master_path])
is_expected.to contain_ironic_config('pxe/uefi_pxe_bootfile_name').with_value(p[:uefi_pxe_bootfile_name])
is_expected.to contain_ironic_config('pxe/uefi_pxe_config_template').with_value(p[:uefi_pxe_config_template])
is_expected.to contain_ironic_config('pxe/uefi_ipxe_bootfile_name').with_value(p[:uefi_ipxe_bootfile_name])
is_expected.to contain_ironic_config('pxe/ipxe_timeout').with_value(p[:ipxe_timeout])
is_expected.to contain_ironic_config('pxe/pxe_bootfile_name').with_value(p[:pxe_bootfile_name])
is_expected.to contain_ironic_config('pxe/boot_retry_timeout').with_value(p[:boot_retry_timeout])

2
spec/classes/ironic_pxe_spec.rb
View File

@ -111,7 +111,7 @@ describe 'ironic::pxe' do
)
end
it 'should contain iPXE UEFI chainload image' do
is_expected.to contain_file('/var/lib/tftpboot/ipxe.efi').with(
is_expected.to contain_file('/var/lib/tftpboot/snponly.efi').with(
'owner' => 'ironic',
'group' => 'ironic',
'require' => 'Anchor[ironic-inspector::install::end]',