From 63f6adf68ee76295a6ce8157362548ad10af8473 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 on iscsi and ansible deploy Following the decomposition of the core step on the 'direct' deploy interface, this change decomposed the iscsi and ansible deploy. Co-Authored-By: Dmitry Tantsur Change-Id: I537c6f6cf66c80b67b9045ea0618b02b7b93d36c Story: #2006963 Task: #40152 --- doc/source/admin/node-deployment.rst | 49 ++--- ironic/drivers/modules/agent_base.py | 90 ++++---- ironic/drivers/modules/ansible/deploy.py | 58 ++--- ironic/drivers/modules/iscsi_deploy.py | 57 +++-- .../drivers/modules/ansible/test_deploy.py | 135 ++---------- .../unit/drivers/modules/test_iscsi_deploy.py | 202 ++++++------------ .../iscsi-ansible-steps-817b52269d2455b0.yaml | 28 +++ 7 files changed, 249 insertions(+), 370 deletions(-) create mode 100644 releasenotes/notes/iscsi-ansible-steps-817b52269d2455b0.yaml diff --git a/doc/source/admin/node-deployment.rst b/doc/source/admin/node-deployment.rst index d535a4a75a..3136685ed4 100644 --- a/doc/source/admin/node-deployment.rst +++ b/doc/source/admin/node-deployment.rst @@ -47,8 +47,10 @@ All deploy interfaces based on ironic-python-agent (i.e. ``direct``, ``iscsi`` and ``ansible`` and any derivatives) expose the following deploy steps: ``deploy.deploy`` (priority 100) - In this step the node is booted using a provisioning image, and the user - image is written to the node's disk. + In this step the node is booted using a provisioning image. +``deploy.write_image`` (priority 80) + An out-of-band (``iscsi``, ``ansible``) or in-band (``direct``) step that + downloads and writes the image to the node. ``deploy.tear_down_agent`` (priority 40) In this step the provisioning image is shut down. ``deploy.switch_to_tenant_network`` (priority 30) @@ -57,10 +59,24 @@ and ``ansible`` and any derivatives) expose the following deploy steps: ``deploy.boot_instance`` (priority 20) In this step the node is booted into the user image. +Additionally, the ``iscsi`` and ``direct`` deploy interfaces have: + +``deploy.prepare_instance_boot`` (priority 60) + In this step the boot device is configured and the bootloader is installed. + + .. note:: + For the ``ansible`` deploy interface these steps are done in + ``deploy.write_image``. + Accordingly, the following priority ranges can be used for custom deploy steps: > 100 Out-of-band steps to run before deployment. +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. 41 to 59 In-band steps to run after the image is written the bootloader is installed. 21 to 39 @@ -68,35 +84,6 @@ 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. -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_base.py b/ironic/drivers/modules/agent_base.py index 8bd995ed45..267214073f 100644 --- a/ironic/drivers/modules/agent_base.py +++ b/ironic/drivers/modules/agent_base.py @@ -767,7 +767,53 @@ class AgentBaseMixin(object): task, manage_boot=self.should_manage_boot(task)) -class AgentDeployMixin(HeartbeatMixin): +class AgentOobStepsMixin(object): + """Mixin with out-of-band deploy steps.""" + + @METRICS.timer('AgentDeployMixin.switch_to_tenant_network') + @base.deploy_step(priority=30) + @task_manager.require_exclusive_lock + def switch_to_tenant_network(self, task): + """Deploy step to switch the node to the tenant network. + + :param task: a TaskManager object containing the node + """ + try: + with manager_utils.power_state_for_network_configuration(task): + task.driver.network.remove_provisioning_network(task) + task.driver.network.configure_tenant_networks(task) + except Exception as e: + msg = (_('Error changing node %(node)s to tenant networks after ' + 'deploy. %(cls)s: %(error)s') % + {'node': task.node.uuid, 'cls': e.__class__.__name__, + 'error': e}) + # NOTE(mgoddard): Don't collect logs since the node has been + # powered off. + log_and_raise_deployment_error(task, msg, collect_logs=False, + exc=e) + + @METRICS.timer('AgentDeployMixin.boot_instance') + @base.deploy_step(priority=20) + @task_manager.require_exclusive_lock + def boot_instance(self, task): + """Deploy step to boot the final instance. + + :param task: a TaskManager object containing the node + """ + try: + manager_utils.node_power_action(task, states.POWER_ON) + except Exception as e: + msg = (_('Error booting node %(node)s after deploy. ' + '%(cls)s: %(error)s') % + {'node': task.node.uuid, 'cls': e.__class__.__name__, + 'error': e}) + # NOTE(mgoddard): Don't collect logs since the node has been + # powered off. + log_and_raise_deployment_error(task, msg, collect_logs=False, + exc=e) + + +class AgentDeployMixin(HeartbeatMixin, AgentOobStepsMixin): """Mixin with deploy methods.""" @METRICS.timer('AgentDeployMixin.get_clean_steps') @@ -1169,48 +1215,6 @@ class AgentDeployMixin(HeartbeatMixin): 'error': e}) log_and_raise_deployment_error(task, msg, exc=e) - @METRICS.timer('AgentDeployMixin.switch_networking') - @base.deploy_step(priority=30) - @task_manager.require_exclusive_lock - def switch_to_tenant_network(self, task): - """Deploy step to switch the node to the tenant network. - - :param task: a TaskManager object containing the node - """ - try: - with manager_utils.power_state_for_network_configuration(task): - task.driver.network.remove_provisioning_network(task) - task.driver.network.configure_tenant_networks(task) - except Exception as e: - msg = (_('Error changing node %(node)s to tenant networks after ' - 'deploy. %(cls)s: %(error)s') % - {'node': task.node.uuid, 'cls': e.__class__.__name__, - 'error': e}) - # NOTE(mgoddard): Don't collect logs since the node has been - # powered off. - log_and_raise_deployment_error(task, msg, collect_logs=False, - exc=e) - - @METRICS.timer('AgentDeployMixin.boot_instance') - @base.deploy_step(priority=20) - @task_manager.require_exclusive_lock - def boot_instance(self, task): - """Deploy step to boot the final instance. - - :param task: a TaskManager object containing the node - """ - try: - manager_utils.node_power_action(task, states.POWER_ON) - except Exception as e: - msg = (_('Error booting node %(node)s after deploy. ' - '%(cls)s: %(error)s') % - {'node': task.node.uuid, 'cls': e.__class__.__name__, - 'error': e}) - # NOTE(mgoddard): Don't collect logs since the node has been - # powered off. - log_and_raise_deployment_error(task, msg, collect_logs=False, - exc=e) - # TODO(dtantsur): remove in W @METRICS.timer('AgentDeployMixin.reboot_and_finish_deploy') def reboot_and_finish_deploy(self, task): diff --git a/ironic/drivers/modules/ansible/deploy.py b/ironic/drivers/modules/ansible/deploy.py index cbecdc9760..d4186741fb 100644 --- a/ironic/drivers/modules/ansible/deploy.py +++ b/ironic/drivers/modules/ansible/deploy.py @@ -375,9 +375,13 @@ def _get_clean_steps(node, interface=None, override_priorities=None): return steps -class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface): +class AnsibleDeploy(agent_base.HeartbeatMixin, + agent_base.AgentOobStepsMixin, + base.DeployInterface): """Interface for deploy-related actions.""" + has_decomposed_deploy_steps = True + def __init__(self): super(AnsibleDeploy, self).__init__() # NOTE(pas-ha) overriding agent creation as we won't be @@ -442,12 +446,22 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface): manager_utils.node_power_action(task, states.REBOOT) return states.DEPLOYWAIT + def process_next_step(self, task, step_type): + """Start the next clean/deploy step if the previous one is complete. + + :param task: a TaskManager instance + :param step_type: "clean" or "deploy" + """ + # Run the next step as soon as agent heartbeats in deploy.deploy + if step_type == 'deploy' and self.in_core_deploy_step(task): + manager_utils.notify_conductor_resume_deploy(task) + @staticmethod def _required_image_info(task): """Gather and save needed image info while the context is good. Gather image info that will be needed later, during the - continue_deploy execution, where the context won't be the same + write_image execution, where the context won't be the same anymore, since coming from the server's heartbeat. """ node = task.node @@ -586,35 +600,30 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface): manager_utils.restore_power_state_if_needed( task, power_state_to_restore) - @METRICS.timer('AnsibleDeploy.continue_deploy') - def continue_deploy(self, task): + @METRICS.timer('AnsibleDeploy.write_image') + @base.deploy_step(priority=80) + def write_image(self, task): # NOTE(pas-ha) the lock should be already upgraded in heartbeat, # just setting its purpose for better logging task.upgrade_lock(purpose='deploy') - task.process_event('resume') # NOTE(pas-ha) this method is called from heartbeat processing only, # so we are sure we need this particular method, not the general one node_address = _get_node_ip(task) self._ansible_deploy(task, node_address) - self.reboot_to_instance(task) - - @METRICS.timer('AnsibleDeploy.reboot_to_instance') - def reboot_to_instance(self, task): - node = task.node - LOG.info('Ansible complete deploy on node %s', node.uuid) - - LOG.debug('Rebooting node %s to instance', node.uuid) + LOG.info('Ansible complete deploy on node %s', task.node.uuid) manager_utils.node_set_boot_device(task, 'disk', persistent=True) - self.reboot_and_finish_deploy(task) - task.driver.boot.clean_up_ramdisk(task) - # TODO(dtantsur): remove these two calls when this function becomes a - # real deploy step. - task.process_event('wait') - manager_utils.notify_conductor_resume_deploy(task) + @METRICS.timer('AnsibleDeploy.tear_down_agent') + @base.deploy_step(priority=40) + @task_manager.require_exclusive_lock + def tear_down_agent(self, task): + """A deploy step to tear down the agent. - @METRICS.timer('AnsibleDeploy.reboot_and_finish_deploy') - def reboot_and_finish_deploy(self, task): + Shuts down the machine and removes it from the provisioning + network. + + :param task: a TaskManager object containing the node + """ wait = CONF.ansible.post_deploy_get_power_state_retry_interval * 1000 attempts = CONF.ansible.post_deploy_get_power_state_retries + 1 @@ -652,13 +661,6 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface): manager_utils.node_power_action(task, states.POWER_OFF) else: manager_utils.node_power_action(task, states.POWER_OFF) - power_state_to_restore = ( - manager_utils.power_on_node_if_needed(task)) - task.driver.network.remove_provisioning_network(task) - task.driver.network.configure_tenant_networks(task) - manager_utils.restore_power_state_if_needed( - task, power_state_to_restore) - manager_utils.node_power_action(task, states.POWER_ON) except Exception as e: msg = (_('Error rebooting node %(node)s after deploy. ' 'Error: %(error)s') % diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 8069de0b52..41c83be612 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -602,6 +602,8 @@ class ISCSIDeploy(agent_base.AgentDeployMixin, agent_base.AgentBaseMixin, base.DeployInterface): """iSCSI Deploy Interface for deploy-related actions.""" + has_decomposed_deploy_steps = True + def get_properties(self): return agent_base.VENDOR_PROPERTIES @@ -647,14 +649,12 @@ class ISCSIDeploy(agent_base.AgentDeployMixin, agent_base.AgentBaseMixin, """ node = task.node if manager_utils.is_fast_track(task): + # NOTE(mgoddard): For fast track we can mostly skip this step and + # proceed to the next step (i.e. write_image). LOG.debug('Performing a fast track deployment for %(node)s.', {'node': task.node.uuid}) deploy_utils.cache_instance_image(task.context, node) check_image_size(task) - # 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) elif task.driver.storage.should_write_image(task): # Standard deploy process deploy_utils.cache_instance_image(task.context, node) @@ -666,29 +666,16 @@ class ISCSIDeploy(agent_base.AgentDeployMixin, agent_base.AgentBaseMixin, manager_utils.node_power_action(task, states.REBOOT) info = task.node.driver_internal_info info.pop('deployment_reboot', None) + info.pop('deployment_uuids', None) task.node.driver_internal_info = info task.node.save() return states.DEPLOYWAIT - else: - # Boot to an Storage Volume - # TODO(TheJulia): At some point, we should de-dupe this code - # as it is nearly identical to the agent 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) - - return None - - @METRICS.timer('AgentDeployMixin.continue_deploy') + @METRICS.timer('ISCSIDeploy.write_image') + @base.deploy_step(priority=90) @task_manager.require_exclusive_lock - def continue_deploy(self, task): + def write_image(self, task): """Method invoked when deployed using iSCSI. This method is invoked during a heartbeat from an agent when @@ -701,18 +688,39 @@ class ISCSIDeploy(agent_base.AgentDeployMixin, agent_base.AgentBaseMixin, :raises: InstanceDeployFailure, if it encounters some error during the deploy. """ - task.process_event('resume') + if not task.driver.storage.should_write_image(task): + LOG.debug('Skipping write_image for node %s', task.node.uuid) + return + node = task.node LOG.debug('Continuing the deployment on node %s', node.uuid) uuid_dict_returned = do_agent_iscsi_deploy(task, self._client) + utils.set_node_nested_field(node, 'driver_internal_info', + 'deployment_uuids', uuid_dict_returned) + node.save() + + @METRICS.timer('ISCSIDeploy.prepare_instance_boot') + @base.deploy_step(priority=80) + def prepare_instance_boot(self, task): + if not task.driver.storage.should_write_image(task): + task.driver.boot.prepare_instance(task) + return + + node = task.node + try: + uuid_dict_returned = node.driver_internal_info['deployment_uuids'] + except KeyError: + raise exception.InstanceDeployFailure( + _('Invalid internal state: the write_image deploy step has ' + 'not been called before prepare_instance_boot')) root_uuid = uuid_dict_returned.get('root uuid') efi_sys_uuid = uuid_dict_returned.get('efi system partition uuid') prep_boot_part_uuid = uuid_dict_returned.get( 'PrEP Boot partition uuid') + self.prepare_instance_to_boot(task, root_uuid, efi_sys_uuid, prep_boot_part_uuid=prep_boot_part_uuid) - self.reboot_and_finish_deploy(task) @METRICS.timer('ISCSIDeploy.prepare') @task_manager.require_exclusive_lock @@ -788,3 +796,6 @@ class ISCSIDeploy(agent_base.AgentDeployMixin, agent_base.AgentBaseMixin, """ deploy_utils.destroy_images(task.node.uuid) super(ISCSIDeploy, self).clean_up(task) + if utils.pop_node_nested_field(task.node, 'driver_internal_info', + 'deployment_uuids'): + task.node.save() diff --git a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py index 94ebe4a40a..cef1fabd41 100644 --- a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py +++ b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py @@ -890,13 +890,11 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): run_playbook_mock.assert_called_once_with( task.node, 'test_pl', ironic_nodes, 'test_k') - @mock.patch.object(utils, 'power_on_node_if_needed', autospec=True) @mock.patch.object(fake.FakePower, 'get_power_state', return_value=states.POWER_OFF, autospec=True) @mock.patch.object(utils, 'node_power_action', autospec=True) - def test_reboot_and_finish_deploy_force_reboot( - self, power_action_mock, get_pow_state_mock, - power_on_node_if_needed_mock): + def test_tear_down_agent_force_reboot( + self, power_action_mock, get_pow_state_mock): d_info = self.node.driver_info d_info['deploy_forces_oob_reboot'] = True self.node.driver_info = d_info @@ -906,27 +904,15 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): self.node.provision_state = states.DEPLOYING self.node.save() - power_on_node_if_needed_mock.return_value = None with task_manager.acquire(self.context, self.node.uuid) as task: - with mock.patch.object(task.driver, 'network', - autospec=True) as net_mock: - self.driver.reboot_and_finish_deploy(task) - net_mock.remove_provisioning_network.assert_called_once_with( - task) - net_mock.configure_tenant_networks.assert_called_once_with( - task) - expected_power_calls = [((task, states.POWER_OFF),), - ((task, states.POWER_ON),)] - self.assertEqual(expected_power_calls, - power_action_mock.call_args_list) + self.driver.tear_down_agent(task) + power_action_mock.assert_called_once_with(task, states.POWER_OFF) get_pow_state_mock.assert_not_called() - @mock.patch.object(utils, 'power_on_node_if_needed', autospec=True) @mock.patch.object(ansible_deploy, '_run_playbook', autospec=True) @mock.patch.object(utils, 'node_power_action', autospec=True) - def test_reboot_and_finish_deploy_soft_poweroff_retry( - self, power_action_mock, run_playbook_mock, - power_on_node_if_needed_mock): + def test_tear_down_agent_soft_poweroff_retry( + self, power_action_mock, run_playbook_mock): self.config(group='ansible', post_deploy_get_power_state_retry_interval=0) self.config(group='ansible', @@ -937,84 +923,38 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): self.node.driver_internal_info = di_info self.node.save() - power_on_node_if_needed_mock.return_value = None with task_manager.acquire(self.context, self.node.uuid) as task: - with mock.patch.object(task.driver, 'network', - autospec=True) as net_mock: - with mock.patch.object(task.driver.power, - 'get_power_state', - return_value=states.POWER_ON, - autospec=True) as p_mock: - self.driver.reboot_and_finish_deploy(task) - p_mock.assert_called_with(task) - self.assertEqual(2, len(p_mock.mock_calls)) - net_mock.remove_provisioning_network.assert_called_once_with( - task) - net_mock.configure_tenant_networks.assert_called_once_with( - task) - power_action_mock.assert_has_calls( - [mock.call(task, states.POWER_OFF), - mock.call(task, states.POWER_ON)]) - expected_power_calls = [((task, states.POWER_OFF),), - ((task, states.POWER_ON),)] - self.assertEqual(expected_power_calls, - power_action_mock.call_args_list) + with mock.patch.object(task.driver.power, + 'get_power_state', + return_value=states.POWER_ON, + autospec=True) as p_mock: + self.driver.tear_down_agent(task) + p_mock.assert_called_with(task) + self.assertEqual(2, len(p_mock.mock_calls)) + power_action_mock.assert_called_once_with(task, states.POWER_OFF) run_playbook_mock.assert_called_once_with( task.node, 'shutdown.yaml', mock.ANY, mock.ANY) + @mock.patch.object(utils, 'node_set_boot_device', autospec=True) @mock.patch.object(ansible_deploy, '_get_node_ip', autospec=True, return_value='1.2.3.4') - def test_continue_deploy(self, getip_mock): - self.node.provision_state = states.DEPLOYWAIT + def test_write_image(self, getip_mock, bootdev_mock): + 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) as task: with mock.patch.multiple(self.driver, autospec=True, _ansible_deploy=mock.DEFAULT, reboot_to_instance=mock.DEFAULT): - self.driver.continue_deploy(task) + result = self.driver.write_image(task) + self.assertIsNone(result) getip_mock.assert_called_once_with(task) self.driver._ansible_deploy.assert_called_once_with( task, '1.2.3.4') - self.driver.reboot_to_instance.assert_called_once_with(task) - self.assertEqual(states.ACTIVE, task.node.target_provision_state) - self.assertEqual(states.DEPLOYING, task.node.provision_state) - - @mock.patch.object(utils, 'notify_conductor_resume_deploy', autospec=True) - @mock.patch.object(utils, 'node_set_boot_device', autospec=True) - def test_reboot_to_instance(self, bootdev_mock, resume_mock): - self.node.provision_state = states.DEPLOYING - self.node.deploy_step = { - 'step': 'deploy', 'priority': 100, 'interface': 'deploy'} - self.node.save() - with task_manager.acquire(self.context, self.node.uuid) as task: - with mock.patch.object(self.driver, 'reboot_and_finish_deploy', - autospec=True): - task.driver.boot = mock.Mock() - self.driver.reboot_to_instance(task) bootdev_mock.assert_called_once_with(task, 'disk', persistent=True) - resume_mock.assert_called_once_with(task) - self.driver.reboot_and_finish_deploy.assert_called_once_with( - task) - task.driver.boot.clean_up_ramdisk.assert_called_once_with( - task) - - @mock.patch.object(utils, 'restore_power_state_if_needed', autospec=True) - @mock.patch.object(utils, 'power_on_node_if_needed', autospec=True) - @mock.patch.object(utils, 'node_power_action', autospec=True) - def test_tear_down_with_smartnic_port( - self, power_mock, power_on_node_if_needed_mock, - restore_power_state_mock): - 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.tear_down(task) - power_mock.assert_called_once_with(task, states.POWER_OFF) - self.assertEqual(driver_return, states.DELETED) - power_on_node_if_needed_mock.assert_called_once_with(task) - restore_power_state_mock.assert_called_once_with( - task, states.POWER_OFF) + self.assertEqual(states.ACTIVE, task.node.target_provision_state) + self.assertEqual(states.DEPLOYING, task.node.provision_state) @mock.patch.object(flat_network.FlatNetwork, 'add_provisioning_network', autospec=True) @@ -1105,36 +1045,3 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): power_on_node_if_needed_mock.assert_called_once_with(task) restore_power_state_mock.assert_called_once_with( task, states.POWER_OFF) - - @mock.patch.object(flat_network.FlatNetwork, 'remove_provisioning_network', - autospec=True) - @mock.patch.object(flat_network.FlatNetwork, 'configure_tenant_networks', - autospec=True) - @mock.patch.object(utils, 'restore_power_state_if_needed', autospec=True) - @mock.patch.object(utils, 'power_on_node_if_needed', autospec=True) - @mock.patch.object(fake.FakePower, 'get_power_state', - return_value=states.POWER_OFF, autospec=True) - @mock.patch.object(utils, 'node_power_action', autospec=True) - def test_reboot_and_finish_deploy_with_smartnic_port( - self, power_action_mock, get_pow_state_mock, - power_on_node_if_needed_mock, restore_power_state_mock, - configure_tenant_networks_mock, remove_provisioning_network_mock): - d_info = self.node.driver_info - d_info['deploy_forces_oob_reboot'] = True - self.node.driver_info = d_info - self.node.save() - self.config(group='ansible', - post_deploy_get_power_state_retry_interval=0) - self.node.provision_state = states.DEPLOYING - self.node.save() - power_on_node_if_needed_mock.return_value = states.POWER_OFF - with task_manager.acquire(self.context, self.node.uuid) as task: - self.driver.reboot_and_finish_deploy(task) - expected_power_calls = [((task, states.POWER_OFF),), - ((task, states.POWER_ON),)] - self.assertEqual( - expected_power_calls, power_action_mock.call_args_list) - power_on_node_if_needed_mock.assert_called_once_with(task) - restore_power_state_mock.assert_called_once_with( - task, states.POWER_OFF) - get_pow_state_mock.assert_not_called() diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index d980ef8caf..17ff141534 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -348,8 +348,8 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): 'node': self.node.uuid, 'params': log_params, } - uuid_dict_returned = {'root uuid': '12345678-87654321'} - deploy_mock.return_value = uuid_dict_returned + deployment_uuids = {'root uuid': '12345678-87654321'} + deploy_mock.return_value = deployment_uuids with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: @@ -362,7 +362,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): self.assertIsNone(task.node.last_error) mock_image_cache.assert_called_once_with() mock_image_cache.return_value.clean_up.assert_called_once_with() - self.assertEqual(uuid_dict_returned, retval) + self.assertEqual(deployment_uuids, retval) mock_disk_layout.assert_called_once_with(task.node, mock.ANY) @mock.patch.object(iscsi_deploy, 'LOG', autospec=True) @@ -392,8 +392,8 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): 'node': self.node.uuid, 'params': log_params, } - uuid_dict_returned = {'disk identifier': '87654321'} - deploy_mock.return_value = uuid_dict_returned + deployment_uuids = {'disk identifier': '87654321'} + deploy_mock.return_value = deployment_uuids with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.driver_internal_info['is_whole_disk_image'] = True @@ -406,7 +406,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): self.assertIsNone(task.node.last_error) mock_image_cache.assert_called_once_with() mock_image_cache.return_value.clean_up.assert_called_once_with() - self.assertEqual(uuid_dict_returned, retval) + self.assertEqual(deployment_uuids, retval) def _test_get_deploy_info(self, extra_instance_info=None): if extra_instance_info is None: @@ -489,8 +489,8 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): driver_internal_info = {'agent_url': 'http://1.2.3.4:1234'} self.node.driver_internal_info = driver_internal_info self.node.save() - uuid_dict_returned = {'root uuid': 'some-root-uuid'} - continue_deploy_mock.return_value = uuid_dict_returned + deployment_uuids = {'root uuid': 'some-root-uuid'} + continue_deploy_mock.return_value = deployment_uuids expected_iqn = 'iqn.2008-10.org.openstack:%s' % self.node.uuid with task_manager.acquire(self.context, self.node.uuid, @@ -504,7 +504,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): self.assertEqual( 'some-root-uuid', task.node.driver_internal_info['root_uuid_or_disk_id']) - self.assertEqual(ret_val, uuid_dict_returned) + self.assertEqual(ret_val, deployment_uuids) @mock.patch.object(iscsi_deploy, 'continue_deploy', autospec=True) def test_do_agent_iscsi_deploy_preserve_ephemeral(self, @@ -517,8 +517,8 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): 'agent_url': 'http://1.2.3.4:1234'} self.node.driver_internal_info = driver_internal_info self.node.save() - uuid_dict_returned = {'root uuid': 'some-root-uuid'} - continue_deploy_mock.return_value = uuid_dict_returned + deployment_uuids = {'root uuid': 'some-root-uuid'} + continue_deploy_mock.return_value = deployment_uuids expected_iqn = 'iqn.2008-10.org.openstack:%s' % self.node.uuid with task_manager.acquire(self.context, self.node.uuid, @@ -831,54 +831,31 @@ class ISCSIDeployTestCase(db_base.DbTestCase): self.assertNotIn( 'deployment_reboot', task.node.driver_internal_info) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', autospec=True) - @mock.patch.object(flat_network.FlatNetwork, - 'configure_tenant_networks', - spec_set=True, autospec=True) - @mock.patch.object(flat_network.FlatNetwork, - 'remove_provisioning_network', - spec_set=True, autospec=True) - @mock.patch.object(pxe.PXEBoot, - 'prepare_instance', - spec_set=True, autospec=True) - @mock.patch.object(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(iscsi_deploy, 'check_image_size', autospec=True) - @mock.patch.object(deploy_utils, 'cache_instance_image', autospec=True) - def test_deploy_storage_check_write_image_false(self, - mock_cache_instance_image, - mock_check_image_size, - mock_node_power_action, - mock_prepare_instance, - mock_remove_network, - mock_tenant_network, - mock_write): + def test_deploy_storage_should_write_image_false( + self, mock_write, mock_node_power_action): 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: + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: ret = task.driver.deploy.deploy(task) self.assertIsNone(ret) - self.assertFalse(mock_cache_instance_image.called) - self.assertFalse(mock_check_image_size.called) - mock_remove_network.assert_called_once_with(mock.ANY, task) - mock_tenant_network.assert_called_once_with(mock.ANY, task) - mock_prepare_instance.assert_called_once_with(mock.ANY, task) - self.assertEqual(2, mock_node_power_action.call_count) - self.assertEqual(states.DEPLOYING, task.node.provision_state) + self.assertFalse(mock_node_power_action.called) @mock.patch.object(iscsi_deploy, 'check_image_size', autospec=True) @mock.patch.object(deploy_utils, 'cache_instance_image', autospec=True) - @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'continue_deploy', + @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'write_image', autospec=True) @mock.patch('ironic.conductor.utils.is_fast_track', autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) def test_deploy_fast_track(self, power_mock, mock_pxe_instance, - mock_is_fast_track, continue_deploy_mock, + mock_is_fast_track, write_image_mock, cache_image_mock, check_image_size_mock): mock_is_fast_track.return_value = True self.node.target_provision_state = states.ACTIVE @@ -889,16 +866,17 @@ class ISCSIDeployTestCase(db_base.DbTestCase): self.node.save() with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: - task.driver.deploy.deploy(task) + result = task.driver.deploy.deploy(task) + self.assertIsNone(result) self.assertFalse(power_mock.called) self.assertFalse(mock_pxe_instance.called) task.node.refresh() - 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) cache_image_mock.assert_called_with(mock.ANY, task.node) check_image_size_mock.assert_called_with(task) - continue_deploy_mock.assert_called_with(mock.ANY, task) + self.assertFalse(write_image_mock.called) @mock.patch.object(noop_storage.NoopStorage, 'detach_volumes', autospec=True) @@ -995,90 +973,95 @@ class ISCSIDeployTestCase(db_base.DbTestCase): agent_execute_clean_step_mock.assert_called_once_with( task, {'some-step': 'step-info'}, 'clean') - @mock.patch.object(agent_base.AgentDeployMixin, - 'reboot_and_finish_deploy', autospec=True) @mock.patch.object(iscsi_deploy, 'do_agent_iscsi_deploy', autospec=True) - def test_continue_deploy_netboot(self, do_agent_iscsi_deploy_mock, - reboot_and_finish_deploy_mock): + def test_write_image(self, do_agent_iscsi_deploy_mock): self.node.instance_info = { 'capabilities': {'boot_option': 'netboot'}} self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE self.node.save() - uuid_dict_returned = {'root uuid': 'some-root-uuid'} - do_agent_iscsi_deploy_mock.return_value = uuid_dict_returned + deployment_uuids = {'root uuid': 'some-root-uuid'} + do_agent_iscsi_deploy_mock.return_value = deployment_uuids self.node.save() + with task_manager.acquire(self.context, self.node.uuid) as task: + task.driver.deploy.write_image(task) + do_agent_iscsi_deploy_mock.assert_called_once_with( + task, task.driver.deploy._client) + self.assertEqual( + task.node.driver_internal_info['deployment_uuids'], + deployment_uuids) + + @mock.patch.object(noop_storage.NoopStorage, 'should_write_image', + autospec=True) + @mock.patch.object(iscsi_deploy, 'do_agent_iscsi_deploy', autospec=True) + def test_write_image_bfv(self, do_agent_iscsi_deploy_mock, + should_write_image_mock): + should_write_image_mock.return_value = False + with task_manager.acquire(self.context, self.node.uuid) as task: + task.driver.deploy.write_image(task) + self.assertFalse(do_agent_iscsi_deploy_mock.called) + + def test_prepare_instance_boot_netboot(self): + deployment_uuids = {'root uuid': 'some-root-uuid'} + self.node.instance_info = { + 'capabilities': {'boot_option': 'netboot'}} + self.node.provision_state = states.DEPLOYWAIT + self.node.target_provision_state = states.ACTIVE + info = self.node.driver_internal_info + info['deployment_uuids'] = deployment_uuids + self.node.driver_internal_info = info + self.node.save() + with task_manager.acquire(self.context, self.node.uuid) as task: with mock.patch.object( task.driver.boot, 'prepare_instance') as m_prep_instance: - task.driver.deploy.continue_deploy(task) - do_agent_iscsi_deploy_mock.assert_called_once_with( - task, task.driver.deploy._client) - reboot_and_finish_deploy_mock.assert_called_once_with( - mock.ANY, task) + task.driver.deploy.prepare_instance_boot(task) m_prep_instance.assert_called_once_with(task) @mock.patch.object(fake.FakeManagement, 'set_boot_device', autospec=True) - @mock.patch.object(agent_base.AgentDeployMixin, - 'reboot_and_finish_deploy', autospec=True) @mock.patch.object(agent_base.AgentDeployMixin, 'configure_local_boot', autospec=True) - @mock.patch.object(iscsi_deploy, 'do_agent_iscsi_deploy', autospec=True) - def test_continue_deploy_localboot(self, do_agent_iscsi_deploy_mock, - configure_local_boot_mock, - reboot_and_finish_deploy_mock, - set_boot_device_mock): + def test_prepare_instance_boot_localboot(self, configure_local_boot_mock, + set_boot_device_mock): - self.node.instance_info = { - 'capabilities': {'boot_option': 'local'}} + deployment_uuids = {'root uuid': 'some-root-uuid'} self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE + info = self.node.driver_internal_info + info['deployment_uuids'] = deployment_uuids + self.node.driver_internal_info = info self.node.save() - uuid_dict_returned = {'root uuid': 'some-root-uuid'} - do_agent_iscsi_deploy_mock.return_value = uuid_dict_returned with task_manager.acquire(self.context, self.node.uuid) as task: - task.driver.deploy.continue_deploy(task) - do_agent_iscsi_deploy_mock.assert_called_once_with( - task, task.driver.deploy._client) + task.driver.deploy.prepare_instance_boot(task) configure_local_boot_mock.assert_called_once_with( task.driver.deploy, task, root_uuid='some-root-uuid', efi_system_part_uuid=None, prep_boot_part_uuid=None) - reboot_and_finish_deploy_mock.assert_called_once_with( - task.driver.deploy, task) set_boot_device_mock.assert_called_once_with( mock.ANY, task, device=boot_devices.DISK, persistent=True) @mock.patch.object(fake.FakeManagement, 'set_boot_device', autospec=True) - @mock.patch.object(agent_base.AgentDeployMixin, - 'reboot_and_finish_deploy', autospec=True) @mock.patch.object(agent_base.AgentDeployMixin, 'configure_local_boot', autospec=True) - @mock.patch.object(iscsi_deploy, 'do_agent_iscsi_deploy', autospec=True) - def test_continue_deploy_localboot_uefi(self, do_agent_iscsi_deploy_mock, - configure_local_boot_mock, - reboot_and_finish_deploy_mock, - set_boot_device_mock): - + def test_prepare_instance_boot_localboot_uefi( + self, configure_local_boot_mock, set_boot_device_mock): + deployment_uuids = {'root uuid': 'some-root-uuid', + 'efi system partition uuid': 'efi-part-uuid'} self.node.instance_info = { 'capabilities': {'boot_option': 'local'}} self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE + info = self.node.driver_internal_info + info['deployment_uuids'] = deployment_uuids + self.node.driver_internal_info = info self.node.save() - uuid_dict_returned = {'root uuid': 'some-root-uuid', - 'efi system partition uuid': 'efi-part-uuid'} - do_agent_iscsi_deploy_mock.return_value = uuid_dict_returned with task_manager.acquire(self.context, self.node.uuid) as task: - task.driver.deploy.continue_deploy(task) - do_agent_iscsi_deploy_mock.assert_called_once_with( - task, task.driver.deploy._client) + task.driver.deploy.prepare_instance_boot(task) configure_local_boot_mock.assert_called_once_with( task.driver.deploy, task, root_uuid='some-root-uuid', efi_system_part_uuid='efi-part-uuid', prep_boot_part_uuid=None) - reboot_and_finish_deploy_mock.assert_called_once_with( - task.driver.deploy, task) set_boot_device_mock.assert_called_once_with( mock.ANY, task, device=boot_devices.DISK, persistent=True) @@ -1157,49 +1140,6 @@ class ISCSIDeployTestCase(db_base.DbTestCase): 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(noop_storage.NoopStorage, 'should_write_image', - autospec=True) - @mock.patch.object(flat_network.FlatNetwork, - 'configure_tenant_networks', - spec_set=True, autospec=True) - @mock.patch.object(flat_network.FlatNetwork, - 'remove_provisioning_network', - spec_set=True, autospec=True) - @mock.patch.object(pxe.PXEBoot, - 'prepare_instance', - spec_set=True, autospec=True) - @mock.patch.object(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(iscsi_deploy, 'check_image_size', autospec=True) - @mock.patch.object(deploy_utils, 'cache_instance_image', autospec=True) - def test_deploy_storage_check_write_image_false_with_smartnic_port( - self, mock_cache_instance_image, mock_check_image_size, - mock_node_power_action, mock_prepare_instance, - mock_remove_network, mock_tenant_network, mock_write, - 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 - ret = task.driver.deploy.deploy(task) - self.assertIsNone(ret) - self.assertFalse(mock_cache_instance_image.called) - self.assertFalse(mock_check_image_size.called) - mock_remove_network.assert_called_once_with(mock.ANY, task) - mock_tenant_network.assert_called_once_with(mock.ANY, task) - mock_prepare_instance.assert_called_once_with(mock.ANY, task) - self.assertEqual(2, mock_node_power_action.call_count) - self.assertEqual(states.DEPLOYING, task.node.provision_state) - power_on_node_if_needed_mock.assert_called_once_with(task) - restore_power_state_mock.assert_called_once_with( - task, states.POWER_OFF) - # Cleanup of iscsi_deploy with pxe boot interface class CleanUpFullFlowTestCase(db_base.DbTestCase): diff --git a/releasenotes/notes/iscsi-ansible-steps-817b52269d2455b0.yaml b/releasenotes/notes/iscsi-ansible-steps-817b52269d2455b0.yaml new file mode 100644 index 0000000000..fec61904c1 --- /dev/null +++ b/releasenotes/notes/iscsi-ansible-steps-817b52269d2455b0.yaml @@ -0,0 +1,28 @@ +--- +features: + - | + The ``deploy`` deploy step of the ``iscsi`` deploy interface has been + split into three deploy steps: + + * ``deploy`` itself (priority 100) boots the deploy ramdisk + + * ``write_image`` (priority 80) writes the image to the disk exposed + via iSCSI. + + * ``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. + - | + The ``deploy`` deploy step of the ``ansible`` deploy interface has been + split into two deploy steps: + + * ``deploy`` itself (priority 100) boots the deploy ramdisk + + * ``write_image`` (priority 80) writes the image to the disk and configures + the bootloader. + + Priorities 81 to 99 to be used for in-band deploy steps that run before + the image is written.