From 2a6b5c14d5c45728fc63fc89b501beb74a64ca60 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Mon, 9 Dec 2019 16:45:53 +0000 Subject: [PATCH] Decompose the core deploy step of the direct deploy This change decomposes the current deploy step of the direct deploy into multiple deploy steps: * deploy (priority 100) * write_image (priority 80) * prepare_instance_boot (priority 60) Note that this patch breaks backwards compatibility with 3rd party drivers that inherit AgentDeploy rather than the base agent class. Co-Authored-By: Dmitry Tantsur Change-Id: Ief586473aca0e22b74efe83ef70c354fd5df17bf Story: 2006963 Task: 37778 --- doc/source/admin/node-deployment.rst | 29 +- ironic/drivers/modules/agent.py | 111 +++--- ironic/drivers/modules/agent_base.py | 13 +- .../tests/unit/drivers/modules/test_agent.py | 354 +++++------------- .../unit/drivers/modules/test_agent_base.py | 17 + .../direct-deploy-steps-36486987156017d7.yaml | 17 + 6 files changed, 208 insertions(+), 333 deletions(-) create mode 100644 releasenotes/notes/direct-deploy-steps-36486987156017d7.yaml diff --git a/doc/source/admin/node-deployment.rst b/doc/source/admin/node-deployment.rst index 03dfbfec2a..d535a4a75a 100644 --- a/doc/source/admin/node-deployment.rst +++ b/doc/source/admin/node-deployment.rst @@ -68,7 +68,34 @@ Accordingly, the following priority ranges can be used for custom deploy steps: 1 to 19 Any steps that are run when the user instance is already running. -.. note:: Priorities 60 to 99 are currently reserved and should not be used. +Direct deploy +^^^^^^^^^^^^^ + +The :ref:`direct-deploy` interface splits the ``deploy.deploy`` step into: + + +``deploy.deploy`` (priority 100) + In this step the node is booted using a provisioning image. +``deploy.write_image`` (priority 80) + A combination of an out-of-band and in-band step that downloads and writes + the image to the node. The step is executed asynchronously by the ramdisk. +``deploy.prepare_instance_boot`` (priority 60) + In this step the boot device is configured and the bootloader is installed. + +Additional priority ranges can be used for custom deploy steps: + +81 to 99 + In-band deploy steps to run before the image is written. +61 to 79 + In-band deploy steps to run after the image is written but before the + bootloader is installed. + +Other deploy interfaces +^^^^^^^^^^^^^^^^^^^^^^^ + +Priorities 60 to 99 are currently reserved and should not be used. + +.. TODO(dtantsur): update once iscsi and ansible are converted Writing a Deploy Step --------------------- diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index d2b8ba417e..601dd7d911 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -31,7 +31,6 @@ from ironic.conductor import utils as manager_utils from ironic.conf import CONF from ironic.drivers import base from ironic.drivers.modules import agent_base -from ironic.drivers.modules import agent_client from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import deploy_utils @@ -170,34 +169,14 @@ def validate_http_provisioning_configuration(node): class AgentDeployMixin(agent_base.AgentDeployMixin): - @METRICS.timer('AgentDeployMixin.deploy_has_started') - def deploy_has_started(self, task): - commands = self._client.get_commands_status(task.node) + has_decomposed_deploy_steps = True - for command in commands: - if command['command_name'] == 'prepare_image': - # deploy did start at some point - return True - return False - - @METRICS.timer('AgentDeployMixin.deploy_is_done') - def deploy_is_done(self, task): - commands = self._client.get_commands_status(task.node) - if not commands: - return False - - try: - last_command = next(cmd for cmd in reversed(commands) - if cmd['command_name'] == 'prepare_image') - except StopIteration: - return False - else: - return last_command['command_status'] != 'RUNNING' - - @METRICS.timer('AgentDeployMixin.continue_deploy') + @METRICS.timer('AgentDeployMixin.write_image') + @base.deploy_step(priority=80) @task_manager.require_exclusive_lock - def continue_deploy(self, task): - task.process_event('resume') + def write_image(self, task): + if not task.driver.storage.should_write_image(task): + return node = task.node image_source = node.instance_info.get('image_source') LOG.debug('Continuing deploy for node %(node)s with image %(img)s', @@ -251,10 +230,33 @@ class AgentDeployMixin(agent_base.AgentDeployMixin): if disk_label is not None: image_info['disk_label'] = disk_label - # Tell the client to download and write the image with the given args - self._client.prepare_image(node, image_info) + has_write_image = agent_base.find_step( + task, 'deploy', 'deploy', 'write_image') is not None + if not has_write_image: + LOG.warning('The agent on node %s does not have the deploy ' + 'step deploy.write_image, using the deprecated ' + 'synchronous fall-back', task.node.uuid) - task.process_event('wait') + if self.has_decomposed_deploy_steps and has_write_image: + configdrive = node.instance_info.get('configdrive') + # Now switch into the corresponding in-band deploy step and let the + # result be polled normally. + new_step = {'interface': 'deploy', + 'step': 'write_image', + 'args': {'image_info': image_info, + 'configdrive': configdrive}} + return agent_base.execute_step(task, new_step, 'deploy', + client=self._client) + else: + # TODO(dtantsur): remove in W + command = self._client.prepare_image(node, image_info, wait=True) + if command['command_status'] == 'FAILED': + # TODO(jimrollenhagen) power off if using neutron dhcp to + # align with pxe driver? + msg = (_('node %(node)s command status errored: %(error)s') % + {'node': node.uuid, 'error': command['command_error']}) + LOG.error(msg) + deploy_utils.set_failed_state(task, msg) # TODO(dtantsur): remove in W def _get_uuid_from_result(self, task, type_uuid): @@ -278,29 +280,18 @@ class AgentDeployMixin(agent_base.AgentDeployMixin): return return result - @METRICS.timer('AgentDeployMixin.check_deploy_success') - def check_deploy_success(self, node): - # should only ever be called after we've validated that - # the prepare_image command is complete - command = self._client.get_commands_status(node)[-1] - if command['command_status'] == 'FAILED': - return agent_client.get_command_error(command) + @METRICS.timer('AgentDeployMixin.prepare_instance_boot') + @base.deploy_step(priority=60) + @task_manager.require_exclusive_lock + def prepare_instance_boot(self, task): + if not task.driver.storage.should_write_image(task): + task.driver.boot.prepare_instance(task) + # Move straight to the final steps + return - @METRICS.timer('AgentDeployMixin.reboot_to_instance') - def reboot_to_instance(self, task): - task.process_event('resume') node = task.node iwdi = task.node.driver_internal_info.get('is_whole_disk_image') cpu_arch = task.node.properties.get('cpu_arch') - error = self.check_deploy_success(node) - if error is not None: - # TODO(jimrollenhagen) power off if using neutron dhcp to - # align with pxe driver? - msg = (_('node %(node)s command status errored: %(error)s') % - {'node': node.uuid, 'error': error}) - LOG.error(msg) - deploy_utils.set_failed_state(task, msg) - return # If `boot_option` is set to `netboot`, PXEBoot.prepare_instance() # would need root_uuid of the whole disk image to add it into the @@ -376,8 +367,6 @@ class AgentDeployMixin(agent_base.AgentDeployMixin): if CONF.agent.image_download_source == 'http': deploy_utils.remove_http_instance_symlink(task.node.uuid) - self.reboot_and_finish_deploy(task) - class AgentDeploy(AgentDeployMixin, agent_base.AgentBaseMixin, base.DeployInterface): @@ -481,13 +470,10 @@ class AgentDeploy(AgentDeployMixin, agent_base.AgentBaseMixin, :returns: status of the deploy. One of ironic.common.states. """ if manager_utils.is_fast_track(task): + # NOTE(mgoddard): For fast track we can skip this step and proceed + # immediately to the next deploy step. LOG.debug('Performing a fast track deployment for %(node)s.', {'node': task.node.uuid}) - # Update the database for the API and the task tracking resumes - # the state machine state going from DEPLOYWAIT -> DEPLOYING - task.process_event('wait') - self.continue_deploy(task) - return states.DEPLOYWAIT elif task.driver.storage.should_write_image(task): # Check if the driver has already performed a reboot in a previous # deploy step. @@ -498,19 +484,6 @@ class AgentDeploy(AgentDeployMixin, agent_base.AgentBaseMixin, task.node.driver_internal_info = info task.node.save() return states.DEPLOYWAIT - else: - # TODO(TheJulia): At some point, we should de-dupe this code - # as it is nearly identical to the iscsi deploy interface. - # This is not being done now as it is expected to be - # refactored in the near future. - manager_utils.node_power_action(task, states.POWER_OFF) - with manager_utils.power_state_for_network_configuration(task): - task.driver.network.remove_provisioning_network(task) - task.driver.network.configure_tenant_networks(task) - task.driver.boot.prepare_instance(task) - manager_utils.node_power_action(task, states.POWER_ON) - LOG.info('Deployment to node %s done', task.node.uuid) - return None @METRICS.timer('AgentDeploy.prepare') @task_manager.require_exclusive_lock diff --git a/ironic/drivers/modules/agent_base.py b/ironic/drivers/modules/agent_base.py index 4ab403312b..8bd995ed45 100644 --- a/ironic/drivers/modules/agent_base.py +++ b/ironic/drivers/modules/agent_base.py @@ -336,6 +336,13 @@ def get_steps(task, step_type, interface=None, override_priorities=None): return steps +def find_step(task, step_type, interface, name): + """Find the given in-band step.""" + steps = get_steps(task, step_type, interface) + return conductor_steps.find_step( + steps, {'interface': interface, 'step': name}) + + def _raise(step_type, msg): assert step_type in ('clean', 'deploy') exc = (exception.NodeCleaningFailure if step_type == 'clean' @@ -343,18 +350,20 @@ def _raise(step_type, msg): raise exc(msg) -def execute_step(task, step, step_type): +def execute_step(task, step, step_type, client=None): """Execute a clean or deploy step asynchronously on the agent. :param task: a TaskManager object containing the node :param step: a step dictionary to execute :param step_type: 'clean' or 'deploy' + :param client: agent client (if available) :raises: NodeCleaningFailure (clean step) or InstanceDeployFailure (deploy step) if the agent does not return a command status. :returns: states.CLEANWAIT/DEPLOYWAIT to signify the step will be completed async """ - client = _get_client() + if client is None: + client = _get_client() ports = objects.Port.list_by_node_id( task.context, task.node.id) call = getattr(client, 'execute_%s_step' % step_type) diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index d1c39256b2..b841ab988e 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -35,7 +35,6 @@ from ironic.drivers.modules.network import flat as flat_network from ironic.drivers.modules.network import neutron as neutron_network from ironic.drivers.modules import pxe from ironic.drivers.modules.storage import noop as noop_storage -from ironic.drivers import utils as driver_utils from ironic.tests.unit.db import base as db_base from ironic.tests.unit.db import utils as db_utils from ironic.tests.unit.objects import utils as object_utils @@ -452,11 +451,11 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertNotIn( 'deployment_reboot', task.node.driver_internal_info) - @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', autospec=True) - def test_deploy_storage_should_write_image_false(self, mock_write, - mock_pxe_instance): + def test_deploy_storage_should_write_image_false( + self, mock_write, mock_power): mock_write.return_value = False self.node.provision_state = states.DEPLOYING self.node.deploy_step = { @@ -466,7 +465,7 @@ class TestAgentDeploy(db_base.DbTestCase): self.context, self.node['uuid'], shared=False) as task: driver_return = self.driver.deploy(task) self.assertIsNone(driver_return) - self.assertTrue(mock_pxe_instance.called) + self.assertFalse(mock_power.called) @mock.patch.object(agent_client.AgentClient, 'prepare_image', autospec=True) @@ -478,26 +477,15 @@ class TestAgentDeploy(db_base.DbTestCase): mock_is_fast_track.return_value = True self.node.target_provision_state = states.ACTIVE self.node.provision_state = states.DEPLOYING - test_temp_url = 'http://image' - expected_image_info = { - 'urls': [test_temp_url], - 'id': 'fake-image', - 'node_uuid': self.node.uuid, - 'checksum': 'checksum', - 'disk_format': 'qcow2', - 'container_format': 'bare', - 'stream_raw_images': CONF.agent.stream_raw_images, - } self.node.save() with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: - self.assertEqual(states.DEPLOYWAIT, self.driver.deploy(task)) + result = self.driver.deploy(task) + self.assertIsNone(result) self.assertFalse(power_mock.called) self.assertFalse(mock_pxe_instance.called) - task.node.refresh() - prepare_image_mock.assert_called_with(mock.ANY, task.node, - expected_image_info) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) + self.assertFalse(prepare_image_mock.called) + self.assertEqual(states.DEPLOYING, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) @@ -1151,13 +1139,21 @@ class TestAgentDeploy(db_base.DbTestCase): tear_down_cleaning_mock.assert_called_once_with( task, manage_boot=False) - def _test_continue_deploy(self, additional_driver_info=None, - additional_expected_image_info=None): + def _test_write_image(self, additional_driver_info=None, + additional_expected_image_info=None, + compat=False): self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE driver_info = self.node.driver_info driver_info.update(additional_driver_info or {}) self.node.driver_info = driver_info + if not compat: + step = {'step': 'write_image', 'interface': 'deploy'} + dii = self.node.driver_internal_info + dii['agent_cached_deploy_steps'] = { + 'deploy': [step], + } + self.node.driver_internal_info = dii self.node.save() test_temp_url = 'http://image' expected_image_info = { @@ -1171,24 +1167,34 @@ class TestAgentDeploy(db_base.DbTestCase): } expected_image_info.update(additional_expected_image_info or {}) - client_mock = mock.MagicMock(spec_set=['prepare_image']) + client_mock = mock.MagicMock(spec_set=['prepare_image', + 'execute_deploy_step']) with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.deploy._client = client_mock - task.driver.deploy.continue_deploy(task) + task.driver.deploy.write_image(task) - client_mock.prepare_image.assert_called_with(task.node, - expected_image_info) + if compat: + client_mock.prepare_image.assert_called_with( + task.node, expected_image_info, wait=True) + else: + step['args'] = {'image_info': expected_image_info, + 'configdrive': None} + client_mock.execute_deploy_step.assert_called_once_with( + step, task.node, mock.ANY) self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) - def test_continue_deploy(self): - self._test_continue_deploy() + def test_write_image(self): + self._test_write_image() - def test_continue_deploy_with_proxies(self): - self._test_continue_deploy( + def test_write_image_compat(self): + self._test_write_image(compat=True) + + def test_write_image_with_proxies(self): + self._test_write_image( additional_driver_info={'image_https_proxy': 'https://spam.ni', 'image_http_proxy': 'spam.ni', 'image_no_proxy': '.eggs.com'}, @@ -1198,22 +1204,22 @@ class TestAgentDeploy(db_base.DbTestCase): 'no_proxy': '.eggs.com'} ) - def test_continue_deploy_with_no_proxy_without_proxies(self): - self._test_continue_deploy( + def test_write_image_with_no_proxy_without_proxies(self): + self._test_write_image( additional_driver_info={'image_no_proxy': '.eggs.com'} ) - def test_continue_deploy_image_source_is_url(self): + def test_write_image_image_source_is_url(self): instance_info = self.node.instance_info instance_info['image_source'] = 'http://example.com/woof.img' self.node.instance_info = instance_info - self._test_continue_deploy( + self._test_write_image( additional_expected_image_info={ 'id': 'woof.img' } ) - def test_continue_deploy_partition_image(self): + def test_write_image_partition_image(self): self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE i_info = self.node.instance_info @@ -1264,16 +1270,15 @@ class TestAgentDeploy(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.deploy._client = client_mock - task.driver.deploy.continue_deploy(task) + task.driver.deploy.write_image(task) client_mock.prepare_image.assert_called_with(task.node, - expected_image_info) + expected_image_info, + wait=True) self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) - @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', - autospec=True) @mock.patch.object(deploy_utils, 'remove_http_instance_symlink', autospec=True) @mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True) @@ -1281,70 +1286,55 @@ class TestAgentDeploy(db_base.DbTestCase): autospec=True) @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(self, check_deploy_mock, prepare_instance_mock, - uuid_mock, log_mock, remove_symlink_mock, - resume_mock): + def test_prepare_instance_boot(self, prepare_instance_mock, + uuid_mock, log_mock, remove_symlink_mock): 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 = {} - self.node.provision_state = states.DEPLOYWAIT + self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: 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) + task.driver.deploy.prepare_instance_boot(task) 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) prepare_instance_mock.assert_called_once_with(mock.ANY, task, None, None, None) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) + self.assertEqual(states.DEPLOYING, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) self.assertTrue(remove_symlink_mock.called) - resume_mock.assert_called_once_with(task) - @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', - 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_client.AgentClient, 'get_partition_uuids', autospec=True) @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_no_manage_agent_boot( - self, check_deploy_mock, prepare_instance_mock, uuid_mock, - bootdev_mock, log_mock, resume_mock): + def test_prepare_instance_boot_no_manage_agent_boot( + self, prepare_instance_mock, uuid_mock, + bootdev_mock, log_mock): self.config(manage_agent_boot=False, group='agent') - check_deploy_mock.return_value = None uuid_mock.return_value = {} - self.node.provision_state = states.DEPLOYWAIT + self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: 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) + task.driver.deploy.prepare_instance_boot(task) 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) self.assertFalse(prepare_instance_mock.called) bootdev_mock.assert_called_once_with(task, 'disk', persistent=True) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) + self.assertEqual(states.DEPLOYING, 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(agent.LOG, 'warning', spec_set=True, autospec=True) @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', autospec=True) @@ -1352,20 +1342,15 @@ class TestAgentDeploy(db_base.DbTestCase): autospec=True) @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(self, check_deploy_mock, - prepare_instance_mock, - uuid_mock, boot_mode_mock, - log_mock, - resume_mock): - check_deploy_mock.return_value = None + def test_prepare_instance_boot_partition_image(self, prepare_instance_mock, + uuid_mock, boot_mode_mock, + log_mock): self.node.instance_info = { 'capabilities': {'boot_option': 'netboot'}} uuid_mock.return_value = { 'command_result': {'root uuid': 'root_uuid'} } - self.node.provision_state = states.DEPLOYWAIT + self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() boot_mode_mock.return_value = 'bios' @@ -1374,8 +1359,7 @@ class TestAgentDeploy(db_base.DbTestCase): 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) + task.driver.deploy.prepare_instance_boot(task) uuid_mock.assert_called_once_with(mock.ANY, task.node) driver_int_info = task.node.driver_internal_info self.assertEqual('root_uuid', @@ -1386,12 +1370,9 @@ class TestAgentDeploy(db_base.DbTestCase): task, 'root_uuid', None, None) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) + self.assertEqual(states.DEPLOYING, 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(agent.LOG, 'warning', spec_set=True, autospec=True) @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', autospec=True) @@ -1401,17 +1382,14 @@ class TestAgentDeploy(db_base.DbTestCase): autospec=True) @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, uuid_mock, - old_uuid_mock, boot_mode_mock, log_mock, resume_mock): - check_deploy_mock.return_value = None + def test_prepare_instance_boot_partition_image_compat( + self, prepare_instance_mock, uuid_mock, + old_uuid_mock, boot_mode_mock, log_mock): 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.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() boot_mode_mock.return_value = 'bios' @@ -1420,8 +1398,7 @@ class TestAgentDeploy(db_base.DbTestCase): 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) + task.driver.deploy.prepare_instance_boot(task) 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 @@ -1433,12 +1410,9 @@ class TestAgentDeploy(db_base.DbTestCase): task, 'root_uuid', None, None) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) + self.assertEqual(states.DEPLOYING, 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(agent.LOG, 'warning', spec_set=True, autospec=True) @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', autospec=True) @@ -1446,19 +1420,16 @@ class TestAgentDeploy(db_base.DbTestCase): autospec=True) @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_localboot_ppc64( - self, check_deploy_mock, prepare_instance_mock, - uuid_mock, boot_mode_mock, log_mock, resume_mock): - check_deploy_mock.return_value = None + def test_prepare_instance_boot_partition_localboot_ppc64( + self, prepare_instance_mock, + uuid_mock, boot_mode_mock, log_mock): 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.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() @@ -1473,9 +1444,8 @@ class TestAgentDeploy(db_base.DbTestCase): properties.update(cpu_arch='ppc64le') task.node.properties = properties boot_mode_mock.return_value = 'bios' - task.driver.deploy.reboot_to_instance(task) + task.driver.deploy.prepare_instance_boot(task) - check_deploy_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']), @@ -1484,60 +1454,26 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertFalse(log_mock.called) prepare_instance_mock.assert_called_once_with( mock.ANY, task, 'root_uuid', None, 'prep_boot_part_uuid') - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) + self.assertEqual(states.DEPLOYING, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) @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_client.AgentClient, 'get_partition_uuids', - autospec=True) - @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_boot_error( - self, check_deploy_mock, prepare_instance_mock, - uuid_mock, collect_ramdisk_logs_mock, log_mock): - check_deploy_mock.return_value = "Error" - uuid_mock.return_value = None - self.node.provision_state = states.DEPLOYWAIT - self.node.target_provision_state = states.ACTIVE - self.node.save() - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - 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) - self.assertFalse(prepare_instance_mock.called) - self.assertFalse(log_mock.called) - collect_ramdisk_logs_mock.assert_called_once_with(task.node) - self.assertEqual(states.DEPLOYFAIL, task.node.provision_state) - self.assertEqual(states.ACTIVE, task.node.target_provision_state) - - @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', - 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(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_localboot(self, check_deploy_mock, - prepare_instance_mock, - uuid_mock, boot_mode_mock, - log_mock, - resume_mock): - check_deploy_mock.return_value = None + def test_prepare_instance_boot_localboot(self, prepare_instance_mock, + uuid_mock, boot_mode_mock, + log_mock): uuid_mock.return_value = { 'command_result': { 'root uuid': 'root_uuid', 'efi system partition uuid': 'efi_uuid', } } - self.node.provision_state = states.DEPLOYWAIT + self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() @@ -1549,9 +1485,8 @@ class TestAgentDeploy(db_base.DbTestCase): boot_option = {'capabilities': '{"boot_option": "local"}'} task.node.instance_info = boot_option boot_mode_mock.return_value = 'uefi' - task.driver.deploy.reboot_to_instance(task) + task.driver.deploy.prepare_instance_boot(task) - check_deploy_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']), @@ -1560,102 +1495,24 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertFalse(log_mock.called) prepare_instance_mock.assert_called_once_with( mock.ANY, task, 'root_uuid', 'efi_uuid', None) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) + self.assertEqual(states.DEPLOYING, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) - resume_mock.assert_called_once_with(task) - @mock.patch.object(agent_client.AgentClient, 'get_commands_status', + @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) + @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', autospec=True) - def test_deploy_has_started(self, mock_get_cmd): - with task_manager.acquire(self.context, self.node.uuid) as task: - mock_get_cmd.return_value = [] - self.assertFalse(task.driver.deploy.deploy_has_started(task)) - - @mock.patch.object(agent_client.AgentClient, 'get_commands_status', - autospec=True) - def test_deploy_has_started_is_done(self, mock_get_cmd): - with task_manager.acquire(self.context, self.node.uuid) as task: - mock_get_cmd.return_value = [{'command_name': 'prepare_image', - 'command_status': 'SUCCESS'}] - self.assertTrue(task.driver.deploy.deploy_has_started(task)) - - @mock.patch.object(agent_client.AgentClient, 'get_commands_status', - autospec=True) - def test_deploy_has_started_did_start(self, mock_get_cmd): - with task_manager.acquire(self.context, self.node.uuid) as task: - mock_get_cmd.return_value = [{'command_name': 'prepare_image', - 'command_status': 'RUNNING'}] - self.assertTrue(task.driver.deploy.deploy_has_started(task)) - - @mock.patch.object(agent_client.AgentClient, 'get_commands_status', - autospec=True) - def test_deploy_has_started_multiple_commands(self, mock_get_cmd): - with task_manager.acquire(self.context, self.node.uuid) as task: - mock_get_cmd.return_value = [{'command_name': 'cache_image', - 'command_status': 'SUCCESS'}, - {'command_name': 'prepare_image', - 'command_status': 'RUNNING'}] - self.assertTrue(task.driver.deploy.deploy_has_started(task)) - - @mock.patch.object(agent_client.AgentClient, 'get_commands_status', - autospec=True) - def test_deploy_has_started_other_commands(self, mock_get_cmd): - with task_manager.acquire(self.context, self.node.uuid) as task: - mock_get_cmd.return_value = [{'command_name': 'cache_image', - 'command_status': 'SUCCESS'}] - self.assertFalse(task.driver.deploy.deploy_has_started(task)) - - @mock.patch.object(agent_client.AgentClient, 'get_commands_status', - autospec=True) - def test_deploy_is_done(self, mock_get_cmd): - with task_manager.acquire(self.context, self.node.uuid) as task: - mock_get_cmd.return_value = [{'command_name': 'prepare_image', - 'command_status': 'SUCCESS'}] - self.assertTrue(task.driver.deploy.deploy_is_done(task)) - - @mock.patch.object(agent_client.AgentClient, 'get_commands_status', - autospec=True) - def test_deploy_is_done_empty_response(self, mock_get_cmd): - with task_manager.acquire(self.context, self.node.uuid) as task: - mock_get_cmd.return_value = [] - self.assertFalse(task.driver.deploy.deploy_is_done(task)) - - @mock.patch.object(agent_client.AgentClient, 'get_commands_status', - autospec=True) - def test_deploy_is_done_race(self, mock_get_cmd): - with task_manager.acquire(self.context, self.node.uuid) as task: - mock_get_cmd.return_value = [{'command_name': 'some_other_command', - 'command_status': 'SUCCESS'}] - self.assertFalse(task.driver.deploy.deploy_is_done(task)) - - @mock.patch.object(agent_client.AgentClient, 'get_commands_status', - autospec=True) - def test_deploy_is_done_still_running(self, mock_get_cmd): - with task_manager.acquire(self.context, self.node.uuid) as task: - mock_get_cmd.return_value = [{'command_name': 'prepare_image', - 'command_status': 'RUNNING'}] - self.assertFalse(task.driver.deploy.deploy_is_done(task)) - - @mock.patch.object(agent_client.AgentClient, 'get_commands_status', - autospec=True) - def test_deploy_is_done_several_results(self, mock_get_cmd): - with task_manager.acquire(self.context, self.node.uuid) as task: - mock_get_cmd.return_value = [ - {'command_name': 'prepare_image', 'command_status': 'SUCCESS'}, - {'command_name': 'other_command', 'command_status': 'SUCCESS'}, - {'command_name': 'prepare_image', 'command_status': 'RUNNING'}, - ] - self.assertFalse(task.driver.deploy.deploy_is_done(task)) - - @mock.patch.object(agent_client.AgentClient, 'get_commands_status', - autospec=True) - def test_deploy_is_done_not_the_last(self, mock_get_cmd): - with task_manager.acquire(self.context, self.node.uuid) as task: - mock_get_cmd.return_value = [ - {'command_name': 'prepare_image', 'command_status': 'SUCCESS'}, - {'command_name': 'other_command', 'command_status': 'SUCCESS'}, - ] - self.assertTrue(task.driver.deploy.deploy_is_done(task)) + def test_prepare_instance_boot_storage_should_write_image_with_smartnic( + self, mock_write, mock_pxe_instance): + mock_write.return_value = False + self.node.provision_state = states.DEPLOYING + self.node.deploy_step = { + 'step': 'deploy', 'priority': 50, 'interface': 'deploy'} + self.node.save() + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + driver_return = self.driver.prepare_instance_boot(task) + self.assertIsNone(driver_return) + self.assertTrue(mock_pxe_instance.called) @mock.patch.object(manager_utils, 'restore_power_state_if_needed', autospec=True) @@ -1753,31 +1610,6 @@ class TestAgentDeploy(db_base.DbTestCase): self.context, self.node['uuid'], shared=False) as task: self.assertEqual(0, len(task.volume_targets)) - @mock.patch.object(manager_utils, 'restore_power_state_if_needed', - autospec=True) - @mock.patch.object(manager_utils, 'power_on_node_if_needed', - autospec=True) - @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) - @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', - autospec=True) - def test_deploy_storage_should_write_image_false_with_smartnic_port( - self, mock_write, mock_pxe_instance, - power_on_node_if_needed_mock, restore_power_state_mock): - mock_write.return_value = False - self.node.provision_state = states.DEPLOYING - self.node.deploy_step = { - 'step': 'deploy', 'priority': 50, 'interface': 'deploy'} - self.node.save() - with task_manager.acquire( - self.context, self.node['uuid'], shared=False) as task: - power_on_node_if_needed_mock.return_value = states.POWER_OFF - driver_return = self.driver.deploy(task) - self.assertIsNone(driver_return) - self.assertTrue(mock_pxe_instance.called) - power_on_node_if_needed_mock.assert_called_once_with(task) - restore_power_state_mock.assert_called_once_with( - task, states.POWER_OFF) - class AgentRAIDTestCase(db_base.DbTestCase): diff --git a/ironic/tests/unit/drivers/modules/test_agent_base.py b/ironic/tests/unit/drivers/modules/test_agent_base.py index 6773ff8347..25fa4dc230 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base.py @@ -2279,6 +2279,23 @@ class StepMethodsTestCase(db_base.DbTestCase): self.context, self.node.uuid, shared=False) as task: self.assertEqual([], agent_base.get_steps(task, 'clean')) + def test_find_step(self): + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + step = agent_base.find_step(task, 'clean', 'deploy', + 'erase_devices') + self.assertEqual(self.clean_steps['deploy'][0], step) + + def test_find_step_not_found(self): + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.assertIsNone(agent_base.find_step( + task, 'clean', 'non-deploy', 'erase_devices')) + self.assertIsNone(agent_base.find_step( + task, 'clean', 'deploy', 'something_else')) + self.assertIsNone(agent_base.find_step( + task, 'deploy', 'deploy', 'erase_devices')) + def test_get_deploy_steps(self): with task_manager.acquire( self.context, self.node.uuid, shared=False) as task: diff --git a/releasenotes/notes/direct-deploy-steps-36486987156017d7.yaml b/releasenotes/notes/direct-deploy-steps-36486987156017d7.yaml new file mode 100644 index 0000000000..d45da41147 --- /dev/null +++ b/releasenotes/notes/direct-deploy-steps-36486987156017d7.yaml @@ -0,0 +1,17 @@ +--- +features: + - | + The ``deploy`` deploy step of the ``direct`` deploy interface has been + split into three deploy steps: + + * ``deploy`` itself (priority 100) boots the deploy ramdisk + + * ``write_image`` (priority 80) downloads the user image from inside + the ramdisk and writes it to the disk. + + * ``prepare_instance_boot`` (priority 60) prepares the boot device and + writes the bootloader (if needed). + + Priorities 81 to 99 to be used for in-band deploy steps that run before + the image is written. Priorities 61 to 79 can be used for in-band deploy + steps that modify the written image before the bootloader is installed.