From 04209eb27e064ad9193bad872c1bae0f6c7007b0 Mon Sep 17 00:00:00 2001 From: Jim Rollenhagen Date: Mon, 16 May 2016 09:19:19 -0400 Subject: [PATCH] Remove backward compat for Liberty cleaning In Liberty, we fetched the last clean step by comparing the current clean step to the full list stored in the node. In Mitaka, we cleaned this up with node.driver_internal_info['clean_step_index']. However, we left a shim in for nodes that were cleaning during the Liberty to Mitaka upgrade, and did not have the clean step index stored. Now that we're in Newton, we can safely remove that shim. Change-Id: I444cd5188a1dfb56fbad13f025179dfdc4cb5c85 --- ironic/conductor/manager.py | 21 +-------- ironic/tests/unit/conductor/test_manager.py | 48 +++------------------ 2 files changed, 7 insertions(+), 62 deletions(-) diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index c665ee200e..ee4f85f691 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -689,26 +689,7 @@ class ConductorManager(base_manager.BaseConductorManager): # index of the first step in the list. return 0 - ind = None - if 'clean_step_index' in node.driver_internal_info: - ind = node.driver_internal_info['clean_step_index'] - else: - # TODO(rloo). driver_internal_info['clean_step_index'] was - # added in Mitaka. We need to maintain backwards compatibility - # so this uses the original code to get the index of the current - # step. This will be deleted in the Newton cycle. - try: - next_steps = node.driver_internal_info['clean_steps'] - ind = next_steps.index(node.clean_step) - except (KeyError, ValueError): - msg = (_('Node %(node)s got an invalid last step for ' - '%(state)s: %(step)s.') % - {'node': node.uuid, 'step': node.clean_step, - 'state': node.provision_state}) - LOG.exception(msg) - utils.cleaning_error_handler(task, msg) - raise exception.NodeCleaningFailure(node=node.uuid, - reason=msg) + ind = node.driver_internal_info.get('clean_step_index') if ind is None: return None diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 6e01c23ad6..48fbb2c1b3 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -1557,7 +1557,8 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, # test a node can continue cleaning via RPC prv_state = return_state tgt_prv_state = states.MANAGEABLE if manual else states.AVAILABLE - driver_info = {'clean_steps': self.clean_steps} + driver_info = {'clean_steps': self.clean_steps, + 'clean_step_index': 0} node = obj_utils.create_test_node(self.context, driver='fake', provision_state=prv_state, target_provision_state=tgt_prv_state, @@ -1582,7 +1583,8 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker') def _continue_node_clean_skip_step(self, mock_spawn, skip=True): # test that skipping current step mechanism works - driver_info = {'clean_steps': self.clean_steps} + driver_info = {'clean_steps': self.clean_steps, + 'clean_step_index': 0} if not skip: driver_info['skip_current_clean_step'] = skip node = obj_utils.create_test_node( @@ -1612,7 +1614,8 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, last_clean_step = self.clean_steps[0] last_clean_step['abortable'] = False last_clean_step['abort_after'] = True - driver_info = {'clean_steps': self.clean_steps} + driver_info = {'clean_steps': self.clean_steps, + 'clean_step_index': 0} tgt_prov_state = states.MANAGEABLE if manual else states.AVAILABLE node = obj_utils.create_test_node( self.context, driver='fake', provision_state=states.CLEANWAIT, @@ -2213,45 +2216,6 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, step_index = self.service._get_node_next_clean_steps(task) self.assertEqual(0, step_index) - def __get_node_next_clean_steps_backwards_compat(self, skip=True): - driver_internal_info = {'clean_steps': self.clean_steps} - node = obj_utils.create_test_node( - self.context, driver='fake', - provision_state=states.CLEANWAIT, - target_provision_state=states.AVAILABLE, - driver_internal_info=driver_internal_info, - last_error=None, - clean_step=self.clean_steps[0]) - - with task_manager.acquire(self.context, node.uuid) as task: - step_index = self.service._get_node_next_clean_steps( - task, skip_current_step=skip) - expected_index = 1 if skip else 0 - self.assertEqual(expected_index, step_index) - - def test__get_node_next_clean_steps_backwards_compat(self): - self.__get_node_next_clean_steps_backwards_compat() - - def test__get_node_next_clean_steps_no_skip_backwards_compat(self): - self.__get_node_next_clean_steps_backwards_compat(skip=False) - - def test__get_node_next_clean_steps_bad_clean_step(self): - # NOTE(rloo) for backwards compatibility - driver_internal_info = {'clean_steps': self.clean_steps} - node = obj_utils.create_test_node( - self.context, driver='fake', - provision_state=states.CLEANWAIT, - target_provision_state=states.AVAILABLE, - driver_internal_info=driver_internal_info, - last_error=None, - clean_step={'interface': 'deploy', - 'step': 'not_a_clean_step', - 'priority': 100}) - - with task_manager.acquire(self.context, node.uuid) as task: - self.assertRaises(exception.NodeCleaningFailure, - self.service._get_node_next_clean_steps, task) - @mgr_utils.mock_record_keepalive class DoNodeVerifyTestCase(mgr_utils.ServiceSetUpMixin,