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 <dtantsur@protonmail.com>
Change-Id: Ief586473aca0e22b74efe83ef70c354fd5df17bf
Story: 2006963
Task: 37778

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
 ---------------------

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')
+    has_decomposed_deploy_steps = True
-    def deploy_has_started(self, task):
-        commands = self._client.get_commands_status(task.node)
 
-        for command in commands:
+    @METRICS.timer('AgentDeployMixin.write_image')
-            if command['command_name'] == 'prepare_image':
+    @base.deploy_step(priority=80)
-                # 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')
     @task_manager.require_exclusive_lock
-    def continue_deploy(self, task):
+    def write_image(self, task):
-        task.process_event('resume')
+        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
+        has_write_image = agent_base.find_step(
-        self._client.prepare_image(node, image_info)
+            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')
+    @METRICS.timer('AgentDeployMixin.prepare_instance_boot')
-    def check_deploy_success(self, node):
+    @base.deploy_step(priority=60)
-        # should only ever be called after we've validated that
+    @task_manager.require_exclusive_lock
-        # the prepare_image command is complete
+    def prepare_instance_boot(self, task):
-        command = self._client.get_commands_status(node)[-1]
+        if not task.driver.storage.should_write_image(task):
-        if command['command_status'] == 'FAILED':
+            task.driver.boot.prepare_instance(task)
-            return agent_client.get_command_error(command)
+            # 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

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)

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,
+    def test_deploy_storage_should_write_image_false(
-                                                     mock_pxe_instance):
+            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()
+            self.assertFalse(prepare_image_mock.called)
-            prepare_image_mock.assert_called_with(mock.ANY, task.node,
+            self.assertEqual(states.DEPLOYING, task.node.provision_state)
-                                                  expected_image_info)
-            self.assertEqual(states.DEPLOYWAIT, 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,
+    def _test_write_image(self, additional_driver_info=None,
-                              additional_expected_image_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,
+            if compat:
-                                                         expected_image_info)
+                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):
+    def test_write_image(self):
-        self._test_continue_deploy()
+        self._test_write_image()
 
-    def test_continue_deploy_with_proxies(self):
+    def test_write_image_compat(self):
-        self._test_continue_deploy(
+        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):
+    def test_write_image_with_no_proxy_without_proxies(self):
-        self._test_continue_deploy(
+        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'
+    def test_prepare_instance_boot(self, prepare_instance_mock,
-                '.check_deploy_success', autospec=True)
+                                   uuid_mock, log_mock, remove_symlink_mock):
-    def test_reboot_to_instance(self, check_deploy_mock, prepare_instance_mock,
-                                uuid_mock, log_mock, remove_symlink_mock,
-                                resume_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)
+            task.driver.deploy.prepare_instance_boot(task)
-            check_deploy_mock.assert_called_once_with(mock.ANY, task.node)
             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'
+    def test_prepare_instance_boot_no_manage_agent_boot(
-                '.check_deploy_success', autospec=True)
+            self, prepare_instance_mock, uuid_mock,
-    def test_reboot_to_instance_no_manage_agent_boot(
+            bootdev_mock, log_mock):
-            self, check_deploy_mock, prepare_instance_mock, uuid_mock,
-            bootdev_mock, log_mock, resume_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)
+            task.driver.deploy.prepare_instance_boot(task)
-            check_deploy_mock.assert_called_once_with(mock.ANY, task.node)
             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'
+    def test_prepare_instance_boot_partition_image(self, prepare_instance_mock,
-                '.check_deploy_success', autospec=True)
+                                                   uuid_mock, boot_mode_mock,
-    def test_reboot_to_instance_partition_image(self, check_deploy_mock,
+                                                   log_mock):
-                                                prepare_instance_mock,
-                                                uuid_mock, boot_mode_mock,
-                                                log_mock,
-                                                resume_mock):
-        check_deploy_mock.return_value = None
         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)
+            task.driver.deploy.prepare_instance_boot(task)
-            check_deploy_mock.assert_called_once_with(mock.ANY, task.node)
             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'
+    def test_prepare_instance_boot_partition_image_compat(
-                '.check_deploy_success', autospec=True)
+            self, prepare_instance_mock, uuid_mock,
-    def test_reboot_to_instance_partition_image_compat(
+            old_uuid_mock, boot_mode_mock, log_mock):
-            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
         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)
+            task.driver.deploy.prepare_instance_boot(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
@@ -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'
+    def test_prepare_instance_boot_partition_localboot_ppc64(
-                '.check_deploy_success', autospec=True)
+            self, prepare_instance_mock,
-    def test_reboot_to_instance_partition_localboot_ppc64(
+            uuid_mock, boot_mode_mock, log_mock):
-            self, check_deploy_mock, prepare_instance_mock,
-            uuid_mock, boot_mode_mock, log_mock, resume_mock):
-        check_deploy_mock.return_value = None
         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'
+    def test_prepare_instance_boot_localboot(self, prepare_instance_mock,
-                '.check_deploy_success', autospec=True)
+                                             uuid_mock, boot_mode_mock,
-    def test_reboot_to_instance_localboot(self, check_deploy_mock,
+                                             log_mock):
-                                          prepare_instance_mock,
-                                          uuid_mock, boot_mode_mock,
-                                          log_mock,
-                                          resume_mock):
-        check_deploy_mock.return_value = None
         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):
+    def test_prepare_instance_boot_storage_should_write_image_with_smartnic(
-        with task_manager.acquire(self.context, self.node.uuid) as task:
+            self, mock_write, mock_pxe_instance):
-            mock_get_cmd.return_value = []
+        mock_write.return_value = False
-            self.assertFalse(task.driver.deploy.deploy_has_started(task))
+        self.node.provision_state = states.DEPLOYING
-
+        self.node.deploy_step = {
-    @mock.patch.object(agent_client.AgentClient, 'get_commands_status',
+            'step': 'deploy', 'priority': 50, 'interface': 'deploy'}
-                       autospec=True)
+        self.node.save()
-    def test_deploy_has_started_is_done(self, mock_get_cmd):
+        with task_manager.acquire(
-        with task_manager.acquire(self.context, self.node.uuid) as task:
+                self.context, self.node['uuid'], shared=False) as task:
-            mock_get_cmd.return_value = [{'command_name': 'prepare_image',
+            driver_return = self.driver.prepare_instance_boot(task)
-                                          'command_status': 'SUCCESS'}]
+            self.assertIsNone(driver_return)
-            self.assertTrue(task.driver.deploy.deploy_has_started(task))
+            self.assertTrue(mock_pxe_instance.called)
-
-    @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))
 
     @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):
 

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:

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.