Cache agent clean steps on node

In order to make getting clean steps a synchronous call, instead of
one that blocks on communication with the agent, the agent clean steps
are cached in the node's driver_internal_info. Any time cleaning is
started, the steps will be fetched from the agent and cached.

This is needed for the 'GET /nodes/<node_ident>/cleaning/steps'
API, which is not yet implemented but see the spec:
(http://specs.openstack.org/openstack/ironic-specs/specs/approved/manual-cleaning.html#get-nodes-node-ident-cleaning-steps)

Change-Id: I26288802d06683fd99138bfea488233c88260a7f
Partial-Bug: #1526290
Co-Authored-By: Josh Gachnang <josh@pcsforeducation.com>

ironic/drivers/base.py

@@ -182,6 +182,10 @@ class BaseInterface(object):
 
         :param task: A TaskManager object, useful for interfaces overriding
             this function
+        :raises NodeCleaningFailure: if there is a problem getting the steps
+            from the driver. For example, when a node (using an agent driver)
+            has just been enrolled and the agent isn't alive yet to be queried
+            for the available clean steps.
         :returns: A list of clean step dictionaries
         """
         return self.clean_steps

ironic/drivers/modules/agent.py

@@ -359,7 +359,9 @@ class AgentDeploy(base.DeployInterface):
         """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 = {

ironic/drivers/modules/agent_base_vendor.py

@@ -16,12 +16,13 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-
+import collections
 import time
 
 from oslo_config import cfg
 from oslo_log import log
 from oslo_utils import excutils
+from oslo_utils import timeutils
 import retrying
 
 from ironic.common import boot_devices
@@ -215,6 +216,62 @@ class BaseAgentVendor(base.VendorInterface):
         task.release_resources()
         rpc.continue_node_clean(task.context, uuid, topic=topic)
 
+    def _refresh_clean_steps(self, task):
+        """Refresh the node's cached clean steps from the booted agent.
+
+        Gets the node's clean steps from the booted agent and caches them.
+        The steps are cached to make get_clean_steps() calls synchronous, and
+        should be refreshed as soon as the agent boots to start cleaning or
+        if cleaning is restarted because of a cleaning version mismatch.
+
+        :param task: a TaskManager instance
+        :raises: NodeCleaningFailure if the agent returns invalid results
+        """
+        node = task.node
+        previous_steps = node.driver_internal_info.get(
+            'agent_cached_clean_steps')
+        LOG.debug('Refreshing agent clean step cache for node %(node)s. '
+                  'Previously cached steps: %(steps)s',
+                  {'node': node.uuid, 'steps': previous_steps})
+
+        agent_result = self._client.get_clean_steps(node, task.ports).get(
+            'command_result', {})
+        missing = set(['clean_steps', 'hardware_manager_version']).difference(
+            agent_result)
+        if missing:
+            raise exception.NodeCleaningFailure(_(
+                'agent get_clean_steps for node %(node)s returned an invalid '
+                'result. Keys: %(keys)s are missing from result: %(result)s.')
+                % ({'node': node.uuid, 'keys': missing,
+                    'result': agent_result}))
+
+        # agent_result['clean_steps'] looks like
+        # {'HardwareManager': [{step1},{steps2}...], ...}
+        steps = collections.defaultdict(list)
+        for step_list in agent_result['clean_steps'].values():
+            for step in step_list:
+                missing = set(['interface', 'step', 'priority']).difference(
+                    step)
+                if missing:
+                    raise exception.NodeCleaningFailure(_(
+                        'agent get_clean_steps for node %(node)s returned an '
+                        'invalid clean step. Keys: %(keys)s are missing from '
+                        'step: %(step)s.') % ({'node': node.uuid,
+                                               'keys': missing, 'step': step}))
+
+                steps[step['interface']].append(step)
+
+        # Save hardware manager version, steps, and date
+        info = node.driver_internal_info
+        info['hardware_manager_version'] = agent_result[
+            'hardware_manager_version']
+        info['agent_cached_clean_steps'] = dict(steps)
+        info['agent_cached_clean_steps_refreshed'] = str(timeutils.utcnow())
+        node.driver_internal_info = info
+        node.save()
+        LOG.debug('Refreshed agent clean step cache for node %(node)s: '
+                  '%(steps)s', {'node': node.uuid, 'steps': steps})
+
     def continue_cleaning(self, task, **kwargs):
         """Start the next cleaning step if the previous one is complete.
 
@@ -249,6 +306,16 @@ class BaseAgentVendor(base.VendorInterface):
             LOG.error(msg)
             return manager_utils.cleaning_error_handler(task, msg)
         elif command.get('command_status') == 'CLEAN_VERSION_MISMATCH':
+            # Cache the new clean steps (and 'hardware_manager_version')
+            try:
+                self._refresh_clean_steps(task)
+            except exception.NodeCleaningFailure as e:
+                msg = (_('Could not continue cleaning on node '
+                         '%(node)s: %(err)s.') %
+                       {'node': node.uuid, 'err': e})
+                LOG.exception(msg)
+                return manager_utils.cleaning_error_handler(task, msg)
+
             if manual_clean:
                 # Don't restart manual cleaning if agent reboots to a new
                 # version. Both are operator actions, unlike automated
@@ -259,27 +326,7 @@ class BaseAgentVendor(base.VendorInterface):
                              'continuing from current step %(step)s.'),
                          {'node': node.uuid, 'step': node.clean_step})
 
-                result = self._client.get_clean_steps(
-                    task.node, task.ports).get('command_result')
-
-                required_keys = ('clean_steps', 'hardware_manager_version')
-                missing_keys = [key for key in required_keys
-                                if key not in result]
-                if missing_keys:
-                    msg = (_('Could not continue manual cleaning from step '
-                             '%(step)s on node %(node)s. get_clean_steps '
-                             'returned invalid result. The keys %(keys)s are '
-                             'missing from result %(result)s.') %
-                           {'node': node.uuid,
-                            'step': node.clean_step,
-                            'keys': missing_keys,
-                            'result': result})
-                    LOG.error(msg)
-                    return manager_utils.cleaning_error_handler(task, msg)
-
                 driver_internal_info = node.driver_internal_info
-                driver_internal_info['hardware_manager_version'] = result[
-                    'hardware_manager_version']
                 driver_internal_info['skip_current_clean_step'] = False
                 node.driver_internal_info = driver_internal_info
                 node.save()
@@ -397,6 +444,9 @@ class BaseAgentVendor(base.VendorInterface):
                                   node.uuid)
                         msg = _('Node failed to start the first cleaning '
                                 'step.')
+                        # First, cache the clean steps
+                        self._refresh_clean_steps(task)
+                        # Then set/verify node clean steps and start cleaning
                         manager_utils.set_node_cleaning_steps(task)
                         self.notify_conductor_resume_clean(task)
                     else:

ironic/drivers/modules/deploy_utils.py

@@ -550,10 +550,12 @@ def parse_instance_info_capabilities(node):
 
 
 def agent_get_clean_steps(task, interface=None, override_priorities=None):
-    """Get the list of clean steps from the agent.
+    """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
@@ -561,42 +563,34 @@ def agent_get_clean_steps(task, interface=None, override_priorities=None):
     :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 agent returns invalid results
+    :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
     """
-    override_priorities = override_priorities or {}
+    node = task.node
-    client = agent_client.AgentClient()
+    try:
-    ports = objects.Port.list_by_node_id(
+        all_steps = node.driver_internal_info['agent_cached_clean_steps']
-        task.context, task.node.id)
+    except KeyError:
-    result = client.get_clean_steps(task.node, ports).get('command_result')
+        raise exception.NodeCleaningFailure(_('Cleaning steps are not yet '
+                                              'available for node %(node)s')
+                                            % {'node': node.uuid})
 
-    if ('clean_steps' not in result or
+    if interface:
-            'hardware_manager_version' not in result):
+        steps = [step.copy() for step in all_steps.get(interface, [])]
-        raise exception.NodeCleaningFailure(_(
+    else:
-            'get_clean_steps for node %(node)s returned invalid result:'
+        steps = [step.copy() for step_list in all_steps.values()
-            ' %(result)s') % ({'node': task.node.uuid, 'result': result}))
-
-    driver_internal_info = task.node.driver_internal_info
-    driver_internal_info['hardware_manager_version'] = result[
-        'hardware_manager_version']
-    task.node.driver_internal_info = driver_internal_info
-    task.node.save()
-
-    # Clean steps looks like {'HardwareManager': [{step1},{steps2}..]..}
-    # Flatten clean steps into one list
-    steps_list = [step for step_list in
-                  result['clean_steps'].values()
                  for step in step_list]
-    result = []
+
-    for step in steps_list:
+    if not steps or not override_priorities:
-        if interface and step.get('interface') != interface:
+        return steps
-            continue
+
+    for step in steps:
         new_priority = override_priorities.get(step.get('step'))
         if new_priority is not None:
             step['priority'] = new_priority
-        result.append(step)
 
-    return result
+    return steps
 
 
 def agent_execute_clean_step(task, step):

ironic/drivers/modules/ilo/deploy.py

@@ -213,6 +213,9 @@ class IloVirtualMediaAgentDeploy(agent.AgentDeploy):
         """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
         """
 

ironic/drivers/modules/iscsi_deploy.py

@@ -751,7 +751,9 @@ class ISCSIDeploy(base.DeployInterface):
         """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. If bash ramdisk is
             used for this node, it returns an empty list.
         """

ironic/tests/unit/drivers/modules/test_agent_base_vendor.py

@@ -357,11 +357,13 @@ class TestBaseAgentVendor(db_base.DbTestCase):
             'is done. Exception: LlamaException')
 
     @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True)
+    @mock.patch.object(agent_base_vendor.BaseAgentVendor,
+                       '_refresh_clean_steps', autospec=True)
     @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True)
     @mock.patch.object(agent_base_vendor.BaseAgentVendor,
                        'notify_conductor_resume_clean', autospec=True)
     def test_heartbeat_resume_clean(self, mock_notify, mock_set_steps,
-                                    mock_touch):
+                                    mock_refresh, mock_touch):
         kwargs = {
             'agent_url': 'http://127.0.0.1:9999/bar'
         }
@@ -374,13 +376,55 @@ class TestBaseAgentVendor(db_base.DbTestCase):
                 self.passthru.heartbeat(task, **kwargs)
 
             mock_touch.assert_called_once_with(mock.ANY)
+            mock_refresh.assert_called_once_with(mock.ANY, task)
             mock_notify.assert_called_once_with(mock.ANY, task)
             mock_set_steps.assert_called_once_with(task)
             # Reset mocks for the next interaction
             mock_touch.reset_mock()
+            mock_refresh.reset_mock()
             mock_notify.reset_mock()
             mock_set_steps.reset_mock()
 
+    @mock.patch.object(manager_utils, 'cleaning_error_handler')
+    @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True)
+    @mock.patch.object(agent_base_vendor.BaseAgentVendor,
+                       '_refresh_clean_steps', autospec=True)
+    @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True)
+    @mock.patch.object(agent_base_vendor.BaseAgentVendor,
+                       'notify_conductor_resume_clean', autospec=True)
+    def test_heartbeat_resume_clean_fails(self, mock_notify, mock_set_steps,
+                                          mock_refresh, mock_touch,
+                                          mock_handler):
+        mocks = [mock_refresh, mock_set_steps, mock_notify]
+        kwargs = {
+            'agent_url': 'http://127.0.0.1:9999/bar'
+        }
+        self.node.clean_step = {}
+        self.node.save()
+        for state in (states.CLEANWAIT, states.CLEANING):
+            self.node.provision_state = state
+            self.node.save()
+            for i in range(len(mocks)):
+                before_failed_mocks = mocks[:i]
+                failed_mock = mocks[i]
+                after_failed_mocks = mocks[i + 1:]
+                failed_mock.side_effect = Exception()
+                with task_manager.acquire(
+                        self.context, self.node.uuid, shared=True) as task:
+                    self.passthru.heartbeat(task, **kwargs)
+
+                mock_touch.assert_called_once_with(mock.ANY)
+                mock_handler.assert_called_once_with(task, mock.ANY)
+                for called in before_failed_mocks + [failed_mock]:
+                    self.assertTrue(called.called)
+                for not_called in after_failed_mocks:
+                    self.assertFalse(not_called.called)
+
+                # Reset mocks for the next interaction
+                for m in mocks + [mock_touch, mock_handler]:
+                    m.reset_mock()
+                failed_mock.side_effect = None
+
     @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True)
     @mock.patch.object(agent_base_vendor.BaseAgentVendor,
                        'continue_cleaning', autospec=True)
@@ -897,25 +941,13 @@ class TestBaseAgentVendor(db_base.DbTestCase):
     @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True)
     @mock.patch.object(agent_base_vendor.BaseAgentVendor,
                        'notify_conductor_resume_clean', autospec=True)
-    @mock.patch.object(agent_client.AgentClient, 'get_clean_steps',
+    @mock.patch.object(agent_base_vendor.BaseAgentVendor,
-                       autospec=True)
+                       '_refresh_clean_steps', autospec=True)
     @mock.patch.object(agent_client.AgentClient, 'get_commands_status',
                        autospec=True)
     def _test_continue_cleaning_clean_version_mismatch(
-            self, status_mock, get_steps_mock, notify_mock, steps_mock,
+            self, status_mock, refresh_steps_mock, notify_mock, steps_mock,
             manual=False):
-        get_steps_mock.return_value = {
-            'command_status': 'CLEAN_VERSION_MISMATCH',
-            'command_name': 'get_clean_step',
-            'command_result': {
-                'hardware_manager_version': {'Generic': '1'},
-                'clean_steps': {
-                    'GenericHardwareManager': [
-                        {'interface': 'deploy',
-                         'step': 'erase_devices',
-                         'priority': 20}]}
-            }
-        }
         status_mock.return_value = [{
             'command_status': 'CLEAN_VERSION_MISMATCH',
             'command_name': 'execute_clean_step',
@@ -928,17 +960,12 @@ class TestBaseAgentVendor(db_base.DbTestCase):
                                   shared=False) as task:
             self.passthru.continue_cleaning(task)
             notify_mock.assert_called_once_with(mock.ANY, task)
+            refresh_steps_mock.assert_called_once_with(mock.ANY, task)
             if manual:
-                get_steps_mock.assert_called_once_with(mock.ANY, task.node,
-                                                       task.ports)
                 self.assertFalse(
                     task.node.driver_internal_info['skip_current_clean_step'])
-                self.assertEqual(
-                    {'Generic': '1'},
-                    task.node.driver_internal_info['hardware_manager_version'])
                 self.assertFalse(steps_mock.called)
             else:
-                self.assertFalse(get_steps_mock.called)
                 steps_mock.assert_called_once_with(task)
                 self.assertFalse('skip_current_clean_step' in
                                  task.node.driver_internal_info)
@@ -950,25 +977,23 @@ class TestBaseAgentVendor(db_base.DbTestCase):
         self._test_continue_cleaning_clean_version_mismatch(manual=True)
 
     @mock.patch.object(manager_utils, 'cleaning_error_handler', autospec=True)
+    @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True)
     @mock.patch.object(agent_base_vendor.BaseAgentVendor,
                        'notify_conductor_resume_clean', autospec=True)
-    @mock.patch.object(agent_client.AgentClient, 'get_clean_steps',
+    @mock.patch.object(agent_base_vendor.BaseAgentVendor,
-                       autospec=True)
+                       '_refresh_clean_steps', autospec=True)
     @mock.patch.object(agent_client.AgentClient, 'get_commands_status',
                        autospec=True)
-    def test_continue_cleaning_manual_version_mismatch_bad(
+    def test_continue_cleaning_clean_version_mismatch_fail(
-            self, status_mock, get_steps_mock, notify_mock, error_mock):
+            self, status_mock, refresh_steps_mock, notify_mock, steps_mock,
-        get_steps_mock.return_value = {
+            error_mock, manual=False):
-            'command_status': 'CLEAN_VERSION_MISMATCH',
-            'command_name': 'get_clean_step',
-            'command_result': {
-                'hardware_manager_version': {'Generic': '1'}}
-        }
         status_mock.return_value = [{
             'command_status': 'CLEAN_VERSION_MISMATCH',
             'command_name': 'execute_clean_step',
+            'command_result': {'hardware_manager_version': {'Generic': '1'}}
         }]
-        tgt_prov_state = states.MANAGEABLE
+        refresh_steps_mock.side_effect = exception.NodeCleaningFailure("boo")
+        tgt_prov_state = states.MANAGEABLE if manual else states.AVAILABLE
         self.node.provision_state = states.CLEANWAIT
         self.node.target_provision_state = tgt_prov_state
         self.node.save()
@@ -976,12 +1001,11 @@ class TestBaseAgentVendor(db_base.DbTestCase):
                                   shared=False) as task:
             self.passthru.continue_cleaning(task)
 
-            get_steps_mock.assert_called_once_with(mock.ANY, task.node,
+            status_mock.assert_called_once_with(mock.ANY, task.node)
-                                                   task.ports)
+            refresh_steps_mock.assert_called_once_with(mock.ANY, task)
             error_mock.assert_called_once_with(task, mock.ANY)
             self.assertFalse(notify_mock.called)
-            self.assertFalse('skip_current_clean_step' in
+            self.assertFalse(steps_mock.called)
-                             task.node.driver_internal_info)
 
     @mock.patch.object(manager_utils, 'cleaning_error_handler', autospec=True)
     @mock.patch.object(agent_client.AgentClient, 'get_commands_status',
@@ -1076,3 +1100,91 @@ class TestBaseAgentVendor(db_base.DbTestCase):
         self.node.save()
         hook_returned = agent_base_vendor._get_post_clean_step_hook(self.node)
         self.assertIsNone(hook_returned)
+
+
+class TestRefreshCleanSteps(TestBaseAgentVendor):
+
+    def setUp(self):
+        super(TestRefreshCleanSteps, self).setUp()
+        self.node.driver_internal_info['agent_url'] = 'http://127.0.0.1:9999'
+        self.ports = [object_utils.create_test_port(self.context,
+                                                    node_id=self.node.id)]
+
+        self.clean_steps = {
+            'hardware_manager_version': '1',
+            'clean_steps': {
+                'GenericHardwareManager': [
+                    {'interface': 'deploy',
+                     'step': 'erase_devices',
+                     'priority': 20},
+                ],
+                'SpecificHardwareManager': [
+                    {'interface': 'deploy',
+                     'step': 'update_firmware',
+                     'priority': 30},
+                    {'interface': 'raid',
+                     'step': 'create_configuration',
+                     'priority': 10},
+                ]
+            }
+        }
+
+    @mock.patch.object(agent_client.AgentClient, 'get_clean_steps',
+                       autospec=True)
+    def test__refresh_clean_steps(self, client_mock):
+        client_mock.return_value = {
+            'command_result': self.clean_steps}
+
+        with task_manager.acquire(
+                self.context, self.node.uuid, shared=False) as task:
+            self.passthru._refresh_clean_steps(task)
+
+            client_mock.assert_called_once_with(mock.ANY, task.node,
+                                                task.ports)
+            self.assertEqual('1', task.node.driver_internal_info[
+                'hardware_manager_version'])
+            self.assertTrue('agent_cached_clean_steps_refreshed' in
+                            task.node.driver_internal_info)
+            steps = task.node.driver_internal_info['agent_cached_clean_steps']
+            # Since steps are returned in dicts, they have non-deterministic
+            # ordering
+            self.assertEqual(2, len(steps))
+            self.assertIn(self.clean_steps['clean_steps'][
+                'GenericHardwareManager'][0], steps['deploy'])
+            self.assertIn(self.clean_steps['clean_steps'][
+                'SpecificHardwareManager'][0], steps['deploy'])
+            self.assertEqual([self.clean_steps['clean_steps'][
+                'SpecificHardwareManager'][1]], steps['raid'])
+
+    @mock.patch.object(agent_client.AgentClient, 'get_clean_steps',
+                       autospec=True)
+    def test__refresh_clean_steps_missing_steps(self, client_mock):
+        del self.clean_steps['clean_steps']
+        client_mock.return_value = {
+            'command_result': self.clean_steps}
+
+        with task_manager.acquire(
+                self.context, self.node.uuid, shared=False) as task:
+            self.assertRaisesRegex(exception.NodeCleaningFailure,
+                                   'invalid result',
+                                   self.passthru._refresh_clean_steps,
+                                   task)
+            client_mock.assert_called_once_with(mock.ANY, task.node,
+                                                task.ports)
+
+    @mock.patch.object(agent_client.AgentClient, 'get_clean_steps',
+                       autospec=True)
+    def test__refresh_clean_steps_missing_interface(self, client_mock):
+        step = self.clean_steps['clean_steps']['SpecificHardwareManager'][1]
+        del step['interface']
+        client_mock.return_value = {
+            'command_result': self.clean_steps}
+
+        with task_manager.acquire(
+                self.context, self.node.uuid, shared=False) as task:
+            self.assertRaisesRegex(exception.NodeCleaningFailure,
+                                   'invalid clean step',
+                                   self.passthru._refresh_clean_steps,
+                                   task)
+            client_mock.assert_called_once_with(mock.ANY, task.node,
+                                                task.ports)

ironic/tests/unit/drivers/modules/test_deploy_utils.py

@@ -1521,142 +1521,75 @@ class AgentMethodsTestCase(db_base.DbTestCase):
     def setUp(self):
         super(AgentMethodsTestCase, self).setUp()
         mgr_utils.mock_the_extension_manager(driver='fake_agent')
-        n = {'driver': 'fake_agent',
-             'driver_internal_info': {'agent_url': 'http://127.0.0.1:9999'}}
 
+        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 = {'driver': 'fake_agent',
+             'driver_internal_info': {
+                 'agent_cached_clean_steps': self.clean_steps}}
         self.node = obj_utils.create_test_node(self.context, **n)
         self.ports = [obj_utils.create_test_port(self.context,
                                                  node_id=self.node.id)]
 
-        self.clean_steps = {
+    def test_agent_get_clean_steps(self):
-            'hardware_manager_version': '1',
-            'clean_steps': {
-                'GenericHardwareManager': [
-                    {'interface': 'deploy',
-                     'step': 'erase_devices',
-                     'priority': 20},
-                ],
-                'SpecificHardwareManager': [
-                    {'interface': 'deploy',
-                     'step': 'update_firmware',
-                     'priority': 30},
-                    {'interface': 'raid',
-                     'step': 'create_configuration',
-                     'priority': 10},
-                ]
-            }
-        }
-
-    @mock.patch('ironic.objects.Port.list_by_node_id',
-                spec_set=types.FunctionType)
-    @mock.patch.object(agent_client.AgentClient, 'get_clean_steps',
-                       autospec=True)
-    def test_get_clean_steps(self, client_mock, list_ports_mock):
-        client_mock.return_value = {
-            'command_result': self.clean_steps}
-        list_ports_mock.return_value = self.ports
-
         with task_manager.acquire(
-                self.context, self.node['uuid'], shared=False) as task:
+                self.context, self.node.uuid, shared=False) as task:
             response = utils.agent_get_clean_steps(task)
-            client_mock.assert_called_once_with(mock.ANY, task.node,
-                                                self.ports)
-            self.assertEqual('1', task.node.driver_internal_info[
-                'hardware_manager_version'])
 
             # Since steps are returned in dicts, they have non-deterministic
             # ordering
             self.assertThat(response, matchers.HasLength(3))
-            self.assertIn(self.clean_steps['clean_steps'][
+            self.assertIn(self.clean_steps['deploy'][0], response)
-                'GenericHardwareManager'][0], response)
+            self.assertIn(self.clean_steps['deploy'][1], response)
-            self.assertIn(self.clean_steps['clean_steps'][
+            self.assertIn(self.clean_steps['raid'][0], response)
-                'SpecificHardwareManager'][0], response)
-            self.assertIn(self.clean_steps['clean_steps'][
-                'SpecificHardwareManager'][1], response)
-
-    @mock.patch('ironic.objects.Port.list_by_node_id',
-                spec_set=types.FunctionType)
-    @mock.patch.object(agent_client.AgentClient, 'get_clean_steps',
-                       autospec=True)
-    def test_get_clean_steps_custom_interface(
-            self, client_mock, list_ports_mock):
-        client_mock.return_value = {
-            'command_result': self.clean_steps}
-        list_ports_mock.return_value = self.ports
 
+    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')
-            client_mock.assert_called_once_with(mock.ANY, task.node,
-                                                self.ports)
-            self.assertEqual('1', task.node.driver_internal_info[
-                'hardware_manager_version'])
-
             self.assertThat(response, matchers.HasLength(1))
-            self.assertIn(self.clean_steps['clean_steps'][
+            self.assertEqual(self.clean_steps['raid'], response)
-                'SpecificHardwareManager'][1], response)
-
-    @mock.patch('ironic.objects.Port.list_by_node_id',
-                spec_set=types.FunctionType)
-    @mock.patch.object(agent_client.AgentClient, 'get_clean_steps',
-                       autospec=True)
-    def test_get_clean_steps_override_priorities(
-            self, client_mock, list_ports_mock):
-        client_mock.return_value = {
-            'command_result': self.clean_steps}
-        list_ports_mock.return_value = self.ports
 
+    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)
-            client_mock.assert_called_once_with(mock.ANY, task.node,
-                                                self.ports)
-            self.assertEqual('1', task.node.driver_internal_info[
-                'hardware_manager_version'])
             self.assertEqual(42, response[0]['priority'])
 
-    @mock.patch('ironic.objects.Port.list_by_node_id',
+    def test_get_clean_steps_override_priorities_none(self):
-                spec_set=types.FunctionType)
-    @mock.patch.object(agent_client.AgentClient, 'get_clean_steps',
-                       autospec=True)
-    def test_get_clean_steps_override_priorities_none(
-            self, client_mock, list_ports_mock):
-        client_mock.return_value = {
-            'command_result': self.clean_steps}
-        list_ports_mock.return_value = self.ports
-
         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)
-            client_mock.assert_called_once_with(mock.ANY, task.node,
-                                                self.ports)
-            self.assertEqual('1', task.node.driver_internal_info[
-                'hardware_manager_version'])
             self.assertEqual(10, response[0]['priority'])
 
-    @mock.patch('ironic.objects.Port.list_by_node_id',
+    def test_get_clean_steps_missing_steps(self):
-                spec_set=types.FunctionType)
+        info = self.node.driver_internal_info
-    @mock.patch.object(agent_client.AgentClient, 'get_clean_steps',
+        del info['agent_cached_clean_steps']
-                       autospec=True)
+        self.node.driver_internal_info = info
-    def test_get_clean_steps_missing_steps(self, client_mock,
+        self.node.save()
-                                           list_ports_mock):
-        del self.clean_steps['clean_steps']
-        client_mock.return_value = {
-            'command_result': self.clean_steps}
-        list_ports_mock.return_value = self.ports
-
         with task_manager.acquire(
                 self.context, self.node.uuid, shared=False) as task:
             self.assertRaises(exception.NodeCleaningFailure,
                               utils.agent_get_clean_steps,
                               task)
-            client_mock.assert_called_once_with(mock.ANY, task.node,
-                                                self.ports)
 
     @mock.patch('ironic.objects.Port.list_by_node_id',
                 spec_set=types.FunctionType)
@@ -1668,10 +1601,10 @@ class AgentMethodsTestCase(db_base.DbTestCase):
         list_ports_mock.return_value = self.ports
 
         with task_manager.acquire(
-                self.context, self.node['uuid'], shared=False) as task:
+                self.context, self.node.uuid, shared=False) as task:
             response = utils.agent_execute_clean_step(
                 task,
-                self.clean_steps['clean_steps']['GenericHardwareManager'][0])
+                self.clean_steps['deploy'][0])
             self.assertEqual(states.CLEANWAIT, response)
 
     @mock.patch('ironic.objects.Port.list_by_node_id',
@@ -1684,10 +1617,10 @@ class AgentMethodsTestCase(db_base.DbTestCase):
         list_ports_mock.return_value = self.ports
 
         with task_manager.acquire(
-                self.context, self.node['uuid'], shared=False) as task:
+                self.context, self.node.uuid, shared=False) as task:
             response = utils.agent_execute_clean_step(
                 task,
-                self.clean_steps['clean_steps']['GenericHardwareManager'][0])
+                self.clean_steps['deploy'][0])
             self.assertEqual(states.CLEANWAIT, response)
 
     @mock.patch('ironic.objects.Port.list_by_node_id',
@@ -1701,16 +1634,16 @@ class AgentMethodsTestCase(db_base.DbTestCase):
         list_ports_mock.return_value = self.ports
 
         with task_manager.acquire(
-                self.context, self.node['uuid'], shared=False) as task:
+                self.context, self.node.uuid, shared=False) as task:
             response = utils.agent_execute_clean_step(
                 task,
-                self.clean_steps['clean_steps']['GenericHardwareManager'][0])
+                self.clean_steps['deploy'][0])
             self.assertEqual(states.CLEANWAIT, response)
 
     def test_agent_add_clean_params(self):
         cfg.CONF.deploy.erase_devices_iterations = 2
         with task_manager.acquire(
-                self.context, self.node['uuid'], shared=False) as task:
+                self.context, self.node.uuid, shared=False) as task:
             utils.agent_add_clean_params(task)
             self.assertEqual(task.node.driver_internal_info.get(
                 'agent_erase_devices_iterations'), 2)