openstack/ironic
Code Issues Proposed changes

Adoption: do not validate boot interface when local booting

Browse Source 
We validate the boot interface during adoption because of:
a) potential rebuilding,
b) non-local boot.

Rebuild proved a rarely used feature, and local boot is the default
nowadays, so it makes less sense to unconditionally validate the boot
interface during adoption. We will run the validation anyway the next
time we need to do something with booting.

Similarly, do not record is_whole_disk_image None if it cannot be
reliably determined.

Change-Id: I95252aea808c48ea2d94569449c871f0d483caaa
This commit is contained in:
Dmitry Tantsur 2021-12-13 10:23:54 +01:00
parent f6f6ce1a31
commit d342b07dd6
3 changed files with 60 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
ironic/conductor/manager.py
View File

@ -72,6 +72,7 @@ from ironic.conductor import utils
from ironic.conductor import verify
from ironic.conf import CONF
from ironic.drivers import base as drivers_base
from ironic.drivers.modules import deploy_utils
from ironic.drivers.modules import image_cache
from ironic import objects
from ironic.objects import base as objects_base
@ -1712,11 +1713,13 @@ class ConductorManager(base_manager.BaseConductorManager):
# supplied.
iwdi = images.is_whole_disk_image(task.context,
task.node.instance_info)
node.set_driver_internal_info('is_whole_disk_image', iwdi)
# Calling boot validate to ensure that sufficient information
# is supplied to allow the node to be able to boot if takeover
# writes items such as kernel/ramdisk data to disk.
task.driver.boot.validate(task)
if iwdi is not None:
node.set_driver_internal_info('is_whole_disk_image', iwdi)
if deploy_utils.get_boot_option(node) != 'local':
# Calling boot validate to ensure that sufficient information
# is supplied to allow the node to be able to boot if takeover
# writes items such as kernel/ramdisk data to disk.
task.driver.boot.validate(task)
# NOTE(TheJulia): While task.driver.boot.validate() is called
# above, and task.driver.power.validate() could be called, it
# is called as part of the transition from ENROLL to MANAGEABLE

53
ironic/tests/unit/conductor/test_manager.py
View File

@ -7286,8 +7286,47 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
mock_prepare.assert_called_once_with(task.driver.deploy, task)
mock_take_over.assert_called_once_with(task.driver.deploy, task)
self.assertFalse(mock_start_console.called)
self.assertTrue(mock_boot_validate.called)
self.assertIn('is_whole_disk_image', task.node.driver_internal_info)
mock_boot_validate.assert_not_called()
self.assertNotIn('is_whole_disk_image', task.node.driver_internal_info)
@mock.patch('ironic.drivers.modules.fake.FakePower.validate',
autospec=True)
@mock.patch('ironic.drivers.modules.fake.FakeBoot.validate', autospec=True)
@mock.patch('ironic.drivers.modules.fake.FakeConsole.start_console',
autospec=True)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.take_over',
autospec=True)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.prepare',
autospec=True)
def test__do_adoption_with_netboot(self,
mock_prepare,
mock_take_over,
mock_start_console,
mock_boot_validate,
mock_power_validate):
"""Test a successful node adoption"""
self._start_service()
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.ADOPTING,
instance_info={
'capabilities': {'boot_option': 'netboot'},
'image_source': 'image',
})
task = task_manager.TaskManager(self.context, node.uuid)
self.service._do_adoption(task)
node.refresh()
self.assertEqual(states.ACTIVE, node.provision_state)
self.assertIsNone(node.last_error)
self.assertFalse(node.console_enabled)
mock_prepare.assert_called_once_with(task.driver.deploy, task)
mock_take_over.assert_called_once_with(task.driver.deploy, task)
self.assertFalse(mock_start_console.called)
mock_boot_validate.assert_called_once_with(task.driver.boot, task)
self.assertTrue(task.node.driver_internal_info.get(
'is_whole_disk_image'))
@mock.patch('ironic.drivers.modules.fake.FakeBoot.validate', autospec=True)
@mock.patch('ironic.drivers.modules.fake.FakeConsole.start_console',
@ -7328,8 +7367,7 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
mock_prepare.assert_called_once_with(task.driver.deploy, task)
mock_take_over.assert_called_once_with(task.driver.deploy, task)
self.assertFalse(mock_start_console.called)
self.assertTrue(mock_boot_validate.called)
self.assertIn('is_whole_disk_image', task.node.driver_internal_info)
mock_boot_validate.assert_not_called()
self.assertEqual(states.NOSTATE, node.power_state)
@mock.patch('ironic.drivers.modules.fake.FakeBoot.validate', autospec=True)
@ -7353,7 +7391,10 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self._start_service()
node = obj_utils.create_test_node(
self.context, driver='fake-hardware',
provision_state=states.ADOPTING)
provision_state=states.ADOPTING,
instance_info={
'capabilities': {'boot_option': 'netboot'},
})
task = task_manager.TaskManager(self.context, node.uuid)
self.service._do_adoption(task)
@ -7365,7 +7406,7 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.assertFalse(mock_prepare.called)
self.assertFalse(mock_take_over.called)
self.assertFalse(mock_start_console.called)
self.assertTrue(mock_boot_validate.called)
mock_boot_validate.assert_called_once_with(task.driver.boot, task)
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
autospec=True)

5
releasenotes/notes/adopt-validation-7249ceb57016f0e4.yaml Normal file
View File

@ -0,0 +1,5 @@
---
fixes:
- |
No longer validates boot interface parameters when adopting a node that
uses local boot.