From 65b5ac6a7a985dd430614be03d168d5a7921ab2c Mon Sep 17 00:00:00 2001 From: Raphael Glon Date: Tue, 26 Nov 2019 19:40:26 +0100 Subject: [PATCH] Software RAID: Pass the boot mode to the IPA Pass the desired target boot mode to the IPA which is needed to prepare the partitions and bootloader accordingly. Change-Id: I4ca1bd781ae622535ced7b2d9ff23ff952d56acf Story: #2006379 Task: #37636 Depends-On: https://review.opendev.org/#/c/696156/ --- ironic/drivers/modules/agent_base.py | 7 ++- ironic/drivers/modules/agent_client.py | 41 +++++++++++--- .../unit/drivers/modules/test_agent_base.py | 56 ++++++++++++++----- .../unit/drivers/modules/test_agent_client.py | 7 ++- ...tware-raid-with-uefi-5b88e6c5af9ea743.yaml | 4 ++ 5 files changed, 91 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/software-raid-with-uefi-5b88e6c5af9ea743.yaml diff --git a/ironic/drivers/modules/agent_base.py b/ironic/drivers/modules/agent_base.py index 70ea04411b..eadd2e0b40 100644 --- a/ironic/drivers/modules/agent_base.py +++ b/ironic/drivers/modules/agent_base.py @@ -1013,6 +1013,9 @@ class AgentDeployMixin(HeartbeatMixin): on encountering error while setting the boot device on the node. """ node = task.node + # Almost never taken into account on agent side, just used for softraid + # Can be useful with whole_disk_images + target_boot_mode = boot_mode_utils.get_boot_mode(task.node) LOG.debug('Configuring local boot for node %s', node.uuid) # If the target RAID configuration is set to 'software' for the @@ -1067,7 +1070,9 @@ class AgentDeployMixin(HeartbeatMixin): result = self._client.install_bootloader( node, root_uuid=root_uuid, efi_system_part_uuid=efi_system_part_uuid, - prep_boot_part_uuid=prep_boot_part_uuid) + prep_boot_part_uuid=prep_boot_part_uuid, + target_boot_mode=target_boot_mode + ) if result['command_status'] == 'FAILED': if not whole_disk_image: msg = (_("Failed to install a bootloader when " diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py index 2644fa46d2..473c340988 100644 --- a/ironic/drivers/modules/agent_client.py +++ b/ironic/drivers/modules/agent_client.py @@ -259,12 +259,14 @@ class AgentClient(object): wait=True) @METRICS.timer('AgentClient.install_bootloader') - def install_bootloader(self, node, root_uuid, efi_system_part_uuid=None, + def install_bootloader(self, node, root_uuid, target_boot_mode, + efi_system_part_uuid=None, prep_boot_part_uuid=None): """Install a boot loader on the image. :param node: A node object. :param root_uuid: The UUID of the root partition. + :param target_boot_mode: The target deployment boot mode. :param efi_system_part_uuid: The UUID of the efi system partition where the bootloader will be installed to, only used for uefi boot mode. @@ -279,7 +281,9 @@ class AgentClient(object): """ params = {'root_uuid': root_uuid, 'efi_system_part_uuid': efi_system_part_uuid, - 'prep_boot_part_uuid': prep_boot_part_uuid} + 'prep_boot_part_uuid': prep_boot_part_uuid, + 'target_boot_mode': target_boot_mode + } # NOTE(TheJulia): This command explicitly sends a larger timeout # factor to the _command call such that the agent ramdisk has enough @@ -289,11 +293,34 @@ class AgentClient(object): # compatible. We could at least begin to delineate the commands apart # over the next cycle or two so we don't need a command timeout # extension factor. - return self._command(node=node, - method='image.install_bootloader', - params=params, - wait=True, - command_timeout_factor=2) + try: + return self._command(node=node, + method='image.install_bootloader', + params=params, + wait=True, + command_timeout_factor=2) + except exception.AgentAPIError: + # NOTE(arne_wiebalck): If we require to pass 'uefi' as the boot + # mode, but find that the IPA does not yet support the additional + # 'target_boot_mode' parameter, we need to fail. For 'bios' boot + # mode on the other hand we can retry without the parameter, + # since 'bios' is the default value the IPA will use. + if target_boot_mode == 'uefi': + LOG.error('Unable to pass UEFI boot mode to an out of date ' + 'agent ramdisk. Please contact the administrator ' + 'to update the ramdisk to contain an ' + 'ironic-python-agent version of at least 6.0.0.') + raise + else: + params = {'root_uuid': root_uuid, + 'efi_system_part_uuid': efi_system_part_uuid, + 'prep_boot_part_uuid': prep_boot_part_uuid + } + return self._command(node=node, + method='image.install_bootloader', + params=params, + wait=True, + command_timeout_factor=2) @METRICS.timer('AgentClient.get_clean_steps') def get_clean_steps(self, node, ports): diff --git a/ironic/tests/unit/drivers/modules/test_agent_base.py b/ironic/tests/unit/drivers/modules/test_agent_base.py index 62b2e892f6..d02b52b170 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base.py @@ -1082,7 +1082,10 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): @mock.patch.object(agent_client.AgentClient, 'install_bootloader', autospec=True) @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) - def test_configure_local_boot(self, try_set_boot_device_mock, + @mock.patch.object(boot_mode_utils, 'get_boot_mode', autospec=True, + return_value='whatever') + def test_configure_local_boot(self, boot_mode_mock, + try_set_boot_device_mock, install_bootloader_mock): install_bootloader_mock.return_value = { 'command_status': 'SUCCESS', 'command_error': None} @@ -1092,17 +1095,24 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.deploy.configure_local_boot(task, root_uuid='some-root-uuid') try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK, persistent=True) + boot_mode_mock.assert_called_once_with(task.node) install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', - efi_system_part_uuid=None, prep_boot_part_uuid=None) + efi_system_part_uuid=None, prep_boot_part_uuid=None, + target_boot_mode='whatever' + ) @mock.patch.object(agent_client.AgentClient, 'install_bootloader', autospec=True) @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) - def test_configure_local_boot_with_prep(self, try_set_boot_device_mock, + @mock.patch.object(boot_mode_utils, 'get_boot_mode', autospec=True, + return_value='whatever') + def test_configure_local_boot_with_prep(self, boot_mode_mock, + try_set_boot_device_mock, install_bootloader_mock): install_bootloader_mock.return_value = { 'command_status': 'SUCCESS', 'command_error': None} + with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: task.node.driver_internal_info['is_whole_disk_image'] = False @@ -1110,14 +1120,20 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): prep_boot_part_uuid='fake-prep') try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK, persistent=True) + boot_mode_mock.assert_called_once_with(task.node) install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', - efi_system_part_uuid=None, prep_boot_part_uuid='fake-prep') + efi_system_part_uuid=None, prep_boot_part_uuid='fake-prep', + target_boot_mode='whatever' + ) @mock.patch.object(agent_client.AgentClient, 'install_bootloader', autospec=True) @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) - def test_configure_local_boot_uefi(self, try_set_boot_device_mock, + @mock.patch.object(boot_mode_utils, 'get_boot_mode', autospec=True, + return_value='uefi') + def test_configure_local_boot_uefi(self, boot_mode_mock, + try_set_boot_device_mock, install_bootloader_mock): install_bootloader_mock.return_value = { 'command_status': 'SUCCESS', 'command_error': None} @@ -1129,10 +1145,13 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): efi_system_part_uuid='efi-system-part-uuid') try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK, persistent=True) + boot_mode_mock.assert_called_once_with(task.node) install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', efi_system_part_uuid='efi-system-part-uuid', - prep_boot_part_uuid=None) + prep_boot_part_uuid=None, + target_boot_mode='uefi' + ) @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) @mock.patch.object(agent_client.AgentClient, 'install_bootloader', @@ -1178,7 +1197,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid=None, efi_system_part_uuid='efi-system-part-uuid', - prep_boot_part_uuid=None) + prep_boot_part_uuid=None, target_boot_mode='uefi') @mock.patch.object(image_service, 'GlanceImageService', autospec=True) @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) @@ -1242,7 +1261,8 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): # check if the root_uuid comes from the driver_internal_info install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid=root_uuid, - efi_system_part_uuid=None, prep_boot_part_uuid=None) + efi_system_part_uuid=None, prep_boot_part_uuid=None, + target_boot_mode='bios') try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK, persistent=True) @@ -1324,8 +1344,11 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): autospec=True) @mock.patch.object(agent_client.AgentClient, 'install_bootloader', autospec=True) + @mock.patch.object(boot_mode_utils, 'get_boot_mode', autospec=True, + return_value='whatever') def test_configure_local_boot_boot_loader_install_fail( - self, install_bootloader_mock, collect_logs_mock): + self, boot_mode_mock, install_bootloader_mock, + collect_logs_mock): install_bootloader_mock.return_value = { 'command_status': 'FAILED', 'command_error': 'boom'} self.node.provision_state = states.DEPLOYING @@ -1337,9 +1360,12 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.assertRaises(exception.InstanceDeployFailure, self.deploy.configure_local_boot, task, root_uuid='some-root-uuid') + boot_mode_mock.assert_called_once_with(task.node) install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', - efi_system_part_uuid=None, prep_boot_part_uuid=None) + efi_system_part_uuid=None, prep_boot_part_uuid=None, + target_boot_mode='whatever' + ) collect_logs_mock.assert_called_once_with(mock.ANY, task.node) self.assertEqual(states.DEPLOYFAIL, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) @@ -1349,9 +1375,11 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) @mock.patch.object(agent_client.AgentClient, 'install_bootloader', autospec=True) + @mock.patch.object(boot_mode_utils, 'get_boot_mode', autospec=True, + return_value='whatever') def test_configure_local_boot_set_boot_device_fail( - self, install_bootloader_mock, try_set_boot_device_mock, - collect_logs_mock): + self, boot_mode_mock, install_bootloader_mock, + try_set_boot_device_mock, collect_logs_mock): install_bootloader_mock.return_value = { 'command_status': 'SUCCESS', 'command_error': None} try_set_boot_device_mock.side_effect = RuntimeError('error') @@ -1365,9 +1393,11 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.deploy.configure_local_boot, task, root_uuid='some-root-uuid', prep_boot_part_uuid=None) + boot_mode_mock.assert_called_once_with(task.node) install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', - efi_system_part_uuid=None, prep_boot_part_uuid=None) + efi_system_part_uuid=None, prep_boot_part_uuid=None, + target_boot_mode='whatever') try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK, persistent=True) collect_logs_mock.assert_called_once_with(mock.ANY, task.node) diff --git a/ironic/tests/unit/drivers/modules/test_agent_client.py b/ironic/tests/unit/drivers/modules/test_agent_client.py index 790425e1e4..248f508e6d 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_client.py +++ b/ironic/tests/unit/drivers/modules/test_agent_client.py @@ -276,11 +276,12 @@ class TestAgentClient(base.TestCase): self.client._command = mock.MagicMock(spec_set=[]) params = {'root_uuid': root_uuid, 'efi_system_part_uuid': efi_system_part_uuid, - 'prep_boot_part_uuid': prep_boot_part_uuid} + 'prep_boot_part_uuid': prep_boot_part_uuid, + 'target_boot_mode': 'hello'} self.client.install_bootloader( self.node, root_uuid, efi_system_part_uuid=efi_system_part_uuid, - prep_boot_part_uuid=prep_boot_part_uuid) + prep_boot_part_uuid=prep_boot_part_uuid, target_boot_mode='hello') self.client._command.assert_called_once_with( command_timeout_factor=2, node=self.node, method='image.install_bootloader', params=params, @@ -290,7 +291,7 @@ class TestAgentClient(base.TestCase): self._test_install_bootloader(root_uuid='fake-root-uuid', efi_system_part_uuid='fake-efi-uuid') - def test_install_bootloaderi_with_prep(self): + def test_install_bootloader_with_prep(self): self._test_install_bootloader(root_uuid='fake-root-uuid', efi_system_part_uuid='fake-efi-uuid', prep_boot_part_uuid='fake-prep-uuid') diff --git a/releasenotes/notes/software-raid-with-uefi-5b88e6c5af9ea743.yaml b/releasenotes/notes/software-raid-with-uefi-5b88e6c5af9ea743.yaml new file mode 100644 index 0000000000..179d2db1ab --- /dev/null +++ b/releasenotes/notes/software-raid-with-uefi-5b88e6c5af9ea743.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Adds support for bootable software RAID with UEFI boot mode.