openstack/ironic
Code Issues Proposed changes

Fix ramdisk boot option handling

Browse Source 
The current ramdisk deploy code expects a user to set the boot_option
capability to "ramdisk". Not only is it redundant, it's also not
documented, so chances are high nobody has ever done that.

As a side effect of e6bb99cd8f boot
interfaces no longer validate kernel/ramdisk/image if boot option
is "local". Unless a user explicitly sets boot option to "ramdisk",
the validation will be skipped for the ramdisk deploy.

This patch follows the pattern of the anaconda deploy and makes
get_boot_option always return "ramdisk" for the ramdisk deploy.

In the future we need to refactor this code so that the deploy interface
provides the boot option it works with, but that is a lot of changes.

Change-Id: I25c28df048c706f0c5b013b4d252f09d5a7e57bd
This commit is contained in:
Dmitry Tantsur 2021-06-22 19:12:02 +02:00
parent 416a0951c8
commit d7a5b3469c
3 changed files with 10 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
ironic/drivers/modules/deploy_utils.py
View File

@ -588,11 +588,21 @@ def get_boot_option(node):
return 'local'
if is_anaconda_deploy(node):
return 'kickstart'
if is_ramdisk_deploy(node):
return 'ramdisk'
capabilities = utils.parse_instance_info_capabilities(node)
return capabilities.get('boot_option',
CONF.deploy.default_boot_option).lower()
# FIXME(dtantsur): relying on deploy interface name is an anti-pattern.
# Refactor the code so that the deploy interface itself provides the only boot
# option it supports.
def is_ramdisk_deploy(node):
return node.deploy_interface == 'ramdisk'
def is_anaconda_deploy(node):
"""Determine if Anaconda deploy interface is in use for the deployment.

13
ironic/drivers/modules/pxe.py
View File

@ -90,19 +90,6 @@ class PXERamdiskDeploy(agent_base.AgentBaseMixin, agent_base.HeartbeatMixin,
@task_manager.require_exclusive_lock
def prepare(self, task):
node = task.node
# Log a warning if the boot_option is wrong... and
# otherwise reset it.
boot_option = deploy_utils.get_boot_option(node)
if boot_option != 'ramdisk':
LOG.warning('Incorrect "boot_option" set for node %(node)s '
'and will be overridden to "ramdisk" as to '
'match the deploy interface. Found: %(boot_opt)s.',
{'node': node.uuid,
'boot_opt': boot_option})
i_info = task.node.instance_info
i_info.update({'capabilities': {'boot_option': 'ramdisk'}})
node.instance_info = i_info
node.save()
deploy_utils.populate_storage_driver_internal_info(task)
if node.provision_state == states.DEPLOYING:

18
ironic/tests/unit/drivers/modules/test_pxe.py
View File

@ -941,8 +941,6 @@ class PXERamdiskDeployTestCase(db_base.DbTestCase):
with task_manager.acquire(self.context, node.uuid) as task:
task.driver.deploy.prepare(task)
self.assertFalse(mock_prepare_instance.called)
self.assertEqual({'boot_option': 'ramdisk'},
task.node.instance_info['capabilities'])
@mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True)
def test_prepare_active(self, mock_prepare_instance):
@ -962,22 +960,6 @@ class PXERamdiskDeployTestCase(db_base.DbTestCase):
task.driver.deploy.prepare(task)
mock_prepare_instance.assert_called_once_with(mock.ANY, task)
@mock.patch.object(pxe.LOG, 'warning', autospec=True)
@mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True)
def test_prepare_fixes_and_logs_boot_option_warning(
self, mock_prepare_instance, mock_warning):
node = self.node
node.properties['capabilities'] = 'boot_option:ramdisk'
node.provision_state = states.DEPLOYING
node.instance_info = {}
node.save()
with task_manager.acquire(self.context, node.uuid) as task:
task.driver.deploy.prepare(task)
self.assertFalse(mock_prepare_instance.called)
self.assertEqual({'boot_option': 'ramdisk'},
task.node.instance_info['capabilities'])
self.assertTrue(mock_warning.called)
@mock.patch.object(deploy_utils, 'validate_image_properties',
autospec=True)
def test_validate(self, mock_validate_img):