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.