diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index dae53779b7..827244ed1d 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -762,16 +762,17 @@ class ConductorManager(base_manager.BaseConductorManager): raise exception.NodeInMaintenance(op=_('cleaning'), node=node.uuid) - # NOTE(rloo): _do_node_clean() will also make a similar call - # to validate the power, but we are doing it again here so that + # NOTE(rloo): _do_node_clean() will also make similar calls to + # validate power & network, but we are doing it again here so that # the user gets immediate feedback of any issues. This behaviour # (of validating) is consistent with other methods like # self.do_node_deploy(). try: task.driver.power.validate(task) + task.driver.network.validate(task) except exception.InvalidParameterValue as e: - msg = (_('Failed to validate power info. ' - 'Cannot clean node %(node)s. Error: %(msg)s') % + msg = (_('Validation failed. Cannot clean node %(node)s. ' + 'Error: %(msg)s') % {'node': node.uuid, 'msg': e}) raise exception.InvalidParameterValue(msg) @@ -898,12 +899,13 @@ class ConductorManager(base_manager.BaseConductorManager): return try: - # NOTE(ghe): Valid power driver values are needed to perform + # NOTE(ghe): Valid power and network values are needed to perform # a cleaning. task.driver.power.validate(task) + task.driver.network.validate(task) except exception.InvalidParameterValue as e: - msg = (_('Failed to validate power driver interface. ' - 'Can not clean node %(node)s. Error: %(msg)s') % + msg = (_('Validation failed. Cannot clean node %(node)s. ' + 'Error: %(msg)s') % {'node': node.uuid, 'msg': e}) return utils.cleaning_error_handler(task, msg) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index fcdc807a92..6714f1d023 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -1840,9 +1840,7 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, self.assertFalse(mock_validate.called) @mock.patch('ironic.conductor.task_manager.TaskManager.process_event') - @mock.patch('ironic.drivers.modules.fake.FakePower.validate') - def test_do_node_clean_validate_fail(self, mock_validate, mock_process): - # power validate fails + def _test_do_node_clean_validate_fail(self, mock_validate, mock_process): mock_validate.side_effect = exception.InvalidParameterValue('error') node = obj_utils.create_test_node( self.context, driver='fake', provision_state=states.MANAGEABLE, @@ -1857,7 +1855,17 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, self.assertFalse(mock_process.called) @mock.patch('ironic.drivers.modules.fake.FakePower.validate') - def test_do_node_clean_invalid_state(self, mock_validate): + def test_do_node_clean_power_validate_fail(self, mock_validate): + self._test_do_node_clean_validate_fail(mock_validate) + + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate') + def test_do_node_clean_network_validate_fail(self, mock_validate): + self._test_do_node_clean_validate_fail(mock_validate) + + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate') + @mock.patch('ironic.drivers.modules.fake.FakePower.validate') + def test_do_node_clean_invalid_state(self, mock_power_valid, + mock_network_valid): # test node.provision_state is incorrect for clean node = obj_utils.create_test_node( self.context, driver='fake', provision_state=states.ENROLL, @@ -1868,20 +1876,24 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, self.context, node.uuid, []) # Compare true exception hidden by @messaging.expected_exceptions self.assertEqual(exception.InvalidStateRequested, exc.exc_info[0]) - mock_validate.assert_called_once_with(mock.ANY) + mock_power_valid.assert_called_once_with(mock.ANY) + mock_network_valid.assert_called_once_with(mock.ANY) node.refresh() self.assertNotIn('clean_steps', node.driver_internal_info) @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker') + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate') @mock.patch('ironic.drivers.modules.fake.FakePower.validate') - def test_do_node_clean_ok(self, mock_validate, mock_spawn): + def test_do_node_clean_ok(self, mock_power_valid, mock_network_valid, + mock_spawn): node = obj_utils.create_test_node( self.context, driver='fake', provision_state=states.MANAGEABLE, target_provision_state=states.NOSTATE, last_error='old error') self._start_service() clean_steps = [self.deploy_raid] self.service.do_node_clean(self.context, node.uuid, clean_steps) - mock_validate.assert_called_once_with(mock.ANY) + mock_power_valid.assert_called_once_with(mock.ANY) + mock_network_valid.assert_called_once_with(mock.ANY) mock_spawn.assert_called_with(self.service._do_node_clean, mock.ANY, clean_steps) node.refresh() @@ -1892,8 +1904,10 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, self.assertIsNone(node.last_error) @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker') + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate') @mock.patch('ironic.drivers.modules.fake.FakePower.validate') - def test_do_node_clean_worker_pool_full(self, mock_validate, mock_spawn): + def test_do_node_clean_worker_pool_full(self, mock_power_valid, + mock_network_valid, mock_spawn): prv_state = states.MANAGEABLE tgt_prv_state = states.NOSTATE node = obj_utils.create_test_node( @@ -1908,7 +1922,8 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, # Compare true exception hidden by @messaging.expected_exceptions self.assertEqual(exception.NoFreeConductorWorker, exc.exc_info[0]) self._stop_service() - mock_validate.assert_called_once_with(mock.ANY) + mock_power_valid.assert_called_once_with(mock.ANY) + mock_network_valid.assert_called_once_with(mock.ANY) mock_spawn.assert_called_with(self.service._do_node_clean, mock.ANY, clean_steps) node.refresh() @@ -2072,9 +2087,8 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, def test_continue_node_clean_manual_abort_last_clean_step(self): self._continue_node_clean_abort_last_clean_step(manual=True) - @mock.patch('ironic.drivers.modules.fake.FakePower.validate') def __do_node_clean_validate_fail(self, mock_validate, clean_steps=None): - # InvalidParameterValue should be cause node to go to CLEANFAIL + # InvalidParameterValue should cause node to go to CLEANFAIL mock_validate.side_effect = exception.InvalidParameterValue('error') tgt_prov_state = states.MANAGEABLE if clean_steps else states.AVAILABLE node = obj_utils.create_test_node( @@ -2089,11 +2103,22 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(tgt_prov_state, node.target_provision_state) mock_validate.assert_called_once_with(mock.ANY) - def test__do_node_clean_automated_validate_fail(self): - self.__do_node_clean_validate_fail() + @mock.patch('ironic.drivers.modules.fake.FakePower.validate') + def test__do_node_clean_automated_power_validate_fail(self, mock_validate): + self.__do_node_clean_validate_fail(mock_validate) - def test__do_node_clean_manual_validate_fail(self): - self.__do_node_clean_validate_fail(clean_steps=[]) + @mock.patch('ironic.drivers.modules.fake.FakePower.validate') + def test__do_node_clean_manual_power_validate_fail(self, mock_validate): + self.__do_node_clean_validate_fail(mock_validate, clean_steps=[]) + + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate') + def test__do_node_clean_automated_network_validate_fail(self, + mock_validate): + self.__do_node_clean_validate_fail(mock_validate) + + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate') + def test__do_node_clean_manual_network_validate_fail(self, mock_validate): + self.__do_node_clean_validate_fail(mock_validate, clean_steps=[]) @mock.patch('ironic.drivers.modules.fake.FakePower.validate') def test__do_node_clean_automated_disabled(self, mock_validate): @@ -2119,8 +2144,10 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, self.assertNotIn('clean_steps', node.driver_internal_info) self.assertNotIn('clean_step_index', node.driver_internal_info) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate') @mock.patch('ironic.drivers.modules.fake.FakeDeploy.prepare_cleaning') - def __do_node_clean_prepare_clean_fail(self, mock_prep, clean_steps=None): + def __do_node_clean_prepare_clean_fail(self, mock_prep, mock_validate, + clean_steps=None): # Exception from task.driver.deploy.prepare_cleaning should cause node # to go to CLEANFAIL mock_prep.side_effect = exception.InvalidParameterValue('error') @@ -2136,6 +2163,7 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(states.CLEANFAIL, node.provision_state) self.assertEqual(tgt_prov_state, node.target_provision_state) mock_prep.assert_called_once_with(mock.ANY) + mock_validate.assert_called_once_with(task) def test__do_node_clean_automated_prepare_clean_fail(self): self.__do_node_clean_prepare_clean_fail() @@ -2143,8 +2171,10 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, def test__do_node_clean_manual_prepare_clean_fail(self): self.__do_node_clean_prepare_clean_fail(clean_steps=[self.deploy_raid]) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate') @mock.patch('ironic.drivers.modules.fake.FakeDeploy.prepare_cleaning') - def __do_node_clean_prepare_clean_wait(self, mock_prep, clean_steps=None): + def __do_node_clean_prepare_clean_wait(self, mock_prep, mock_validate, + clean_steps=None): mock_prep.return_value = states.CLEANWAIT tgt_prov_state = states.MANAGEABLE if clean_steps else states.AVAILABLE node = obj_utils.create_test_node( @@ -2158,6 +2188,7 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(states.CLEANWAIT, node.provision_state) self.assertEqual(tgt_prov_state, node.target_provision_state) mock_prep.assert_called_once_with(mock.ANY) + mock_validate.assert_called_once_with(mock.ANY) def test__do_node_clean_automated_prepare_clean_wait(self): self.__do_node_clean_prepare_clean_wait() @@ -2165,9 +2196,10 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, def test__do_node_clean_manual_prepare_clean_wait(self): self.__do_node_clean_prepare_clean_wait(clean_steps=[self.deploy_raid]) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) @mock.patch.object(conductor_utils, 'set_node_cleaning_steps') - def __do_node_clean_steps_fail(self, mock_steps, clean_steps=None, - invalid_exc=True): + def __do_node_clean_steps_fail(self, mock_steps, mock_validate, + clean_steps=None, invalid_exc=True): if invalid_exc: mock_steps.side_effect = exception.InvalidParameterValue('invalid') else: @@ -2181,6 +2213,7 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, with task_manager.acquire( self.context, node.uuid, shared=False) as task: self.service._do_node_clean(task, clean_steps=clean_steps) + mock_validate.assert_called_once_with(mock.ANY, task) node.refresh() self.assertEqual(states.CLEANFAIL, node.provision_state) self.assertEqual(tgt_prov_state, node.target_provision_state) @@ -2198,9 +2231,10 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, @mock.patch.object(conductor_utils, 'set_node_cleaning_steps') @mock.patch('ironic.conductor.manager.ConductorManager.' '_do_next_clean_step') + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate') @mock.patch('ironic.drivers.modules.fake.FakePower.validate') - def __do_node_clean(self, mock_validate, mock_next_step, mock_steps, - clean_steps=None): + def __do_node_clean(self, mock_power_valid, mock_network_valid, + mock_next_step, mock_steps, clean_steps=None): if clean_steps: tgt_prov_state = states.MANAGEABLE driver_info = {} @@ -2223,7 +2257,8 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, self._stop_service() node.refresh() - mock_validate.assert_called_once_with(task) + mock_power_valid.assert_called_once_with(task) + mock_network_valid.assert_called_once_with(task) mock_next_step.assert_called_once_with(mock.ANY, 0) mock_steps.assert_called_once_with(task) if clean_steps: