openstack
/
ironic
Code Issues Proposed changes

Use the new extension call for getting partition UUIDs 

This is more efficient (one call instead of up to three) and easier
to debug and understand.

Also remove a bogus warning when no root UUID is returned for whole
disk images. We don't strictly needed, nor do we actually return
anything meaningful.

Change-Id: Ib9ca43b8fe86481179b3045a26e6e3b500f87511
Depends-On: https://review.opendev.org/731742
This commit is contained in:
Dmitry Tantsur 2020-05-29 14:44:36 +02:00
parent c25921f90a
commit 2509d282a3
3 changed files with 137 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
ironic/drivers/modules/agent.py
View File

@ -260,6 +260,7 @@ class AgentDeployMixin(agent_base.AgentDeployMixin):
task.process_event('wait')
# TODO(dtantsur): remove in W
def _get_uuid_from_result(self, task, type_uuid):
command = self._client.get_commands_status(task.node)[-1]
@ -319,39 +320,47 @@ class AgentDeployMixin(agent_base.AgentDeployMixin):
# ppc64* hardware we need to provide the 'PReP_Boot_partition_uuid' to
# direct where the bootloader should be installed.
driver_internal_info = task.node.driver_internal_info
root_uuid = self._get_uuid_from_result(task, 'root_uuid')
try:
partition_uuids = self._client.get_partition_uuids(node).get(
'command_result') or {}
root_uuid = partition_uuids.get('root uuid')
except exception.AgentAPIError:
# TODO(dtantsur): remove in W
LOG.warning('Old ironic-python-agent detected, please update '
'to Victoria or newer')
partition_uuids = None
root_uuid = self._get_uuid_from_result(task, 'root_uuid')
if root_uuid:
driver_internal_info['root_uuid_or_disk_id'] = root_uuid
task.node.driver_internal_info = driver_internal_info
task.node.save()
elif iwdi and CONF.agent.manage_agent_boot:
# IPA version less than 3.1.0 will not return root_uuid for
# whole disk image. Also IPA version introduced a requirement
# for hexdump utility that may not be always available. Need to
# fall back to older behavior for the same.
LOG.warning("With the deploy ramdisk based on Ironic Python Agent "
"version 3.1.0 and beyond, the drivers using "
"`direct` deploy interface performs `netboot` or "
"`local` boot for whole disk image based on value "
"of boot option setting. When you upgrade Ironic "
"Python Agent in your deploy ramdisk, ensure that "
"boot option is set appropriately for the node %s. "
"The boot option can be set using configuration "
"`[deploy]/default_boot_option` or as a `boot_option` "
"capability in node's `properties['capabilities']`. "
"Also please note that this functionality requires "
"`hexdump` command in the ramdisk.", node.uuid)
elif not iwdi:
LOG.error('No root UUID returned from the ramdisk for node '
'%(node)s, the deploy will likely fail. Partition '
'UUIDs are %(uuids)s',
{'node': node.uuid, 'uuid': partition_uuids})
efi_sys_uuid = None
if not iwdi:
if boot_mode_utils.get_boot_mode(node) == 'uefi':
efi_sys_uuid = (self._get_uuid_from_result(task,
'efi_system_partition_uuid'))
# TODO(dtantsur): remove in W
if partition_uuids is None:
efi_sys_uuid = (self._get_uuid_from_result(task,
'efi_system_partition_uuid'))
else:
efi_sys_uuid = partition_uuids.get(
'efi system partition uuid')
prep_boot_part_uuid = None
if cpu_arch is not None and cpu_arch.startswith('ppc64'):
prep_boot_part_uuid = (self._get_uuid_from_result(task,
'PReP_Boot_partition_uuid'))
# TODO(dtantsur): remove in W
if partition_uuids is None:
prep_boot_part_uuid = (self._get_uuid_from_result(task,
'PReP_Boot_partition_uuid'))
else:
prep_boot_part_uuid = partition_uuids.get(
'PReP Boot partition uuid')
LOG.info('Image successfully written to node %s', node.uuid)

16
ironic/drivers/modules/agent_client.py
View File

@ -467,6 +467,22 @@ class AgentClient(object):
method='deploy.execute_deploy_step',
params=params)
@METRICS.timer('AgentClient.get_partition_uuids')
def get_partition_uuids(self, node):
"""Get deploy steps from agent.
:param node: A node object.
:raises: IronicException when failed to issue the request or there was
a malformed response from the agent.
:raises: AgentAPIError when agent failed to execute specified command.
:returns: A dict containing command response from agent.
"""
return self._command(node=node,
method='standby.get_partition_uuids',
params={},
wait=True)
@METRICS.timer('AgentClient.power_off')
def power_off(self, node):
"""Soft powers off the bare metal node by shutting down ramdisk OS.

115
ironic/tests/unit/drivers/modules/test_agent.py
View File

@ -1280,7 +1280,7 @@ class TestAgentDeploy(db_base.DbTestCase):
@mock.patch.object(deploy_utils, 'remove_http_instance_symlink',
autospec=True)
@mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True)
@mock.patch.object(agent.AgentDeployMixin, '_get_uuid_from_result',
@mock.patch.object(agent_client.AgentClient, 'get_partition_uuids',
autospec=True)
@mock.patch.object(manager_utils, 'node_power_action', autospec=True)
@mock.patch.object(fake.FakePower, 'get_power_state',
@ -1299,7 +1299,7 @@ class TestAgentDeploy(db_base.DbTestCase):
self.config(manage_agent_boot=True, group='agent')
self.config(image_download_source='http', group='agent')
check_deploy_mock.return_value = None
uuid_mock.return_value = None
uuid_mock.return_value = {}
self.node.provision_state = states.DEPLOYWAIT
self.node.target_provision_state = states.ACTIVE
self.node.save()
@ -1310,12 +1310,10 @@ class TestAgentDeploy(db_base.DbTestCase):
task.node.driver_internal_info['is_whole_disk_image'] = True
task.driver.deploy.reboot_to_instance(task)
check_deploy_mock.assert_called_once_with(mock.ANY, task.node)
uuid_mock.assert_called_once_with(mock.ANY, task, 'root_uuid')
uuid_mock.assert_called_once_with(mock.ANY, task.node)
self.assertNotIn('root_uuid_or_disk_id',
task.node.driver_internal_info)
self.assertTrue(log_mock.called)
self.assertIn("Ironic Python Agent version 3.1.0 and beyond",
log_mock.call_args[0][0])
self.assertFalse(log_mock.called)
prepare_instance_mock.assert_called_once_with(mock.ANY, task,
None, None, None)
power_off_mock.assert_called_once_with(task.node)
@ -1333,7 +1331,7 @@ class TestAgentDeploy(db_base.DbTestCase):
autospec=True)
@mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True)
@mock.patch.object(manager_utils, 'node_set_boot_device', autospec=True)
@mock.patch.object(agent.AgentDeployMixin, '_get_uuid_from_result',
@mock.patch.object(agent_client.AgentClient, 'get_partition_uuids',
autospec=True)
@mock.patch.object(manager_utils, 'node_power_action', autospec=True)
@mock.patch.object(fake.FakePower, 'get_power_state',
@ -1351,7 +1349,7 @@ class TestAgentDeploy(db_base.DbTestCase):
resume_mock):
self.config(manage_agent_boot=False, group='agent')
check_deploy_mock.return_value = None
uuid_mock.return_value = None
uuid_mock.return_value = {}
self.node.provision_state = states.DEPLOYWAIT
self.node.target_provision_state = states.ACTIVE
self.node.save()
@ -1362,7 +1360,7 @@ class TestAgentDeploy(db_base.DbTestCase):
task.node.driver_internal_info['is_whole_disk_image'] = True
task.driver.deploy.reboot_to_instance(task)
check_deploy_mock.assert_called_once_with(mock.ANY, task.node)
uuid_mock.assert_called_once_with(mock.ANY, task, 'root_uuid')
uuid_mock.assert_called_once_with(mock.ANY, task.node)
self.assertNotIn('root_uuid_or_disk_id',
task.node.driver_internal_info)
self.assertFalse(log_mock.called)
@ -1383,7 +1381,7 @@ class TestAgentDeploy(db_base.DbTestCase):
@mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True)
@mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy',
autospec=True)
@mock.patch.object(agent.AgentDeployMixin, '_get_uuid_from_result',
@mock.patch.object(agent_client.AgentClient, 'get_partition_uuids',
autospec=True)
@mock.patch.object(manager_utils, 'node_power_action', autospec=True)
@mock.patch.object(fake.FakePower, 'get_power_state',
@ -1406,7 +1404,9 @@ class TestAgentDeploy(db_base.DbTestCase):
check_deploy_mock.return_value = None
self.node.instance_info = {
'capabilities': {'boot_option': 'netboot'}}
uuid_mock.return_value = 'root_uuid'
uuid_mock.return_value = {
'command_result': {'root uuid': 'root_uuid'}
}
self.node.provision_state = states.DEPLOYWAIT
self.node.target_provision_state = states.ACTIVE
self.node.save()
@ -1420,8 +1420,7 @@ class TestAgentDeploy(db_base.DbTestCase):
task.node.driver_internal_info = driver_internal_info
task.driver.deploy.reboot_to_instance(task)
check_deploy_mock.assert_called_once_with(mock.ANY, task.node)
uuid_mock.assert_called_once_with(mock.ANY,
task, 'root_uuid')
uuid_mock.assert_called_once_with(mock.ANY, task.node)
driver_int_info = task.node.driver_internal_info
self.assertEqual('root_uuid',
driver_int_info['root_uuid_or_disk_id']),
@ -1448,6 +1447,68 @@ class TestAgentDeploy(db_base.DbTestCase):
autospec=True)
@mock.patch.object(agent.AgentDeployMixin, '_get_uuid_from_result',
autospec=True)
@mock.patch.object(agent_client.AgentClient, 'get_partition_uuids',
autospec=True)
@mock.patch.object(manager_utils, 'node_power_action', autospec=True)
@mock.patch.object(fake.FakePower, 'get_power_state',
spec=types.FunctionType)
@mock.patch.object(agent_client.AgentClient, 'power_off',
spec=types.FunctionType)
@mock.patch.object(agent.AgentDeployMixin, 'prepare_instance_to_boot',
autospec=True)
@mock.patch('ironic.drivers.modules.agent.AgentDeployMixin'
'.check_deploy_success', autospec=True)
def test_reboot_to_instance_partition_image_compat(
self, check_deploy_mock, prepare_instance_mock, power_off_mock,
get_power_state_mock, node_power_action_mock, uuid_mock,
old_uuid_mock, boot_mode_mock, log_mock,
power_on_node_if_needed_mock, resume_mock):
check_deploy_mock.return_value = None
self.node.instance_info = {
'capabilities': {'boot_option': 'netboot'}}
uuid_mock.side_effect = exception.AgentAPIError
old_uuid_mock.return_value = 'root_uuid'
self.node.provision_state = states.DEPLOYWAIT
self.node.target_provision_state = states.ACTIVE
self.node.save()
boot_mode_mock.return_value = 'bios'
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
power_on_node_if_needed_mock.return_value = None
get_power_state_mock.return_value = states.POWER_OFF
driver_internal_info = task.node.driver_internal_info
driver_internal_info['is_whole_disk_image'] = False
task.node.driver_internal_info = driver_internal_info
task.driver.deploy.reboot_to_instance(task)
check_deploy_mock.assert_called_once_with(mock.ANY, task.node)
uuid_mock.assert_called_once_with(mock.ANY, task.node)
old_uuid_mock.assert_called_once_with(mock.ANY, task, 'root_uuid')
driver_int_info = task.node.driver_internal_info
self.assertEqual('root_uuid',
driver_int_info['root_uuid_or_disk_id']),
boot_mode_mock.assert_called_once_with(task.node)
self.assertTrue(log_mock.called)
prepare_instance_mock.assert_called_once_with(mock.ANY,
task,
'root_uuid',
None, None)
power_off_mock.assert_called_once_with(task.node)
get_power_state_mock.assert_called_once_with(task)
node_power_action_mock.assert_called_once_with(
task, states.POWER_ON)
self.assertEqual(states.DEPLOYWAIT, task.node.provision_state)
self.assertEqual(states.ACTIVE, task.node.target_provision_state)
resume_mock.assert_called_once_with(task)
@mock.patch.object(manager_utils, 'notify_conductor_resume_deploy',
autospec=True)
@mock.patch.object(manager_utils, 'power_on_node_if_needed',
autospec=True)
@mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True)
@mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy',
autospec=True)
@mock.patch.object(agent_client.AgentClient, 'get_partition_uuids',
autospec=True)
@mock.patch.object(manager_utils, 'node_power_action', autospec=True)
@mock.patch.object(fake.FakePower, 'get_power_state',
spec=types.FunctionType)
@ -1463,7 +1524,12 @@ class TestAgentDeploy(db_base.DbTestCase):
node_power_action_mock, uuid_mock, boot_mode_mock, log_mock,
power_on_node_if_needed_mock, resume_mock):
check_deploy_mock.return_value = None
uuid_mock.side_effect = ['root_uuid', 'prep_boot_part_uuid']
uuid_mock.return_value = {
'command_result': {
'root uuid': 'root_uuid',
'PReP Boot partition uuid': 'prep_boot_part_uuid',
}
}
self.node.provision_state = states.DEPLOYWAIT
self.node.target_provision_state = states.ACTIVE
self.node.save()
@ -1487,10 +1553,7 @@ class TestAgentDeploy(db_base.DbTestCase):
driver_int_info = task.node.driver_internal_info
self.assertEqual('root_uuid',
driver_int_info['root_uuid_or_disk_id']),
uuid_mock_calls = [
mock.call(mock.ANY, task, 'root_uuid'),
mock.call(mock.ANY, task, 'PReP_Boot_partition_uuid')]
uuid_mock.assert_has_calls(uuid_mock_calls)
uuid_mock.assert_called_once_with(mock.ANY, task.node)
boot_mode_mock.assert_called_once_with(task.node)
self.assertFalse(log_mock.called)
prepare_instance_mock.assert_called_once_with(
@ -1504,7 +1567,7 @@ class TestAgentDeploy(db_base.DbTestCase):
@mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True)
@mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True)
@mock.patch.object(agent.AgentDeployMixin, '_get_uuid_from_result',
@mock.patch.object(agent_client.AgentClient, 'get_partition_uuids',
autospec=True)
@mock.patch.object(manager_utils, 'node_power_action', autospec=True)
@mock.patch.object(fake.FakePower, 'get_power_state',
@ -1544,7 +1607,7 @@ class TestAgentDeploy(db_base.DbTestCase):
@mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True)
@mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy',
autospec=True)
@mock.patch.object(agent.AgentDeployMixin, '_get_uuid_from_result',
@mock.patch.object(agent_client.AgentClient, 'get_partition_uuids',
autospec=True)
@mock.patch.object(manager_utils, 'node_power_action', autospec=True)
@mock.patch.object(fake.FakePower, 'get_power_state',
@ -1565,7 +1628,12 @@ class TestAgentDeploy(db_base.DbTestCase):
power_on_node_if_needed_mock,
resume_mock):
check_deploy_mock.return_value = None
uuid_mock.side_effect = ['root_uuid', 'efi_uuid']
uuid_mock.return_value = {
'command_result': {
'root uuid': 'root_uuid',
'efi system partition uuid': 'efi_uuid',
}
}
self.node.provision_state = states.DEPLOYWAIT
self.node.target_provision_state = states.ACTIVE
self.node.save()
@ -1586,10 +1654,7 @@ class TestAgentDeploy(db_base.DbTestCase):
driver_int_info = task.node.driver_internal_info
self.assertEqual('root_uuid',
driver_int_info['root_uuid_or_disk_id']),
uuid_mock_calls = [
mock.call(mock.ANY, task, 'root_uuid'),
mock.call(mock.ANY, task, 'efi_system_partition_uuid')]
uuid_mock.assert_has_calls(uuid_mock_calls)
uuid_mock.assert_called_once_with(mock.ANY, task.node)
boot_mode_mock.assert_called_once_with(task.node)
self.assertFalse(log_mock.called)
prepare_instance_mock.assert_called_once_with(