openstack
/
ironic
Code Issues Proposed changes

Merge "Validate the network interface before cleaning"

This commit is contained in:
Jenkins 2017-02-07 21:17:36 +00:00 committed by Gerrit Code Review
parent 9da91aaaf5 f192abb9ed
commit 72516959f3
2 changed files with 66 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
ironic/conductor/manager.py
View File

@ -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)

79
ironic/tests/unit/conductor/test_manager.py
View File

@ -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: