diff --git a/doc/source/admin/interfaces/boot.rst b/doc/source/admin/interfaces/boot.rst index 35d5561e36..2b74b17de4 100644 --- a/doc/source/admin/interfaces/boot.rst +++ b/doc/source/admin/interfaces/boot.rst @@ -39,12 +39,15 @@ specific implementations of the PXE boot interface. Additional configuration is required for this boot interface - see :doc:`/install/configure-pxe` for details. +Common options +-------------- + Enable persistent boot device for deploy/clean operation ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Ironic uses non-persistent boot for cleaning/deploying phases as default, -in PXE interface. For some drivers, a persistent change is far more -costly than a non-persistent one, so this can bring performance improvements. +Ironic uses non-persistent boot for cleaning/deploying phases as default. +For some drivers, a persistent change is far more costly than a non-persistent +one, so this can bring performance improvements. Set the flag ``force_persistent_boot_device`` to ``True`` in the node's ``driver_info``:: diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index d0e2bbd157..32ce3e9d39 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -26,6 +26,7 @@ from oslo_log import log from oslo_serialization import jsonutils from oslo_service import loopingcall from oslo_utils import excutils +from oslo_utils import strutils from oslo_utils import timeutils from ironic.common import boot_devices @@ -67,10 +68,22 @@ def node_set_boot_device(task, device, persistent=False): """ task.driver.management.validate(task) - if task.node.provision_state != states.ADOPTING: - task.driver.management.set_boot_device(task, - device=device, - persistent=persistent) + if task.node.provision_state == states.ADOPTING: + return + + force_persistent = task.node.driver_info.get( + 'force_persistent_boot_device') + if force_persistent == 'Always': + persistent = True + elif force_persistent == 'Never': + persistent = False + elif force_persistent not in (None, 'Default'): + # Backward compatibility (used to be a boolean and only True mattered) + if strutils.bool_from_string(force_persistent, strict=False): + persistent = True + + task.driver.management.set_boot_device(task, device=device, + persistent=persistent) def node_get_boot_mode(task): diff --git a/ironic/drivers/modules/ilo/boot.py b/ironic/drivers/modules/ilo/boot.py index c0e0411084..0ed7b190f3 100644 --- a/ironic/drivers/modules/ilo/boot.py +++ b/ironic/drivers/modules/ilo/boot.py @@ -37,6 +37,7 @@ from ironic.drivers.modules.ilo import common as ilo_common from ironic.drivers.modules import image_utils from ironic.drivers.modules import ipxe from ironic.drivers.modules import pxe +from ironic.drivers import utils as driver_utils LOG = logging.getLogger(__name__) @@ -350,7 +351,7 @@ class IloVirtualMediaBoot(base.BootInterface): # TODO(stendulker): COMMON_PROPERTIES should also include rescue # related properties (RESCUE_PROPERTIES). We can add them in Rocky, # when classic drivers get removed. - return COMMON_PROPERTIES + return dict(driver_utils.OPTIONAL_PROPERTIES, **COMMON_PROPERTIES) @METRICS.timer('IloVirtualMediaBoot.validate') def validate(self, task): diff --git a/ironic/drivers/modules/irmc/boot.py b/ironic/drivers/modules/irmc/boot.py index b2bcd9c308..0fcc7263f0 100644 --- a/ironic/drivers/modules/irmc/boot.py +++ b/ironic/drivers/modules/irmc/boot.py @@ -41,6 +41,7 @@ from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.irmc import common as irmc_common from ironic.drivers.modules.irmc import management as irmc_management from ironic.drivers.modules import pxe +from ironic.drivers import utils as driver_utils scci = importutils.try_import('scciclient.irmc.scci') @@ -83,6 +84,7 @@ OPTIONAL_PROPERTIES = { } COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy() +COMMON_PROPERTIES.update(driver_utils.OPTIONAL_PROPERTIES) COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES) diff --git a/ironic/drivers/modules/pxe_base.py b/ironic/drivers/modules/pxe_base.py index 0944e7d4e7..70162a2eec 100644 --- a/ironic/drivers/modules/pxe_base.py +++ b/ironic/drivers/modules/pxe_base.py @@ -17,7 +17,6 @@ from futurist import periodics from ironic_lib import metrics_utils from oslo_config import cfg from oslo_log import log as logging -from oslo_utils import strutils from ironic.common import boot_devices from ironic.common import dhcp_factory @@ -45,18 +44,6 @@ REQUIRED_PROPERTIES = { 'deploy_ramdisk': _("UUID (from Glance) of the ramdisk that is " "mounted at boot time. Required."), } -OPTIONAL_PROPERTIES = { - 'force_persistent_boot_device': _("Controls the persistency of boot order " - "changes. 'Always' will make all " - "changes persistent, 'Default' will " - "make all but the final one upon " - "instance deployment non-persistent, " - "and 'Never' will make no persistent " - "changes at all. The old values 'True' " - "and 'False' are still supported but " - "deprecated in favor of the new ones." - "Defaults to 'Default'. Optional."), -} RESCUE_PROPERTIES = { 'rescue_kernel': _('UUID (from Glance) of the rescue kernel. This value ' 'is required for rescue mode.'), @@ -65,7 +52,7 @@ RESCUE_PROPERTIES = { 'required for rescue mode.'), } COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy() -COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES) +COMMON_PROPERTIES.update(driver_utils.OPTIONAL_PROPERTIES) COMMON_PROPERTIES.update(RESCUE_PROPERTIES) @@ -211,9 +198,8 @@ class PXEBaseMixin(object): pxe_utils.create_pxe_config(task, pxe_options, pxe_config_template, ipxe_enabled=self.ipxe_enabled) - persistent = self._persistent_ramdisk_boot(node) manager_utils.node_set_boot_device(task, boot_devices.PXE, - persistent=persistent) + persistent=False) if self.ipxe_enabled and CONF.pxe.ipxe_use_swift: kernel_label = '%s_kernel' % mode @@ -328,12 +314,8 @@ class PXEBaseMixin(object): # NOTE(pas-ha) do not re-set boot device on ACTIVE nodes # during takeover if boot_device and task.node.provision_state != states.ACTIVE: - persistent = True - if node.driver_info.get('force_persistent_boot_device', - 'Default') == 'Never': - persistent = False manager_utils.node_set_boot_device(task, boot_device, - persistent=persistent) + persistent=True) def _validate_common(self, task): node = task.node @@ -436,14 +418,6 @@ class PXEBaseMixin(object): raise exception.UnsupportedDriverExtension( driver=task.node.driver, extension='inspection') - def _persistent_ramdisk_boot(self, node): - """If the ramdisk should be configured as a persistent boot device.""" - value = node.driver_info.get('force_persistent_boot_device', 'Default') - if value in {'Always', 'Default', 'Never'}: - return value == 'Always' - else: - return strutils.bool_from_string(value, False) - _RETRY_ALLOWED_STATES = {states.DEPLOYWAIT, states.CLEANWAIT, states.RESCUEWAIT} @@ -493,11 +467,8 @@ class PXEBaseMixin(object): 'timeout': CONF.pxe.boot_retry_timeout}) manager_utils.node_power_action(task, states.POWER_OFF) - # NOTE(dtantsur): retry even persistent boot setting in case it did not - # work for some reason. - persistent = self._persistent_ramdisk_boot(task.node) manager_utils.node_set_boot_device(task, boot_devices.PXE, - persistent=persistent) + persistent=False) manager_utils.node_power_action(task, states.POWER_ON) diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index dbd05ab843..7ffc520812 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -29,6 +29,7 @@ from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import image_utils from ironic.drivers.modules.redfish import utils as redfish_utils +from ironic.drivers import utils as driver_utils LOG = log.getLogger(__name__) @@ -68,6 +69,7 @@ RESCUE_PROPERTIES = { } COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy() +COMMON_PROPERTIES.update(driver_utils.OPTIONAL_PROPERTIES) COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES) COMMON_PROPERTIES.update(RESCUE_PROPERTIES) diff --git a/ironic/drivers/utils.py b/ironic/drivers/utils.py index 07d7d4358b..4aa2335d67 100644 --- a/ironic/drivers/utils.py +++ b/ironic/drivers/utils.py @@ -370,3 +370,17 @@ def collect_ramdisk_logs(node, label=None): LOG.exception('Unknown error when storing logs from the node ' '%(node)s deployment. Error: %(error)s', {'node': node.uuid, 'error': e}) + + +OPTIONAL_PROPERTIES = { + 'force_persistent_boot_device': _("Controls the persistency of boot order " + "changes. 'Always' will make all " + "changes persistent, 'Default' will " + "make all but the final one upon " + "instance deployment non-persistent, " + "and 'Never' will make no persistent " + "changes at all. The old values 'True' " + "and 'False' are still supported but " + "deprecated in favor of the new ones." + "Defaults to 'Default'. Optional."), +} diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index c5fd842797..076963d5c3 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -5718,15 +5718,10 @@ class ManagerTestProperties(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): 'image_http_proxy', 'image_https_proxy', 'image_no_proxy']) if pxe_common: - expected.extend(['force_persistent_boot_device', - 'rescue_kernel', 'rescue_ramdisk']) + expected.extend(['rescue_kernel', 'rescue_ramdisk']) + expected.append('force_persistent_boot_device') self.assertCountEqual(expected, properties) - def test_driver_properties_fake(self): - expected = ['B1', 'B2'] - self._check_driver_properties("fake-hardware", expected, - agent_common=False, pxe_common=False) - def test_driver_properties_ipmi(self): self.config(enabled_hardware_types='ipmi', enabled_power_interfaces=['ipmitool'], diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index 8d0bf5f35c..5e0b9520ba 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -86,6 +86,37 @@ class NodeSetBootDeviceTestCase(db_base.DbTestCase): conductor_utils.node_set_boot_device(self.task, device='pxe') self.assertFalse(mock_sbd.called) + @mock.patch.object(fake.FakeManagement, 'set_boot_device', autospec=True) + def test_node_set_boot_device_force_default(self, mock_sbd): + # Boolean value False was equivalent to the default + for value in ('false', False, 'Default'): + self.task.node.driver_info['force_persistent_boot_device'] = value + for request in (True, False): + mock_sbd.reset_mock() + conductor_utils.node_set_boot_device(self.task, device='pxe', + persistent=request) + mock_sbd.assert_called_once_with(mock.ANY, self.task, + device='pxe', + persistent=request) + + @mock.patch.object(fake.FakeManagement, 'set_boot_device', autospec=True) + def test_node_set_boot_device_force_always(self, mock_sbd): + for value in ('true', True, 'Always'): + mock_sbd.reset_mock() + self.task.node.driver_info['force_persistent_boot_device'] = value + conductor_utils.node_set_boot_device(self.task, device='pxe', + persistent=False) + mock_sbd.assert_called_once_with(mock.ANY, self.task, + device='pxe', persistent=True) + + @mock.patch.object(fake.FakeManagement, 'set_boot_device', autospec=True) + def test_node_set_boot_device_force_never(self, mock_sbd): + self.task.node.driver_info['force_persistent_boot_device'] = 'Never' + conductor_utils.node_set_boot_device(self.task, device='pxe', + persistent=True) + mock_sbd.assert_called_once_with(mock.ANY, self.task, + device='pxe', persistent=False) + class NodeGetBootModeTestCase(db_base.DbTestCase): diff --git a/ironic/tests/unit/drivers/modules/test_ipxe.py b/ironic/tests/unit/drivers/modules/test_ipxe.py index 4c0536fc51..5229cc2500 100644 --- a/ironic/tests/unit/drivers/modules/test_ipxe.py +++ b/ironic/tests/unit/drivers/modules/test_ipxe.py @@ -343,80 +343,6 @@ class iPXEBootTestCase(db_base.DbTestCase): self.node.save() self._test_prepare_ramdisk() - def test_prepare_ramdisk_force_persistent_boot_device_true(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = 'True' - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=True) - - def test_prepare_ramdisk_force_persistent_boot_device_bool_true(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = True - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=True) - - def test_prepare_ramdisk_force_persistent_boot_device_sloppy_true(self): - for value in ['true', 't', '1', 'on', 'y', 'YES']: - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = value - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=True) - - def test_prepare_ramdisk_force_persistent_boot_device_false(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = 'False' - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk() - - def test_prepare_ramdisk_force_persistent_boot_device_bool_false(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = False - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=False) - - def test_prepare_ramdisk_force_persistent_boot_device_sloppy_false(self): - for value in ['false', 'f', '0', 'off', 'n', 'NO', 'yxz']: - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = value - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk() - - def test_prepare_ramdisk_force_persistent_boot_device_default(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = 'Default' - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=False) - - def test_prepare_ramdisk_force_persistent_boot_device_always(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = 'Always' - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=True) - - def test_prepare_ramdisk_force_persistent_boot_device_never(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = 'Never' - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=False) - def test_prepare_ramdisk_rescue(self): self.node.provision_state = states.RESCUING self.node.save() diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index 1ccc336993..da1d9da973 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/ironic/tests/unit/drivers/modules/test_pxe.py @@ -323,80 +323,6 @@ class PXEBootTestCase(db_base.DbTestCase): self.node.save() self._test_prepare_ramdisk() - def test_prepare_ramdisk_force_persistent_boot_device_true(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = 'True' - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=True) - - def test_prepare_ramdisk_force_persistent_boot_device_bool_true(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = True - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=True) - - def test_prepare_ramdisk_force_persistent_boot_device_sloppy_true(self): - for value in ['true', 't', '1', 'on', 'y', 'YES']: - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = value - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=True) - - def test_prepare_ramdisk_force_persistent_boot_device_false(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = 'False' - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk() - - def test_prepare_ramdisk_force_persistent_boot_device_bool_false(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = False - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=False) - - def test_prepare_ramdisk_force_persistent_boot_device_sloppy_false(self): - for value in ['false', 'f', '0', 'off', 'n', 'NO', 'yxz']: - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = value - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk() - - def test_prepare_ramdisk_force_persistent_boot_device_default(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = 'Default' - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=False) - - def test_prepare_ramdisk_force_persistent_boot_device_always(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = 'Always' - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=True) - - def test_prepare_ramdisk_force_persistent_boot_device_never(self): - self.node.provision_state = states.DEPLOYING - driver_info = self.node.driver_info - driver_info['force_persistent_boot_device'] = 'Never' - self.node.driver_info = driver_info - self.node.save() - self._test_prepare_ramdisk(persistent=False) - def test_prepare_ramdisk_rescue(self): self.node.provision_state = states.RESCUING self.node.save() diff --git a/releasenotes/notes/force-persistent-common-6ef2537f7ccd0dcb.yaml b/releasenotes/notes/force-persistent-common-6ef2537f7ccd0dcb.yaml new file mode 100644 index 0000000000..569d724900 --- /dev/null +++ b/releasenotes/notes/force-persistent-common-6ef2537f7ccd0dcb.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + The ``force_persistent_boot_device`` parameter now consistently applies + to all boot interfaces, rather than only PXE and iPXE.