From 4d3f1b40e2977d905e8f244a63675fc2fd2bdc3f Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 19 Feb 2020 17:16:14 +0100 Subject: [PATCH] Refactoring: move generic agent clean step functions to agent_base * Clean steps methods from deploy_utils * Identical implementations from agent.py and iscsi_deploy.py Change-Id: I45756a43bf33037281c919e2bc418afdf0e655d0 --- ironic/drivers/modules/agent.py | 35 +---- ironic/drivers/modules/agent_base.py | 98 ++++++++++++++ ironic/drivers/modules/deploy_utils.py | 67 ---------- ironic/drivers/modules/ilo/management.py | 2 +- ironic/drivers/modules/iscsi_deploy.py | 31 ----- .../drivers/modules/ilo/test_management.py | 10 +- .../tests/unit/drivers/modules/test_agent.py | 26 ++-- .../unit/drivers/modules/test_agent_base.py | 126 ++++++++++++++++++ .../unit/drivers/modules/test_deploy_utils.py | 98 -------------- .../unit/drivers/modules/test_iscsi_deploy.py | 5 +- 10 files changed, 242 insertions(+), 256 deletions(-) diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 4e5e53b719..00c8a9021d 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -637,37 +637,6 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface): """ pass - @METRICS.timer('AgentDeploy.get_clean_steps') - def get_clean_steps(self, task): - """Get the list of clean steps from the agent. - - :param task: a TaskManager object containing the node - :raises NodeCleaningFailure: if the clean steps are not yet - available (cached), for example, when a node has just been - enrolled and has not been cleaned yet. - :returns: A list of clean step dictionaries - """ - new_priorities = { - 'erase_devices': CONF.deploy.erase_devices_priority, - 'erase_devices_metadata': - CONF.deploy.erase_devices_metadata_priority, - } - return deploy_utils.agent_get_clean_steps( - task, interface='deploy', - override_priorities=new_priorities) - - @METRICS.timer('AgentDeploy.execute_clean_step') - def execute_clean_step(self, task, step): - """Execute a clean step asynchronously on the agent. - - :param task: a TaskManager object containing the node - :param step: a clean step dictionary to execute - :raises: NodeCleaningFailure if the agent does not return a command - status - :returns: states.CLEANWAIT to signify the step will be completed async - """ - return deploy_utils.agent_execute_clean_step(task, step) - @METRICS.timer('AgentDeploy.prepare_cleaning') def prepare_cleaning(self, task): """Boot into the agent to prepare for cleaning. @@ -744,7 +713,7 @@ class AgentRAID(base.RAIDInterface): "with the following target RAID configuration: %(target)s", {'node': node.uuid, 'target': target_raid_config}) step = node.clean_step - return deploy_utils.agent_execute_clean_step(task, step) + return agent_base.execute_clean_step(task, step) @staticmethod @agent_base.post_clean_step_hook( @@ -787,7 +756,7 @@ class AgentRAID(base.RAIDInterface): LOG.debug("Agent RAID delete_configuration invoked for node %s.", task.node.uuid) step = task.node.clean_step - return deploy_utils.agent_execute_clean_step(task, step) + return agent_base.execute_clean_step(task, step) @staticmethod @agent_base.post_clean_step_hook( diff --git a/ironic/drivers/modules/agent_base.py b/ironic/drivers/modules/agent_base.py index fd0246fd82..f7975c25ce 100644 --- a/ironic/drivers/modules/agent_base.py +++ b/ironic/drivers/modules/agent_base.py @@ -36,6 +36,7 @@ from ironic.drivers.modules import agent_client from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import deploy_utils from ironic.drivers import utils as driver_utils +from ironic import objects LOG = log.getLogger(__name__) @@ -232,6 +233,72 @@ def log_and_raise_deployment_error(task, msg, collect_logs=True, exc=None): raise exception.InstanceDeployFailure(msg) +def get_clean_steps(task, interface=None, override_priorities=None): + """Get the list of cached clean steps from the agent. + + #TODO(JoshNang) move to BootInterface + + The clean steps cache is updated at the beginning of cleaning. + + :param task: a TaskManager object containing the node + :param interface: The interface for which clean steps + are to be returned. If this is not provided, it returns the + clean steps for all interfaces. + :param override_priorities: a dictionary with keys being step names and + values being new priorities for them. If a step isn't in this + dictionary, the step's original priority is used. + :raises NodeCleaningFailure: if the clean steps are not yet cached, + for example, when a node has just been enrolled and has not been + cleaned yet. + :returns: A list of clean step dictionaries + """ + node = task.node + try: + all_steps = node.driver_internal_info['agent_cached_clean_steps'] + except KeyError: + raise exception.NodeCleaningFailure(_('Cleaning steps are not yet ' + 'available for node %(node)s') + % {'node': node.uuid}) + + if interface: + steps = [step.copy() for step in all_steps.get(interface, [])] + else: + steps = [step.copy() for step_list in all_steps.values() + for step in step_list] + + if not steps or not override_priorities: + return steps + + for step in steps: + new_priority = override_priorities.get(step.get('step')) + if new_priority is not None: + step['priority'] = new_priority + + return steps + + +def execute_clean_step(task, step): + """Execute a clean step asynchronously on the agent. + + #TODO(JoshNang) move to BootInterface + + :param task: a TaskManager object containing the node + :param step: a clean step dictionary to execute + :raises: NodeCleaningFailure if the agent does not return a command status + :returns: states.CLEANWAIT to signify the step will be completed async + """ + client = _get_client() + ports = objects.Port.list_by_node_id( + task.context, task.node.id) + result = client.execute_clean_step(step, task.node, ports) + if not result.get('command_status'): + raise exception.NodeCleaningFailure(_( + 'Agent on node %(node)s returned bad command result: ' + '%(result)s') % {'node': task.node.uuid, + 'result': result.get('command_error')}) + return states.CLEANWAIT + + class HeartbeatMixin(object): """Mixin class implementing heartbeat processing.""" @@ -478,6 +545,25 @@ class HeartbeatMixin(object): class AgentDeployMixin(HeartbeatMixin): """Mixin with deploy methods.""" + @METRICS.timer('AgentDeployMixin.get_clean_steps') + def get_clean_steps(self, task): + """Get the list of clean steps from the agent. + + :param task: a TaskManager object containing the node + :raises NodeCleaningFailure: if the clean steps are not yet + available (cached), for example, when a node has just been + enrolled and has not been cleaned yet. + :returns: A list of clean step dictionaries + """ + new_priorities = { + 'erase_devices': CONF.deploy.erase_devices_priority, + 'erase_devices_metadata': + CONF.deploy.erase_devices_metadata_priority, + } + return get_clean_steps( + task, interface='deploy', + override_priorities=new_priorities) + @METRICS.timer('AgentDeployMixin.refresh_clean_steps') def refresh_clean_steps(self, task): """Refresh the node's cached clean steps from the booted agent. @@ -535,6 +621,18 @@ class AgentDeployMixin(HeartbeatMixin): LOG.debug('Refreshed agent clean step cache for node %(node)s: ' '%(steps)s', {'node': node.uuid, 'steps': steps}) + @METRICS.timer('AgentDeployMixin.execute_clean_step') + def execute_clean_step(self, task, step): + """Execute a clean step asynchronously on the agent. + + :param task: a TaskManager object containing the node + :param step: a clean step dictionary to execute + :raises: NodeCleaningFailure if the agent does not return a command + status + :returns: states.CLEANWAIT to signify the step will be completed async + """ + return execute_clean_step(task, step) + @METRICS.timer('AgentDeployMixin.continue_cleaning') def continue_cleaning(self, task, **kwargs): """Start the next cleaning step if the previous one is complete. diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 1d6e79247d..7b3bcd852c 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -40,7 +40,6 @@ from ironic.common import states from ironic.common import utils from ironic.conductor import utils as manager_utils from ironic.conf import CONF -from ironic.drivers.modules import agent_client from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import image_cache from ironic.drivers import utils as driver_utils @@ -579,72 +578,6 @@ def get_single_nic_with_vif_port_id(task): return port.address -def agent_get_clean_steps(task, interface=None, override_priorities=None): - """Get the list of cached clean steps from the agent. - - #TODO(JoshNang) move to BootInterface - - The clean steps cache is updated at the beginning of cleaning. - - :param task: a TaskManager object containing the node - :param interface: The interface for which clean steps - are to be returned. If this is not provided, it returns the - clean steps for all interfaces. - :param override_priorities: a dictionary with keys being step names and - values being new priorities for them. If a step isn't in this - dictionary, the step's original priority is used. - :raises NodeCleaningFailure: if the clean steps are not yet cached, - for example, when a node has just been enrolled and has not been - cleaned yet. - :returns: A list of clean step dictionaries - """ - node = task.node - try: - all_steps = node.driver_internal_info['agent_cached_clean_steps'] - except KeyError: - raise exception.NodeCleaningFailure(_('Cleaning steps are not yet ' - 'available for node %(node)s') - % {'node': node.uuid}) - - if interface: - steps = [step.copy() for step in all_steps.get(interface, [])] - else: - steps = [step.copy() for step_list in all_steps.values() - for step in step_list] - - if not steps or not override_priorities: - return steps - - for step in steps: - new_priority = override_priorities.get(step.get('step')) - if new_priority is not None: - step['priority'] = new_priority - - return steps - - -def agent_execute_clean_step(task, step): - """Execute a clean step asynchronously on the agent. - - #TODO(JoshNang) move to BootInterface - - :param task: a TaskManager object containing the node - :param step: a clean step dictionary to execute - :raises: NodeCleaningFailure if the agent does not return a command status - :returns: states.CLEANWAIT to signify the step will be completed async - """ - client = agent_client.AgentClient() - ports = objects.Port.list_by_node_id( - task.context, task.node.id) - result = client.execute_clean_step(step, task.node, ports) - if not result.get('command_status'): - raise exception.NodeCleaningFailure(_( - 'Agent on node %(node)s returned bad command result: ' - '%(result)s') % {'node': task.node.uuid, - 'result': result.get('command_error')}) - return states.CLEANWAIT - - def agent_add_clean_params(task): """Add required config parameters to node's driver_internal_info. diff --git a/ironic/drivers/modules/ilo/management.py b/ironic/drivers/modules/ilo/management.py index 31f61c23c4..e92fbcd2bc 100644 --- a/ironic/drivers/modules/ilo/management.py +++ b/ironic/drivers/modules/ilo/management.py @@ -543,7 +543,7 @@ class IloManagement(base.ManagementInterface): ilo_common.attach_vmedia(node, 'CDROM', url) step = node.clean_step - return deploy_utils.agent_execute_clean_step(task, step) + return agent_base.execute_clean_step(task, step) @staticmethod @agent_base.post_clean_step_hook( diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index ffea163f0e..cb885ae51d 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -549,37 +549,6 @@ class ISCSIDeploy(AgentDeployMixin, base.DeployInterface): def take_over(self, task): pass - @METRICS.timer('ISCSIDeploy.get_clean_steps') - def get_clean_steps(self, task): - """Get the list of clean steps from the agent. - - :param task: a TaskManager object containing the node - :raises NodeCleaningFailure: if the clean steps are not yet - available (cached), for example, when a node has just been - enrolled and has not been cleaned yet. - :returns: A list of clean step dictionaries. - """ - steps = deploy_utils.agent_get_clean_steps( - task, interface='deploy', - override_priorities={ - 'erase_devices': CONF.deploy.erase_devices_priority, - 'erase_devices_metadata': - CONF.deploy.erase_devices_metadata_priority}) - return steps - - @METRICS.timer('ISCSIDeploy.execute_clean_step') - def execute_clean_step(self, task, step): - """Execute a clean step asynchronously on the agent. - - :param task: a TaskManager object containing the node - :param step: a clean step dictionary to execute - :raises: NodeCleaningFailure if the agent does not return a command - status - :returns: states.CLEANWAIT to signify the step will be completed - asynchronously. - """ - return deploy_utils.agent_execute_clean_step(task, step) - @METRICS.timer('ISCSIDeploy.prepare_cleaning') def prepare_cleaning(self, task): """Boot into the agent to prepare for cleaning. diff --git a/ironic/tests/unit/drivers/modules/ilo/test_management.py b/ironic/tests/unit/drivers/modules/ilo/test_management.py index 13623bfa54..fd59abbceb 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_management.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_management.py @@ -23,6 +23,7 @@ from ironic.common import exception from ironic.common import states from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils +from ironic.drivers.modules import agent_base from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.ilo import boot as ilo_boot from ironic.drivers.modules.ilo import common as ilo_common @@ -822,8 +823,7 @@ class IloManagementTestCase(test_common.BaseIloTest): @mock.patch.object(ilo_common, 'attach_vmedia', spec_set=True, autospec=True) - @mock.patch.object(deploy_utils, 'agent_execute_clean_step', - autospec=True) + @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) def test_update_firmware_sum_mode_with_component( self, execute_mock, attach_vmedia_mock): with task_manager.acquire(self.context, self.node.uuid, @@ -851,8 +851,7 @@ class IloManagementTestCase(test_common.BaseIloTest): autospec=True) @mock.patch.object(ilo_management.firmware_processor, 'get_swift_url', autospec=True) - @mock.patch.object(deploy_utils, 'agent_execute_clean_step', - autospec=True) + @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) def test_update_firmware_sum_mode_swift_url( self, execute_mock, swift_url_mock, attach_vmedia_mock): with task_manager.acquire(self.context, self.node.uuid, @@ -880,8 +879,7 @@ class IloManagementTestCase(test_common.BaseIloTest): @mock.patch.object(ilo_common, 'attach_vmedia', spec_set=True, autospec=True) - @mock.patch.object(deploy_utils, 'agent_execute_clean_step', - autospec=True) + @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) def test_update_firmware_sum_mode_without_component( self, execute_mock, attach_vmedia_mock): with task_manager.acquire(self.context, self.node.uuid, diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index f161a83d4c..aa3a1091a6 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -973,8 +973,7 @@ class TestAgentDeploy(db_base.DbTestCase): set_dhcp_provider_mock.assert_called_once_with() clean_dhcp_mock.assert_called_once_with(task) - @mock.patch('ironic.drivers.modules.deploy_utils.agent_get_clean_steps', - autospec=True) + @mock.patch.object(agent_base, 'get_clean_steps', autospec=True) def test_get_clean_steps(self, mock_get_clean_steps): # Test getting clean steps mock_steps = [{'priority': 10, 'interface': 'deploy', @@ -988,8 +987,7 @@ class TestAgentDeploy(db_base.DbTestCase): 'erase_devices_metadata': None}) self.assertEqual(mock_steps, steps) - @mock.patch('ironic.drivers.modules.deploy_utils.agent_get_clean_steps', - autospec=True) + @mock.patch.object(agent_base, 'get_clean_steps', autospec=True) def test_get_clean_steps_config_priority(self, mock_get_clean_steps): # Test that we can override the priority of get clean steps # Use 0 because it is an edge case (false-y) and used in devstack @@ -1690,7 +1688,7 @@ class AgentRAIDTestCase(db_base.DbTestCase): } self.node = object_utils.create_test_node(self.context, **n) - @mock.patch.object(deploy_utils, 'agent_get_clean_steps', autospec=True) + @mock.patch.object(agent_base, 'get_clean_steps', autospec=True) def test_get_clean_steps(self, get_steps_mock): get_steps_mock.return_value = [ {'step': 'create_configuration', 'interface': 'raid', @@ -1705,8 +1703,7 @@ class AgentRAIDTestCase(db_base.DbTestCase): self.assertEqual(0, ret[1]['priority']) @mock.patch.object(raid, 'filter_target_raid_config') - @mock.patch.object(deploy_utils, 'agent_execute_clean_step', - autospec=True) + @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) def test_create_configuration(self, execute_mock, filter_target_raid_config_mock): with task_manager.acquire(self.context, self.node.uuid) as task: @@ -1722,8 +1719,7 @@ class AgentRAIDTestCase(db_base.DbTestCase): execute_mock.assert_called_once_with(task, self.clean_step) @mock.patch.object(raid, 'filter_target_raid_config') - @mock.patch.object(deploy_utils, 'agent_execute_clean_step', - autospec=True) + @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) def test_create_configuration_skip_root(self, execute_mock, filter_target_raid_config_mock): with task_manager.acquire(self.context, self.node.uuid) as task: @@ -1743,8 +1739,7 @@ class AgentRAIDTestCase(db_base.DbTestCase): task.node.driver_internal_info['target_raid_config']) @mock.patch.object(raid, 'filter_target_raid_config') - @mock.patch.object(deploy_utils, 'agent_execute_clean_step', - autospec=True) + @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) def test_create_configuration_skip_nonroot(self, execute_mock, filter_target_raid_config_mock): with task_manager.acquire(self.context, self.node.uuid) as task: @@ -1764,8 +1759,7 @@ class AgentRAIDTestCase(db_base.DbTestCase): task.node.driver_internal_info['target_raid_config']) @mock.patch.object(raid, 'filter_target_raid_config') - @mock.patch.object(deploy_utils, 'agent_execute_clean_step', - autospec=True) + @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) def test_create_configuration_no_target_raid_config_after_skipping( self, execute_mock, filter_target_raid_config_mock): with task_manager.acquire(self.context, self.node.uuid) as task: @@ -1780,8 +1774,7 @@ class AgentRAIDTestCase(db_base.DbTestCase): self.assertFalse(execute_mock.called) @mock.patch.object(raid, 'filter_target_raid_config') - @mock.patch.object(deploy_utils, 'agent_execute_clean_step', - autospec=True) + @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) def test_create_configuration_empty_target_raid_config( self, execute_mock, filter_target_raid_config_mock): execute_mock.return_value = states.CLEANING @@ -1827,8 +1820,7 @@ class AgentRAIDTestCase(db_base.DbTestCase): task, command) self.assertFalse(update_raid_info_mock.called) - @mock.patch.object(deploy_utils, 'agent_execute_clean_step', - autospec=True) + @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) def test_delete_configuration(self, execute_mock): execute_mock.return_value = states.CLEANING with task_manager.acquire(self.context, self.node.uuid) as task: diff --git a/ironic/tests/unit/drivers/modules/test_agent_base.py b/ironic/tests/unit/drivers/modules/test_agent_base.py index 457452892d..6927391f22 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base.py @@ -18,6 +18,7 @@ import types import mock from oslo_config import cfg +from testtools import matchers from ironic.common import boot_devices from ironic.common import exception @@ -2073,3 +2074,128 @@ class TestRefreshCleanSteps(AgentDeployMixinBaseTest): task) client_mock.assert_called_once_with(mock.ANY, task.node, task.ports) + + +class CleanStepMethodsTestCase(db_base.DbTestCase): + + def setUp(self): + super(CleanStepMethodsTestCase, self).setUp() + + self.clean_steps = { + 'deploy': [ + {'interface': 'deploy', + 'step': 'erase_devices', + 'priority': 20}, + {'interface': 'deploy', + 'step': 'update_firmware', + 'priority': 30} + ], + 'raid': [ + {'interface': 'raid', + 'step': 'create_configuration', + 'priority': 10} + ] + } + n = {'boot_interface': 'pxe', + 'deploy_interface': 'direct', + 'driver_internal_info': { + 'agent_cached_clean_steps': self.clean_steps}} + self.node = object_utils.create_test_node(self.context, **n) + self.ports = [object_utils.create_test_port(self.context, + node_id=self.node.id)] + + def test_agent_get_clean_steps(self): + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + response = agent_base.get_clean_steps(task) + + # Since steps are returned in dicts, they have non-deterministic + # ordering + self.assertThat(response, matchers.HasLength(3)) + self.assertIn(self.clean_steps['deploy'][0], response) + self.assertIn(self.clean_steps['deploy'][1], response) + self.assertIn(self.clean_steps['raid'][0], response) + + def test_get_clean_steps_custom_interface(self): + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + response = agent_base.get_clean_steps(task, interface='raid') + self.assertThat(response, matchers.HasLength(1)) + self.assertEqual(self.clean_steps['raid'], response) + + def test_get_clean_steps_override_priorities(self): + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + new_priorities = {'create_configuration': 42} + response = agent_base.get_clean_steps( + task, interface='raid', override_priorities=new_priorities) + self.assertEqual(42, response[0]['priority']) + + def test_get_clean_steps_override_priorities_none(self): + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + # this is simulating the default value of a configuration option + new_priorities = {'create_configuration': None} + response = agent_base.get_clean_steps( + task, interface='raid', override_priorities=new_priorities) + self.assertEqual(10, response[0]['priority']) + + def test_get_clean_steps_missing_steps(self): + info = self.node.driver_internal_info + del info['agent_cached_clean_steps'] + self.node.driver_internal_info = info + self.node.save() + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.assertRaises(exception.NodeCleaningFailure, + agent_base.get_clean_steps, + task) + + @mock.patch('ironic.objects.Port.list_by_node_id', + spec_set=types.FunctionType) + @mock.patch.object(agent_client.AgentClient, 'execute_clean_step', + autospec=True) + def test_execute_clean_step(self, client_mock, list_ports_mock): + client_mock.return_value = { + 'command_status': 'SUCCEEDED'} + list_ports_mock.return_value = self.ports + + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + response = agent_base.execute_clean_step( + task, + self.clean_steps['deploy'][0]) + self.assertEqual(states.CLEANWAIT, response) + + @mock.patch('ironic.objects.Port.list_by_node_id', + spec_set=types.FunctionType) + @mock.patch.object(agent_client.AgentClient, 'execute_clean_step', + autospec=True) + def test_execute_clean_step_running(self, client_mock, list_ports_mock): + client_mock.return_value = { + 'command_status': 'RUNNING'} + list_ports_mock.return_value = self.ports + + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + response = agent_base.execute_clean_step( + task, + self.clean_steps['deploy'][0]) + self.assertEqual(states.CLEANWAIT, response) + + @mock.patch('ironic.objects.Port.list_by_node_id', + spec_set=types.FunctionType) + @mock.patch.object(agent_client.AgentClient, 'execute_clean_step', + autospec=True) + def test_execute_clean_step_version_mismatch( + self, client_mock, list_ports_mock): + client_mock.return_value = { + 'command_status': 'RUNNING'} + list_ports_mock.return_value = self.ports + + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + response = agent_base.execute_clean_step( + task, + self.clean_steps['deploy'][0]) + self.assertEqual(states.CLEANWAIT, response) diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index fe1a8a41f4..39595ca72f 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -27,7 +27,6 @@ from oslo_config import cfg from oslo_utils import fileutils from oslo_utils import uuidutils import testtools -from testtools import matchers from ironic.common import boot_devices from ironic.common import exception @@ -37,7 +36,6 @@ from ironic.common import states from ironic.common import utils as common_utils from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils -from ironic.drivers.modules import agent_client from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import deploy_utils as utils from ironic.drivers.modules import fake @@ -1627,102 +1625,6 @@ class AgentMethodsTestCase(db_base.DbTestCase): self.ports = [obj_utils.create_test_port(self.context, node_id=self.node.id)] - def test_agent_get_clean_steps(self): - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - response = utils.agent_get_clean_steps(task) - - # Since steps are returned in dicts, they have non-deterministic - # ordering - self.assertThat(response, matchers.HasLength(3)) - self.assertIn(self.clean_steps['deploy'][0], response) - self.assertIn(self.clean_steps['deploy'][1], response) - self.assertIn(self.clean_steps['raid'][0], response) - - def test_get_clean_steps_custom_interface(self): - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - response = utils.agent_get_clean_steps(task, interface='raid') - self.assertThat(response, matchers.HasLength(1)) - self.assertEqual(self.clean_steps['raid'], response) - - def test_get_clean_steps_override_priorities(self): - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - new_priorities = {'create_configuration': 42} - response = utils.agent_get_clean_steps( - task, interface='raid', override_priorities=new_priorities) - self.assertEqual(42, response[0]['priority']) - - def test_get_clean_steps_override_priorities_none(self): - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - # this is simulating the default value of a configuration option - new_priorities = {'create_configuration': None} - response = utils.agent_get_clean_steps( - task, interface='raid', override_priorities=new_priorities) - self.assertEqual(10, response[0]['priority']) - - def test_get_clean_steps_missing_steps(self): - info = self.node.driver_internal_info - del info['agent_cached_clean_steps'] - self.node.driver_internal_info = info - self.node.save() - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - self.assertRaises(exception.NodeCleaningFailure, - utils.agent_get_clean_steps, - task) - - @mock.patch('ironic.objects.Port.list_by_node_id', - spec_set=types.FunctionType) - @mock.patch.object(agent_client.AgentClient, 'execute_clean_step', - autospec=True) - def test_execute_clean_step(self, client_mock, list_ports_mock): - client_mock.return_value = { - 'command_status': 'SUCCEEDED'} - list_ports_mock.return_value = self.ports - - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - response = utils.agent_execute_clean_step( - task, - self.clean_steps['deploy'][0]) - self.assertEqual(states.CLEANWAIT, response) - - @mock.patch('ironic.objects.Port.list_by_node_id', - spec_set=types.FunctionType) - @mock.patch.object(agent_client.AgentClient, 'execute_clean_step', - autospec=True) - def test_execute_clean_step_running(self, client_mock, list_ports_mock): - client_mock.return_value = { - 'command_status': 'RUNNING'} - list_ports_mock.return_value = self.ports - - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - response = utils.agent_execute_clean_step( - task, - self.clean_steps['deploy'][0]) - self.assertEqual(states.CLEANWAIT, response) - - @mock.patch('ironic.objects.Port.list_by_node_id', - spec_set=types.FunctionType) - @mock.patch.object(agent_client.AgentClient, 'execute_clean_step', - autospec=True) - def test_execute_clean_step_version_mismatch( - self, client_mock, list_ports_mock): - client_mock.return_value = { - 'command_status': 'RUNNING'} - list_ports_mock.return_value = self.ports - - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - response = utils.agent_execute_clean_step( - task, - self.clean_steps['deploy'][0]) - self.assertEqual(states.CLEANWAIT, response) - def test_agent_add_clean_params(self): cfg.CONF.set_override('shred_random_overwrite_iterations', 2, 'deploy') cfg.CONF.set_override('shred_final_overwrite_with_zeros', False, diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index fb7c3f770b..adf4fc2471 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -963,8 +963,7 @@ class ISCSIDeployTestCase(db_base.DbTestCase): tear_down_cleaning_mock.assert_called_once_with( task, manage_boot=True) - @mock.patch('ironic.drivers.modules.deploy_utils.agent_get_clean_steps', - autospec=True) + @mock.patch.object(agent_base, 'get_clean_steps', autospec=True) def test_get_clean_steps(self, mock_get_clean_steps): # Test getting clean steps self.config(group='deploy', erase_devices_priority=10) @@ -983,7 +982,7 @@ class ISCSIDeployTestCase(db_base.DbTestCase): 'erase_devices_metadata': 5}) self.assertEqual(mock_steps, steps) - @mock.patch.object(deploy_utils, 'agent_execute_clean_step', autospec=True) + @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) def test_execute_clean_step(self, agent_execute_clean_step_mock): with task_manager.acquire(self.context, self.node.uuid) as task: task.driver.deploy.execute_clean_step(