From deec7f4a926b7e032a44a0c56ac4016bf250ef54 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur <dtantsur@protonmail.com> Date: Wed, 24 Jun 2020 09:20:51 +0200 Subject: [PATCH] agent_base: support inserting in-band deploy steps Currently all almost all of the deployment process is contained in a single deploy step called deploy, on the deploy interface. This restricts the customisation that can be applied via deploy steps, since steps may only be added before or after this step. This change allows deploy interfaces inheriting agent_base.AgentDeployMixin to be used with in-band deploy steps. It is implemented by decomposing the reboot_and_finish_deploy call into three deploy steps: * tear_down_agent (priority 40) * switch_to_tenant_network (priority 30) * boot_instance (priority 20) All steps with priorities between 99 and 41 can run in-band. Backwards compatibility with drivers that do not support decomposed steps is maintained via a 'has_decomposed_deploy_steps' method on the agent heartbeat mixin. The old reboot_and_finish_deploy call is also left for compatibility but does nothing since the new steps will be automatically run. Change-Id: Ie4fdd195efa941165e22bd4ce9484988a1760b2d Story: #2006963 Task: #40151 --- doc/source/admin/node-deployment.rst | 32 +- ironic/conductor/deployments.py | 7 + ironic/drivers/modules/agent.py | 1 - ironic/drivers/modules/agent_base.py | 85 +++- .../tests/unit/drivers/modules/test_agent.py | 117 +---- .../unit/drivers/modules/test_agent_base.py | 421 +++++++----------- .../notes/in-band-steps-e4a1fe759029fea5.yaml | 23 + 7 files changed, 300 insertions(+), 386 deletions(-) create mode 100644 releasenotes/notes/in-band-steps-e4a1fe759029fea5.yaml 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.