diff --git a/doc/source/admin/node-deployment.rst b/doc/source/admin/node-deployment.rst index 39dbc28a5e..03dfbfec2a 100644 --- a/doc/source/admin/node-deployment.rst +++ b/doc/source/admin/node-deployment.rst @@ -40,15 +40,35 @@ BIOS, and RAID interfaces. .. _node-deployment-core-steps: -Core steps ----------- +Agent steps +----------- -Certain default deploy steps are designated as 'core' deploy steps. The -following deploy steps are core: +All deploy interfaces based on ironic-python-agent (i.e. ``direct``, ``iscsi`` +and ``ansible`` and any derivatives) expose the following deploy steps: -``deploy.deploy`` +``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. It has a priority of 100. + image is written to the node's disk. +``deploy.tear_down_agent`` (priority 40) + In this step the provisioning image is shut down. +``deploy.switch_to_tenant_network`` (priority 30) + In this step networking for the node is switched from provisioning to + tenant networks. +``deploy.boot_instance`` (priority 20) + In this step the node is booted into the user image. + +Accordingly, the following priority ranges can be used for custom deploy steps: + +> 100 + Out-of-band steps to run before deployment. +41 to 59 + In-band steps to run after the image is written the bootloader is installed. +21 to 39 + Out-of-band steps to run after the provisioning image is shut down. +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. Writing a Deploy Step --------------------- diff --git a/ironic/conductor/deployments.py b/ironic/conductor/deployments.py index d0d3acc39e..8e50f713f2 100644 --- a/ironic/conductor/deployments.py +++ b/ironic/conductor/deployments.py @@ -268,6 +268,13 @@ def do_next_deploy_step(task, step_index, conductor_id): _("Failed to deploy. Exception: %s") % e, traceback=True) return + if task.node.provision_state == states.DEPLOYFAIL: + # NOTE(dtantsur): some deploy steps do not raise but rather update + # the node and return. Take them into account. + LOG.debug('Node %s is in error state, not processing ' + 'the remaining deploy steps', task.node) + return + if ind == 0: # We've done the very first deploy step. # Update conductor_affinity to reference this conductor's ID diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index c905d6a9bf..d2b8ba417e 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -376,7 +376,6 @@ class AgentDeployMixin(agent_base.AgentDeployMixin): if CONF.agent.image_download_source == 'http': deploy_utils.remove_http_instance_symlink(task.node.uuid) - LOG.debug('Rebooting node %s to instance', node.uuid) self.reboot_and_finish_deploy(task) diff --git a/ironic/drivers/modules/agent_base.py b/ironic/drivers/modules/agent_base.py index a85832de5a..4ab403312b 100644 --- a/ironic/drivers/modules/agent_base.py +++ b/ironic/drivers/modules/agent_base.py @@ -35,6 +35,7 @@ from ironic.conductor import steps as conductor_steps from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils from ironic.conf import CONF +from ironic.drivers import base from ironic.drivers.modules import agent_client from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import deploy_utils @@ -382,8 +383,21 @@ def _step_failure_handler(task, msg, step_type): class HeartbeatMixin(object): """Mixin class implementing heartbeat processing.""" + has_decomposed_deploy_steps = False + """Whether the driver supports decomposed deploy steps. + + Previously (since Rocky), drivers used a single 'deploy' deploy step on + the deploy interface. Some additional steps were added for the 'direct' + and 'iscsi' deploy interfaces in the Ussuri cycle, which means that + more of the deployment flow is driven by deploy steps. + """ + def __init__(self): self._client = _get_client() + if not self.has_decomposed_deploy_steps: + LOG.warning('%s does not support decomposed deploy steps. This ' + 'is deprecated and will stop working in a future ' + 'release', self.__class__.__name__) def continue_deploy(self, task): """Continues the deployment of baremetal node. @@ -501,8 +515,12 @@ class HeartbeatMixin(object): # are currently in the core deploy.deploy step. Other deploy steps # may cause the agent to boot, but we should not trigger deployment # at that point if the driver is polling for completion of a step. - if self.in_core_deploy_step(task): + if (not self.has_decomposed_deploy_steps + and self.in_core_deploy_step(task)): msg = _('Failed checking if deploy is done') + # NOTE(mgoddard): support backwards compatibility for + # drivers which do not implement continue_deploy and + # reboot_to_instance as deploy steps. if not self.deploy_has_started(task): msg = _('Node failed to deploy') self.continue_deploy(task) @@ -1067,16 +1085,13 @@ class AgentDeployMixin(HeartbeatMixin): LOG.error(msg) return _step_failure_handler(task, msg, step_type) - @METRICS.timer('AgentDeployMixin.reboot_and_finish_deploy') - def reboot_and_finish_deploy(self, task): - """Helper method to trigger reboot on the node and finish deploy. - - This method initiates a reboot on the node. On success, it - marks the deploy as complete. On failure, it logs the error - and marks deploy as failure. + @METRICS.timer('AgentDeployMixin.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. :param task: a TaskManager object containing the node - :raises: InstanceDeployFailure, if node reboot failed. """ wait = CONF.agent.post_deploy_get_power_state_retry_interval * 1000 attempts = CONF.agent.post_deploy_get_power_state_retries + 1 @@ -1145,23 +1160,63 @@ 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) - manager_utils.node_power_action(task, states.POWER_ON) except Exception as e: - msg = (_('Error rebooting node %(node)s after deploy. ' - '%(cls)s: %(error)s') % - {'node': node.uuid, 'cls': e.__class__.__name__, + 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) - # TODO(dtantsur): remove these two calls when this function becomes a - # real deploy step. + @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): + """Helper method to trigger reboot on the node and finish deploy. + + This method initiates a reboot on the node. On success, it + marks the deploy as complete. On failure, it logs the error + and marks deploy as failure. + + :param task: a TaskManager object containing the node + :raises: InstanceDeployFailure, if node reboot failed. + """ + # NOTE(dtantsur): do nothing here, the new deploy steps tear_down_agent + # and boot_instance will be picked up and finish the deploy (even for + # legacy deploy interfaces without decomposed steps). task.process_event('wait') manager_utils.notify_conductor_resume_deploy(task) diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 57f7c49900..d1c39256b2 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import types from unittest import mock from oslo_config import cfg @@ -1275,27 +1274,18 @@ class TestAgentDeploy(db_base.DbTestCase): @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', autospec=True) - @mock.patch.object(manager_utils, 'power_on_node_if_needed', - autospec=True) @mock.patch.object(deploy_utils, 'remove_http_instance_symlink', autospec=True) @mock.patch.object(agent.LOG, 'warning', spec_set=True, autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_partition_uuids', autospec=True) - @mock.patch.object(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(fake.FakePower, 'get_power_state', - spec=types.FunctionType) - @mock.patch.object(agent_client.AgentClient, 'power_off', - spec=types.FunctionType) @mock.patch.object(agent.AgentDeployMixin, 'prepare_instance_to_boot', autospec=True) @mock.patch('ironic.drivers.modules.agent.AgentDeployMixin' '.check_deploy_success', autospec=True) - def test_reboot_to_instance(self, check_deploy_mock, - prepare_instance_mock, power_off_mock, - get_power_state_mock, node_power_action_mock, + def test_reboot_to_instance(self, check_deploy_mock, prepare_instance_mock, uuid_mock, log_mock, remove_symlink_mock, - power_on_node_if_needed_mock, resume_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 @@ -1305,8 +1295,6 @@ class TestAgentDeploy(db_base.DbTestCase): self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - get_power_state_mock.return_value = states.POWER_OFF - power_on_node_if_needed_mock.return_value = None 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) @@ -1316,10 +1304,6 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertFalse(log_mock.called) prepare_instance_mock.assert_called_once_with(mock.ANY, task, None, None, None) - power_off_mock.assert_called_once_with(task.node) - get_power_state_mock.assert_called_once_with(task) - node_power_action_mock.assert_called_once_with( - task, states.POWER_ON) self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) self.assertTrue(remove_symlink_mock.called) @@ -1327,26 +1311,17 @@ class TestAgentDeploy(db_base.DbTestCase): @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', autospec=True) - @mock.patch.object(manager_utils, 'power_on_node_if_needed', - 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(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(fake.FakePower, 'get_power_state', - spec=types.FunctionType) - @mock.patch.object(agent_client.AgentClient, 'power_off', - spec=types.FunctionType) @mock.patch.object(agent.AgentDeployMixin, 'prepare_instance_to_boot', autospec=True) @mock.patch('ironic.drivers.modules.agent.AgentDeployMixin' '.check_deploy_success', autospec=True) def test_reboot_to_instance_no_manage_agent_boot( - self, check_deploy_mock, prepare_instance_mock, power_off_mock, - get_power_state_mock, node_power_action_mock, uuid_mock, - bootdev_mock, log_mock, power_on_node_if_needed_mock, - resume_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 = {} @@ -1355,8 +1330,6 @@ class TestAgentDeploy(db_base.DbTestCase): self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - power_on_node_if_needed_mock.return_value = None - get_power_state_mock.return_value = states.POWER_OFF 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) @@ -1366,40 +1339,25 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertFalse(log_mock.called) self.assertFalse(prepare_instance_mock.called) bootdev_mock.assert_called_once_with(task, 'disk', persistent=True) - power_off_mock.assert_called_once_with(task.node) - get_power_state_mock.assert_called_once_with(task) - node_power_action_mock.assert_called_once_with( - task, states.POWER_ON) self.assertEqual(states.DEPLOYWAIT, 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(manager_utils, 'power_on_node_if_needed', - 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(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(fake.FakePower, 'get_power_state', - spec=types.FunctionType) - @mock.patch.object(agent_client.AgentClient, 'power_off', - spec=types.FunctionType) @mock.patch.object(agent.AgentDeployMixin, 'prepare_instance_to_boot', autospec=True) @mock.patch('ironic.drivers.modules.agent.AgentDeployMixin' '.check_deploy_success', autospec=True) def test_reboot_to_instance_partition_image(self, check_deploy_mock, prepare_instance_mock, - power_off_mock, - get_power_state_mock, - node_power_action_mock, uuid_mock, boot_mode_mock, log_mock, - power_on_node_if_needed_mock, resume_mock): check_deploy_mock.return_value = None self.node.instance_info = { @@ -1413,8 +1371,6 @@ class TestAgentDeploy(db_base.DbTestCase): boot_mode_mock.return_value = 'bios' with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - power_on_node_if_needed_mock.return_value = None - get_power_state_mock.return_value = states.POWER_OFF driver_internal_info = task.node.driver_internal_info driver_internal_info['is_whole_disk_image'] = False task.node.driver_internal_info = driver_internal_info @@ -1430,18 +1386,12 @@ class TestAgentDeploy(db_base.DbTestCase): task, 'root_uuid', None, None) - power_off_mock.assert_called_once_with(task.node) - get_power_state_mock.assert_called_once_with(task) - node_power_action_mock.assert_called_once_with( - task, states.POWER_ON) self.assertEqual(states.DEPLOYWAIT, 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(manager_utils, 'power_on_node_if_needed', - 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) @@ -1449,20 +1399,13 @@ class TestAgentDeploy(db_base.DbTestCase): autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_partition_uuids', autospec=True) - @mock.patch.object(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(fake.FakePower, 'get_power_state', - spec=types.FunctionType) - @mock.patch.object(agent_client.AgentClient, 'power_off', - spec=types.FunctionType) @mock.patch.object(agent.AgentDeployMixin, 'prepare_instance_to_boot', autospec=True) @mock.patch('ironic.drivers.modules.agent.AgentDeployMixin' '.check_deploy_success', autospec=True) def test_reboot_to_instance_partition_image_compat( - self, check_deploy_mock, prepare_instance_mock, power_off_mock, - get_power_state_mock, node_power_action_mock, uuid_mock, - old_uuid_mock, boot_mode_mock, log_mock, - power_on_node_if_needed_mock, resume_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'}} @@ -1474,8 +1417,6 @@ class TestAgentDeploy(db_base.DbTestCase): boot_mode_mock.return_value = 'bios' with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - power_on_node_if_needed_mock.return_value = None - get_power_state_mock.return_value = states.POWER_OFF driver_internal_info = task.node.driver_internal_info driver_internal_info['is_whole_disk_image'] = False task.node.driver_internal_info = driver_internal_info @@ -1492,37 +1433,24 @@ class TestAgentDeploy(db_base.DbTestCase): task, 'root_uuid', None, None) - power_off_mock.assert_called_once_with(task.node) - get_power_state_mock.assert_called_once_with(task) - node_power_action_mock.assert_called_once_with( - task, states.POWER_ON) self.assertEqual(states.DEPLOYWAIT, 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(manager_utils, 'power_on_node_if_needed', - 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(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(fake.FakePower, 'get_power_state', - spec=types.FunctionType) - @mock.patch.object(agent_client.AgentClient, 'power_off', - spec=types.FunctionType) @mock.patch.object(agent.AgentDeployMixin, 'prepare_instance_to_boot', autospec=True) @mock.patch('ironic.drivers.modules.agent.AgentDeployMixin' '.check_deploy_success', autospec=True) def test_reboot_to_instance_partition_localboot_ppc64( self, check_deploy_mock, prepare_instance_mock, - power_off_mock, get_power_state_mock, - node_power_action_mock, uuid_mock, boot_mode_mock, log_mock, - power_on_node_if_needed_mock, resume_mock): + uuid_mock, boot_mode_mock, log_mock, resume_mock): check_deploy_mock.return_value = None uuid_mock.return_value = { 'command_result': { @@ -1536,8 +1464,6 @@ class TestAgentDeploy(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - power_on_node_if_needed_mock.return_value = None - get_power_state_mock.return_value = states.POWER_OFF driver_internal_info = task.node.driver_internal_info driver_internal_info['is_whole_disk_image'] = False task.node.driver_internal_info = driver_internal_info @@ -1558,10 +1484,6 @@ 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') - power_off_mock.assert_called_once_with(task.node) - get_power_state_mock.assert_called_once_with(task) - node_power_action_mock.assert_called_once_with( - task, states.POWER_ON) self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) @@ -1569,18 +1491,12 @@ class TestAgentDeploy(db_base.DbTestCase): @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_partition_uuids', autospec=True) - @mock.patch.object(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(fake.FakePower, 'get_power_state', - spec=types.FunctionType) - @mock.patch.object(agent_client.AgentClient, 'power_off', - spec=types.FunctionType) @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, - power_off_mock, get_power_state_mock, node_power_action_mock, uuid_mock, collect_ramdisk_logs_mock, log_mock): check_deploy_mock.return_value = "Error" uuid_mock.return_value = None @@ -1589,43 +1505,30 @@ class TestAgentDeploy(db_base.DbTestCase): self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - get_power_state_mock.return_value = states.POWER_OFF 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) - self.assertFalse(power_off_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(manager_utils, 'power_on_node_if_needed', - 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(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(fake.FakePower, 'get_power_state', - spec=types.FunctionType) - @mock.patch.object(agent_client.AgentClient, 'power_off', - spec=types.FunctionType) @mock.patch.object(agent.AgentDeployMixin, 'prepare_instance_to_boot', autospec=True) @mock.patch('ironic.drivers.modules.agent.AgentDeployMixin' '.check_deploy_success', autospec=True) def test_reboot_to_instance_localboot(self, check_deploy_mock, prepare_instance_mock, - power_off_mock, - get_power_state_mock, - node_power_action_mock, uuid_mock, boot_mode_mock, log_mock, - power_on_node_if_needed_mock, resume_mock): check_deploy_mock.return_value = None uuid_mock.return_value = { @@ -1640,8 +1543,6 @@ class TestAgentDeploy(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - power_on_node_if_needed_mock.return_value = None - get_power_state_mock.return_value = states.POWER_OFF driver_internal_info = task.node.driver_internal_info driver_internal_info['is_whole_disk_image'] = False task.node.driver_internal_info = driver_internal_info @@ -1659,10 +1560,6 @@ 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) - power_off_mock.assert_called_once_with(task.node) - get_power_state_mock.assert_called_once_with(task) - node_power_action_mock.assert_called_once_with( - task, states.POWER_ON) self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) resume_mock.assert_called_once_with(task) diff --git a/ironic/tests/unit/drivers/modules/test_agent_base.py b/ironic/tests/unit/drivers/modules/test_agent_base.py index 160fef0128..6773ff8347 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base.py @@ -171,6 +171,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: self.deploy.heartbeat(task, 'url', '3.2.0') + self.assertIsNone(task.node.last_error) self.assertFalse(task.shared) self.assertEqual( 'url', task.node.driver_internal_info['agent_url']) @@ -304,6 +305,44 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): self.assertFalse(rti_mock.called) self.assertFalse(in_resume_deploy_mock.called) + @mock.patch.object(agent_base.HeartbeatMixin, 'process_next_step', + autospec=True) + @mock.patch.object(agent_base.HeartbeatMixin, + 'in_core_deploy_step', autospec=True) + @mock.patch.object(agent_base.HeartbeatMixin, + 'deploy_has_started', autospec=True) + @mock.patch.object(agent_base.HeartbeatMixin, + 'deploy_is_done', autospec=True) + @mock.patch.object(agent_base.HeartbeatMixin, 'continue_deploy', + autospec=True) + @mock.patch.object(agent_base.HeartbeatMixin, + 'reboot_to_instance', autospec=True) + def test_heartbeat_decomposed_steps(self, rti_mock, cd_mock, + deploy_is_done_mock, + deploy_started_mock, + in_deploy_mock, + next_step_mock): + self.deploy.has_decomposed_deploy_steps = True + # Check that heartbeats do not trigger deployment actions when the + # driver has decomposed deploy steps. + self.node.provision_state = states.DEPLOYWAIT + self.node.save() + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + self.deploy.heartbeat(task, 'url', '3.2.0') + self.assertFalse(task.shared) + self.assertEqual( + 'url', task.node.driver_internal_info['agent_url']) + self.assertEqual( + '3.2.0', + task.node.driver_internal_info['agent_version']) + self.assertFalse(in_deploy_mock.called) + self.assertFalse(deploy_started_mock.called) + self.assertFalse(deploy_is_done_mock.called) + self.assertFalse(cd_mock.called) + self.assertFalse(rti_mock.called) + self.assertTrue(next_step_mock.called) + @mock.patch.object(agent_base.HeartbeatMixin, 'continue_deploy', autospec=True) @mock.patch.object(agent_base.HeartbeatMixin, @@ -849,8 +888,6 @@ class AgentRescueTests(AgentDeployMixinBaseTest): class AgentDeployMixinTest(AgentDeployMixinBaseTest): @mock.patch.object(manager_utils, 'power_on_node_if_needed', autospec=True) - @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', - autospec=True) @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @mock.patch.object(time, 'sleep', lambda seconds: None) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @@ -858,34 +895,27 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): spec=types.FunctionType) @mock.patch.object(agent_client.AgentClient, 'power_off', spec=types.FunctionType) - def test_reboot_and_finish_deploy( + def test_tear_down_agent( self, power_off_mock, get_power_state_mock, - node_power_action_mock, collect_mock, resume_mock, + node_power_action_mock, collect_mock, power_on_node_if_needed_mock): cfg.CONF.set_override('deploy_logs_collect', 'always', 'agent') 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=True) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: get_power_state_mock.side_effect = [states.POWER_ON, states.POWER_OFF] power_on_node_if_needed_mock.return_value = None - self.deploy.reboot_and_finish_deploy(task) + self.deploy.tear_down_agent(task) power_off_mock.assert_called_once_with(task.node) self.assertEqual(2, get_power_state_mock.call_count) - node_power_action_mock.assert_called_once_with( - task, states.POWER_ON) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) + self.assertFalse(node_power_action_mock.called) + self.assertEqual(states.DEPLOYING, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) collect_mock.assert_called_once_with(task.node) - resume_mock.assert_called_once_with(task) - @mock.patch.object(manager_utils, 'power_on_node_if_needed', - autospec=True) - @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', - autospec=True) @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @mock.patch.object(time, 'sleep', lambda seconds: None) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @@ -893,38 +923,23 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): spec=types.FunctionType) @mock.patch.object(agent_client.AgentClient, 'power_off', spec=types.FunctionType) - @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' - 'remove_provisioning_network', spec_set=True, autospec=True) - @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' - 'configure_tenant_networks', spec_set=True, autospec=True) - def test_reboot_and_finish_deploy_soft_poweroff_doesnt_complete( - self, configure_tenant_net_mock, remove_provisioning_net_mock, - power_off_mock, get_power_state_mock, - node_power_action_mock, mock_collect, resume_mock, - power_on_node_if_needed_mock): + def test_tear_down_agent_soft_poweroff_doesnt_complete( + self, power_off_mock, get_power_state_mock, + node_power_action_mock, mock_collect): 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=True) as task: - power_on_node_if_needed_mock.return_value = None + with task_manager.acquire(self.context, self.node.uuid) as task: get_power_state_mock.return_value = states.POWER_ON - self.deploy.reboot_and_finish_deploy(task) + self.deploy.tear_down_agent(task) power_off_mock.assert_called_once_with(task.node) self.assertEqual(7, get_power_state_mock.call_count) - node_power_action_mock.assert_has_calls([ - mock.call(task, states.POWER_OFF), - mock.call(task, states.POWER_ON)]) - remove_provisioning_net_mock.assert_called_once_with(mock.ANY, - task) - configure_tenant_net_mock.assert_called_once_with(mock.ANY, task) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) + node_power_action_mock.assert_called_once_with(task, + states.POWER_OFF) + self.assertEqual(states.DEPLOYING, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) self.assertFalse(mock_collect.called) - resume_mock.assert_called_once_with(task) - @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', - autospec=True) @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @mock.patch.object(time, 'sleep', lambda seconds: None) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @@ -932,35 +947,23 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): spec=types.FunctionType) @mock.patch.object(agent_client.AgentClient, 'power_off', spec=types.FunctionType) - @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' - 'remove_provisioning_network', spec_set=True, autospec=True) - @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' - 'configure_tenant_networks', spec_set=True, autospec=True) - def test_reboot_and_finish_deploy_soft_poweroff_fails( - self, configure_tenant_net_mock, remove_provisioning_net_mock, - power_off_mock, get_power_state_mock, node_power_action_mock, - mock_collect, resume_mock): + def test_tear_down_agent_soft_poweroff_fails( + self, power_off_mock, get_power_state_mock, node_power_action_mock, + mock_collect): power_off_mock.side_effect = RuntimeError("boom") 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=True) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: get_power_state_mock.return_value = states.POWER_ON - self.deploy.reboot_and_finish_deploy(task) + self.deploy.tear_down_agent(task) power_off_mock.assert_called_once_with(task.node) - node_power_action_mock.assert_has_calls([ - mock.call(task, states.POWER_OFF), - mock.call(task, states.POWER_ON)]) - remove_provisioning_net_mock.assert_called_once_with(mock.ANY, - task) - configure_tenant_net_mock.assert_called_once_with(mock.ANY, task) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) + node_power_action_mock.assert_called_once_with(task, + states.POWER_OFF) + self.assertEqual(states.DEPLOYING, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) self.assertFalse(mock_collect.called) - @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', - autospec=True) @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @mock.patch.object(time, 'sleep', lambda seconds: None) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @@ -968,38 +971,26 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): spec=types.FunctionType) @mock.patch.object(agent_client.AgentClient, 'power_off', spec=types.FunctionType) - @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' - 'remove_provisioning_network', spec_set=True, autospec=True) - @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' - 'configure_tenant_networks', spec_set=True, autospec=True) - def test_reboot_and_finish_deploy_soft_poweroff_race( - self, configure_tenant_net_mock, remove_provisioning_net_mock, - power_off_mock, get_power_state_mock, node_power_action_mock, - mock_collect, resume_mock): + def test_tear_down_agent_soft_poweroff_race( + self, power_off_mock, get_power_state_mock, node_power_action_mock, + mock_collect): # Test the situation when soft power off works, but ironic doesn't # learn about it. power_off_mock.side_effect = RuntimeError("boom") 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=True) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: get_power_state_mock.side_effect = [states.POWER_ON, states.POWER_OFF] - self.deploy.reboot_and_finish_deploy(task) + self.deploy.tear_down_agent(task) power_off_mock.assert_called_once_with(task.node) - node_power_action_mock.assert_called_once_with( - task, states.POWER_ON) - remove_provisioning_net_mock.assert_called_once_with(mock.ANY, - task) - configure_tenant_net_mock.assert_called_once_with(mock.ANY, task) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) + self.assertFalse(node_power_action_mock.called) + self.assertEqual(states.DEPLOYING, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) self.assertFalse(mock_collect.called) @mock.patch.object(manager_utils, 'power_on_node_if_needed', autospec=True) - @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', - autospec=True) @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @mock.patch.object(time, 'sleep', lambda seconds: None) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @@ -1007,67 +998,21 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): spec=types.FunctionType) @mock.patch.object(agent_client.AgentClient, 'power_off', spec=types.FunctionType) - @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' - 'remove_provisioning_network', spec_set=True, autospec=True) - @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' - 'configure_tenant_networks', spec_set=True, autospec=True) - def test_reboot_and_finish_deploy_get_power_state_fails( - self, configure_tenant_net_mock, remove_provisioning_net_mock, - power_off_mock, get_power_state_mock, node_power_action_mock, - mock_collect, resume_mock, power_on_node_if_needed_mock): + def test_tear_down_agent_get_power_state_fails( + self, power_off_mock, get_power_state_mock, node_power_action_mock, + mock_collect, power_on_node_if_needed_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, - shared=True) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: get_power_state_mock.side_effect = RuntimeError("boom") power_on_node_if_needed_mock.return_value = None - self.deploy.reboot_and_finish_deploy(task) + self.deploy.tear_down_agent(task) power_off_mock.assert_called_once_with(task.node) self.assertEqual(7, get_power_state_mock.call_count) - node_power_action_mock.assert_has_calls([ - mock.call(task, states.POWER_OFF), - mock.call(task, states.POWER_ON)]) - remove_provisioning_net_mock.assert_called_once_with(mock.ANY, - task) - configure_tenant_net_mock.assert_called_once_with(mock.ANY, task) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) - self.assertEqual(states.ACTIVE, task.node.target_provision_state) - self.assertFalse(mock_collect.called) - - @mock.patch.object(manager_utils, 'power_on_node_if_needed', - autospec=True) - @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) - @mock.patch.object(time, 'sleep', lambda seconds: None) - @mock.patch.object(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(fake.FakePower, 'get_power_state', - spec=types.FunctionType) - @mock.patch.object(agent_client.AgentClient, 'power_off', - spec=types.FunctionType) - @mock.patch('ironic.drivers.modules.network.neutron.NeutronNetwork.' - 'remove_provisioning_network', spec_set=True, autospec=True) - @mock.patch('ironic.drivers.modules.network.neutron.NeutronNetwork.' - 'configure_tenant_networks', spec_set=True, autospec=True) - def test_reboot_and_finish_deploy_configure_tenant_network_exception( - self, configure_tenant_net_mock, remove_provisioning_net_mock, - power_off_mock, get_power_state_mock, node_power_action_mock, - mock_collect, power_on_node_if_needed_mock): - self.node.network_interface = 'neutron' - self.node.provision_state = states.DEPLOYING - self.node.target_provision_state = states.ACTIVE - self.node.save() - power_on_node_if_needed_mock.return_value = None - with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: - configure_tenant_net_mock.side_effect = exception.NetworkError( - "boom") - self.assertRaises(exception.InstanceDeployFailure, - self.deploy.reboot_and_finish_deploy, task) - self.assertEqual(7, get_power_state_mock.call_count) - remove_provisioning_net_mock.assert_called_once_with(mock.ANY, - task) - configure_tenant_net_mock.assert_called_once_with(mock.ANY, task) - self.assertEqual(states.DEPLOYFAIL, task.node.provision_state) + node_power_action_mock.assert_called_once_with(task, + states.POWER_OFF) + self.assertEqual(states.DEPLOYING, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) self.assertFalse(mock_collect.called) @@ -1078,78 +1023,31 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): spec=types.FunctionType) @mock.patch.object(agent_client.AgentClient, 'power_off', spec=types.FunctionType) - def test_reboot_and_finish_deploy_power_off_fails( + def test_tear_down_agent_power_off_fails( self, power_off_mock, get_power_state_mock, node_power_action_mock, mock_collect): 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=True) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: get_power_state_mock.return_value = states.POWER_ON node_power_action_mock.side_effect = RuntimeError("boom") self.assertRaises(exception.InstanceDeployFailure, - self.deploy.reboot_and_finish_deploy, + self.deploy.tear_down_agent, task) power_off_mock.assert_called_once_with(task.node) self.assertEqual(7, get_power_state_mock.call_count) - node_power_action_mock.assert_has_calls([ - mock.call(task, states.POWER_OFF)]) + node_power_action_mock.assert_called_with(task, states.POWER_OFF) self.assertEqual(states.DEPLOYFAIL, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) mock_collect.assert_called_once_with(task.node) - @mock.patch.object(manager_utils, 'power_on_node_if_needed', - autospec=True) - @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) - @mock.patch.object(time, 'sleep', lambda seconds: None) - @mock.patch.object(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(fake.FakePower, 'get_power_state', - spec=types.FunctionType) - @mock.patch.object(agent_client.AgentClient, 'power_off', - spec=types.FunctionType) - @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' - 'remove_provisioning_network', spec_set=True, autospec=True) - @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' - 'configure_tenant_networks', spec_set=True, autospec=True) - def test_reboot_and_finish_deploy_power_on_fails( - self, configure_tenant_net_mock, remove_provisioning_net_mock, - power_off_mock, get_power_state_mock, - node_power_action_mock, mock_collect, - power_on_node_if_needed_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, - shared=True) as task: - power_on_node_if_needed_mock.return_value = None - get_power_state_mock.return_value = states.POWER_ON - node_power_action_mock.side_effect = [None, - RuntimeError("boom")] - self.assertRaises(exception.InstanceDeployFailure, - self.deploy.reboot_and_finish_deploy, - task) - power_off_mock.assert_called_once_with(task.node) - self.assertEqual(7, get_power_state_mock.call_count) - node_power_action_mock.assert_has_calls([ - mock.call(task, states.POWER_OFF), - mock.call(task, states.POWER_ON)]) - remove_provisioning_net_mock.assert_called_once_with(mock.ANY, - task) - configure_tenant_net_mock.assert_called_once_with(mock.ANY, task) - self.assertEqual(states.DEPLOYFAIL, task.node.provision_state) - self.assertEqual(states.ACTIVE, task.node.target_provision_state) - self.assertFalse(mock_collect.called) - - @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', - autospec=True) @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @mock.patch.object(agent_client.AgentClient, 'sync', spec=types.FunctionType) - def test_reboot_and_finish_deploy_power_action_oob_power_off( - self, sync_mock, node_power_action_mock, mock_collect, - resume_mock): + def test_tear_down_agent_power_action_oob_power_off( + self, sync_mock, node_power_action_mock, mock_collect): # Enable force power off driver_info = self.node.driver_info driver_info['deploy_forces_oob_reboot'] = True @@ -1158,30 +1056,23 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): 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=True) as task: - self.deploy.reboot_and_finish_deploy(task) + with task_manager.acquire(self.context, self.node.uuid) as task: + self.deploy.tear_down_agent(task) sync_mock.assert_called_once_with(task.node) - node_power_action_mock.assert_has_calls([ - mock.call(task, states.POWER_OFF), - mock.call(task, states.POWER_ON), - ]) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) + node_power_action_mock.assert_called_once_with(task, + states.POWER_OFF) + self.assertEqual(states.DEPLOYING, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) self.assertFalse(mock_collect.called) - resume_mock.assert_called_once_with(task) - @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', - autospec=True) @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @mock.patch.object(agent_base.LOG, 'warning', autospec=True) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @mock.patch.object(agent_client.AgentClient, 'sync', spec=types.FunctionType) - def test_reboot_and_finish_deploy_power_action_oob_power_off_failed( - self, sync_mock, node_power_action_mock, log_mock, mock_collect, - resume_mock): + def test_tear_down_agent_power_action_oob_power_off_failed( + self, sync_mock, node_power_action_mock, log_mock, mock_collect): # Enable force power off driver_info = self.node.driver_info driver_info['deploy_forces_oob_reboot'] = True @@ -1190,17 +1081,16 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): 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=True) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: + log_mock.reset_mock() + sync_mock.return_value = {'faultstring': 'Unknown command: blah'} - self.deploy.reboot_and_finish_deploy(task) + self.deploy.tear_down_agent(task) sync_mock.assert_called_once_with(task.node) - node_power_action_mock.assert_has_calls([ - mock.call(task, states.POWER_OFF), - mock.call(task, states.POWER_ON), - ]) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) + node_power_action_mock.assert_called_once_with(task, + states.POWER_OFF) + self.assertEqual(states.DEPLOYING, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) log_error = ('The version of the IPA ramdisk used in the ' 'deployment do not support the command "sync"') @@ -1210,6 +1100,51 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): {'node': task.node.uuid, 'error': log_error}) self.assertFalse(mock_collect.called) + @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('ironic.drivers.modules.network.noop.NoopNetwork.' + 'remove_provisioning_network', spec_set=True, autospec=True) + @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' + 'configure_tenant_networks', spec_set=True, autospec=True) + def test_switch_to_tenant_network(self, configure_tenant_net_mock, + remove_provisioning_net_mock, + power_on_node_if_needed_mock, + restore_power_state_mock): + power_on_node_if_needed_mock.return_value = states.POWER_OFF + 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: + self.deploy.switch_to_tenant_network(task) + remove_provisioning_net_mock.assert_called_once_with(mock.ANY, + task) + configure_tenant_net_mock.assert_called_once_with(mock.ANY, task) + 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(driver_utils, 'collect_ramdisk_logs', autospec=True) + @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' + 'remove_provisioning_network', spec_set=True, autospec=True) + @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' + 'configure_tenant_networks', spec_set=True, autospec=True) + def test_switch_to_tenant_network_fails(self, configure_tenant_net_mock, + remove_provisioning_net_mock, + mock_collect): + 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: + configure_tenant_net_mock.side_effect = exception.NetworkError( + "boom") + self.assertRaises(exception.InstanceDeployFailure, + self.deploy.switch_to_tenant_network, task) + remove_provisioning_net_mock.assert_called_once_with(mock.ANY, + task) + configure_tenant_net_mock.assert_called_once_with(mock.ANY, task) + self.assertFalse(mock_collect.called) + @mock.patch.object(agent_client.AgentClient, 'install_bootloader', autospec=True) @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) @@ -2107,46 +2042,6 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): hook_returned = agent_base._get_post_step_hook(self.node, 'clean') self.assertIsNone(hook_returned) - @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(manager_utils, 'notify_conductor_resume_deploy', - autospec=True) - @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) - @mock.patch.object(time, 'sleep', lambda seconds: None) - @mock.patch.object(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(fake.FakePower, 'get_power_state', - spec=types.FunctionType) - @mock.patch.object(agent_client.AgentClient, 'power_off', - spec=types.FunctionType) - def test_reboot_and_finish_deploy_with_smartnic_port( - self, power_off_mock, get_power_state_mock, - node_power_action_mock, collect_mock, resume_mock, - power_on_node_if_needed_mock, restore_power_state_mock): - cfg.CONF.set_override('deploy_logs_collect', 'always', 'agent') - self.node.provision_state = states.DEPLOYING - self.node.target_provision_state = states.ACTIVE - self.node.deploy_step = { - 'step': 'deploy', 'priority': 50, 'interface': 'deploy'} - self.node.save() - with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: - get_power_state_mock.side_effect = [states.POWER_ON, - states.POWER_OFF] - power_on_node_if_needed_mock.return_value = states.POWER_OFF - self.deploy.reboot_and_finish_deploy(task) - power_off_mock.assert_called_once_with(task.node) - self.assertEqual(2, get_power_state_mock.call_count) - node_power_action_mock.assert_called_once_with( - task, states.POWER_ON) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) - self.assertEqual(states.ACTIVE, task.node.target_provision_state) - collect_mock.assert_called_once_with(task.node) - resume_mock.assert_called_once_with(task) - power_on_node_if_needed_mock.assert_called_once_with(task) - restore_power_state_mock.assert_called_once_with( - task, states.POWER_OFF) - class TestRefreshCleanSteps(AgentDeployMixinBaseTest): @@ -2244,6 +2139,7 @@ class TestRefreshCleanSteps(AgentDeployMixinBaseTest): with task_manager.acquire( self.context, self.node.uuid, shared=False) as task: + log_mock.reset_mock() self.deploy.refresh_steps(task, 'deploy') client_mock.assert_called_once_with(mock.ANY, task.node, @@ -2252,7 +2148,7 @@ class TestRefreshCleanSteps(AgentDeployMixinBaseTest): task.node.driver_internal_info) self.assertIsNone(task.node.driver_internal_info.get( 'agent_cached_deploy_steps')) - self.assertFalse(log_mock.called) + log_mock.assert_not_called() @mock.patch.object(agent_client.AgentClient, 'get_clean_steps', autospec=True) @@ -2390,18 +2286,35 @@ class StepMethodsTestCase(db_base.DbTestCase): 'agent_cached_deploy_steps': self.clean_steps } steps = self.deploy.get_deploy_steps(task) - # 2 in-band steps + one out-of-band - self.assertEqual(3, len(steps)) - self.assertIn(self.clean_steps['deploy'][0], steps) - self.assertIn(self.clean_steps['deploy'][1], steps) - self.assertNotIn(self.clean_steps['raid'][0], steps) + # 2 in-band steps + 3 out-of-band + expected = [ + {'step': 'deploy', 'priority': 100, 'argsinfo': None, + 'interface': 'deploy'}, + {'step': 'tear_down_agent', 'priority': 40, 'argsinfo': None, + 'interface': 'deploy'}, + {'step': 'switch_to_tenant_network', 'priority': 30, + 'argsinfo': None, 'interface': 'deploy'}, + {'step': 'boot_instance', 'priority': 20, 'argsinfo': None, + 'interface': 'deploy'}, + ] + self.clean_steps['deploy'] + self.assertCountEqual(expected, steps) def test_get_deploy_steps_only_oob(self): with task_manager.acquire( self.context, self.node.uuid, shared=False) as task: steps = self.deploy.get_deploy_steps(task) - # one out-of-band step - self.assertEqual(1, len(steps)) + # three base out-of-band steps + expected = [ + {'step': 'deploy', 'priority': 100, 'argsinfo': None, + 'interface': 'deploy'}, + {'step': 'tear_down_agent', 'priority': 40, 'argsinfo': None, + 'interface': 'deploy'}, + {'step': 'switch_to_tenant_network', 'priority': 30, + 'argsinfo': None, 'interface': 'deploy'}, + {'step': 'boot_instance', 'priority': 20, 'argsinfo': None, + 'interface': 'deploy'}, + ] + self.assertCountEqual(expected, steps) @mock.patch('ironic.objects.Port.list_by_node_id', spec_set=types.FunctionType) diff --git a/releasenotes/notes/in-band-steps-e4a1fe759029fea5.yaml b/releasenotes/notes/in-band-steps-e4a1fe759029fea5.yaml new file mode 100644 index 0000000000..d3867a3446 --- /dev/null +++ b/releasenotes/notes/in-band-steps-e4a1fe759029fea5.yaml @@ -0,0 +1,23 @@ +--- +features: + - | + Adds support for running custom in-band deploy steps when provisioning. + Step priorities from 41 to 59 can be used for steps that run after + the image is written and the bootloader is installed. +deprecations: + - | + Running the whole deployment process as a monolithic ``deploy.deploy`` + deploy step is now deprecated. In a future release this step will only be + used to prepare deployment and starting the agent, and special handling + will be removed. All third party deploy interfaces must be updated + to provide real deploy steps instead and set the + ``has_decomposed_deploy_steps`` attribute to ``True`` on the deploy + interface level. +other: + - | + As part of the agent deploy interfaces refactoring, breaking changes will + be made to implementations of ``AgentDeploy`` and ``ISCSIDeploy``. + Third party deploy interfaces must be updated to inherit + ``HeartbeatMixin``, ``AgentBaseMixin`` or ``AgentDeployMixin`` + from ``ironic.drivers.modules.agent_base`` instead since their API is + considered more stable.