From ccf9d91f0e89da808445636b532b26ae9a381313 Mon Sep 17 00:00:00 2001 From: saripurigopi Date: Fri, 12 Jun 2015 09:56:59 +0530 Subject: [PATCH] Address follow-up comments on ucs drivers This commit addresses follow up comments on I02211da5fad039dc7e6b509d547e473e9b57009b. The change is to add missing unit-test case for set and reboot power operations. Change-Id: Ic77c7c318888ffbda268610b71363a74e7f98d85 --- ironic/drivers/modules/ucs/helper.py | 4 +- ironic/drivers/modules/ucs/power.py | 13 +++--- ironic/tests/drivers/ucs/test_power.py | 55 +++++++++++++++++++++++--- 3 files changed, 60 insertions(+), 12 deletions(-) diff --git a/ironic/drivers/modules/ucs/helper.py b/ironic/drivers/modules/ucs/helper.py index 5abafa5663..ffd3639458 100644 --- a/ironic/drivers/modules/ucs/helper.py +++ b/ironic/drivers/modules/ucs/helper.py @@ -78,7 +78,9 @@ def parse_driver_info(node): info = {} for param in REQUIRED_PROPERTIES: info[param] = node.driver_info.get(param) - error_msg = _("cisco driver requries these parameter to be set.") + error_msg = (_("%s driver requires these parameters to be set in the " + "node's driver_info.") % + node.driver) deploy_utils.check_for_missing_params(info, error_msg) return info diff --git a/ironic/drivers/modules/ucs/power.py b/ironic/drivers/modules/ucs/power.py index 85652101ac..779389fc58 100644 --- a/ironic/drivers/modules/ucs/power.py +++ b/ironic/drivers/modules/ucs/power.py @@ -122,9 +122,10 @@ class Power(base.PowerInterface): power_handle = ucs_power.UcsPower(helper) power_status = power_handle.get_power_state() except ucs_error.UcsOperationError as ucs_exception: - LOG.error(_LE("%(driver)s: get_power_status operation failed for " - "node %(uuid)s with error."), - {'driver': task.node.driver, 'uuid': task.node.uuid}) + LOG.error(_LE("%(driver)s: get_power_state operation failed for " + "node %(uuid)s with error: %(msg)s."), + {'driver': task.node.driver, 'uuid': task.node.uuid, + 'msg': ucs_exception}) operation = _('getting power status') raise exception.UcsOperationError(operation=operation, error=ucs_exception, @@ -163,8 +164,10 @@ class Power(base.PowerInterface): else: return except ucs_error.UcsOperationError as ucs_exception: - LOG.error(_LE("Cisco client exception %(msg)s for node %(uuid)s"), - {'msg': ucs_exception, 'uuid': task.node.uuid}) + LOG.error(_LE("%(driver)s: set_power_state operation failed for " + "node %(uuid)s with error: %(msg)s."), + {'driver': task.node.driver, 'uuid': task.node.uuid, + 'msg': ucs_exception}) operation = _("setting power status") raise exception.UcsOperationError(operation=operation, error=ucs_exception, diff --git a/ironic/tests/drivers/ucs/test_power.py b/ironic/tests/drivers/ucs/test_power.py index 8f5a2f070e..2f2cca036c 100644 --- a/ironic/tests/drivers/ucs/test_power.py +++ b/ironic/tests/drivers/ucs/test_power.py @@ -189,7 +189,7 @@ class UcsPowerTestCase(db_base.DbTestCase): spec_set=True, autospec=True) @mock.patch('ironic.drivers.modules.ucs.power.ucs_power.UcsPower', spec_set=True, autospec=True) - def test__set_and_wait_for_state_change_already_target_state( + def test__wait_for_state_change_already_target_state( self, mock_ucs_power, mock_helper): @@ -206,7 +206,7 @@ class UcsPowerTestCase(db_base.DbTestCase): spec_set=True, autospec=True) @mock.patch('ironic.drivers.modules.ucs.power.ucs_power.UcsPower', spec_set=True, autospec=True) - def test__set_and_wait_for_state_change_exceed_iterations( + def test__wait_for_state_change_exceed_iterations( self, mock_power_helper, mock_helper): @@ -222,6 +222,33 @@ class UcsPowerTestCase(db_base.DbTestCase): mock_power.get_power_state.assert_called_with() self.assertEqual(4, mock_power.get_power_state.call_count) + @mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper', + spec_set=True, autospec=True) + @mock.patch('ironic.drivers.modules.ucs.power._wait_for_state_change', + spec_set=True, autospec=True) + @mock.patch('ironic.drivers.modules.ucs.power.ucs_power.UcsPower', + spec_set=True, autospec=True) + def test_set_and_wait_for_state_change_fail( + self, + mock_power_helper, + mock__wait, + mock_helper): + target_state = states.POWER_ON + mock_power = mock_power_helper.return_value + mock_power.get_power_state.return_value = 'down' + mock_helper.generate_ucsm_handle.return_value = (True, mock.Mock()) + mock__wait.return_value = states.POWER_OFF + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertRaises(exception.PowerStateFailure, + self.interface.set_power_state, + task, + target_state) + + mock_power.set_power_state.assert_called_once_with('up') + mock_power.get_power_state.assert_called_once_with() + mock__wait.assert_called_once_with(target_state, mock_power) + @mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper', spec_set=True, autospec=True) @mock.patch('ironic.drivers.modules.ucs.power._wait_for_state_change', @@ -239,17 +266,14 @@ class UcsPowerTestCase(db_base.DbTestCase): @mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper', spec_set=True, autospec=True) - @mock.patch('ironic.drivers.modules.ucs.power._wait_for_state_change', - spec_set=True, autospec=True) @mock.patch('ironic.drivers.modules.ucs.power.ucs_power.UcsPower', spec_set=True, autospec=True) - def test_reboot_fail(self, mock_power_helper, mock__wait, + def test_reboot_fail(self, mock_power_helper, mock_ucs_helper): mock_ucs_helper.generate_ucsm_handle.return_value = (True, mock.Mock()) mock_power = mock_power_helper.return_value mock_power.reboot.side_effect = ( ucs_error.UcsOperationError(operation='rebooting', error='failed')) - mock__wait.return_value = states.ERROR with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: self.assertRaises(exception.UcsOperationError, @@ -257,3 +281,22 @@ class UcsPowerTestCase(db_base.DbTestCase): task ) mock_power.reboot.assert_called_once_with() + + @mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper', + spec_set=True, autospec=True) + @mock.patch('ironic.drivers.modules.ucs.power._wait_for_state_change', + spec_set=True, autospec=True) + @mock.patch('ironic.drivers.modules.ucs.power.ucs_power.UcsPower', + spec_set=True, autospec=True) + def test_reboot__wait_state_change_fail(self, mock_power_helper, + mock__wait, + mock_ucs_helper): + mock_ucs_helper.generate_ucsm_handle.return_value = (True, mock.Mock()) + mock_power = mock_power_helper.return_value + mock__wait.return_value = states.ERROR + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertRaises(exception.PowerStateFailure, + self.interface.reboot, + task) + mock_power.reboot.assert_called_once_with()