diff --git a/cinder/tests/unit/zonemanager/test_cisco_fc_zone_client_cli.py b/cinder/tests/unit/zonemanager/test_cisco_fc_zone_client_cli.py index 137d5dda944..57ca8e40b6b 100644 --- a/cinder/tests/unit/zonemanager/test_cisco_fc_zone_client_cli.py +++ b/cinder/tests/unit/zonemanager/test_cisco_fc_zone_client_cli.py @@ -192,7 +192,38 @@ class TestCiscoFCZoneClientCLI(cli.CiscoFCZoneClientCLI, test.TestCase): def test__cfg_save(self, run_ssh_mock): cmd_list = ['copy', 'running-config', 'startup-config'] self._cfg_save() - run_ssh_mock.assert_called_once_with(cmd_list, True, 1) + run_ssh_mock.assert_called_once_with(cmd_list, True) + + @mock.patch.object(cli.CiscoFCZoneClientCLI, '_run_ssh') + def test__cfg_save_with_retry(self, run_ssh_mock): + cmd_list = ['copy', 'running-config', 'startup-config'] + run_ssh_mock.side_effect = [ + processutils.ProcessExecutionError, + ('', None) + ] + + self._cfg_save() + + self.assertEqual(2, run_ssh_mock.call_count) + run_ssh_mock.assert_has_calls([ + mock.call(cmd_list, True), + mock.call(cmd_list, True) + ]) + + @mock.patch.object(cli.CiscoFCZoneClientCLI, '_run_ssh') + def test__cfg_save_with_error(self, run_ssh_mock): + cmd_list = ['copy', 'running-config', 'startup-config'] + run_ssh_mock.side_effect = processutils.ProcessExecutionError + + self.assertRaises(processutils.ProcessExecutionError, self._cfg_save) + + expected_num_calls = 5 + expected_calls = [] + for i in xrange(expected_num_calls): + expected_calls.append(mock.call(cmd_list, True)) + + self.assertEqual(expected_num_calls, run_ssh_mock.call_count) + run_ssh_mock.assert_has_calls(expected_calls) @mock.patch.object(cli.CiscoFCZoneClientCLI, '_run_ssh') def test__get_switch_info(self, run_ssh_mock): @@ -201,7 +232,7 @@ class TestCiscoFCZoneClientCLI(cli.CiscoFCZoneClientCLI, test.TestCase): run_ssh_mock.return_value = (Stream(nsshow), Stream()) switch_data = self._get_switch_info(cmd_list) self.assertEqual(nsshow_list, switch_data) - run_ssh_mock.assert_called_once_with(cmd_list, True, 1) + run_ssh_mock.assert_called_once_with(cmd_list, True) def test__parse_ns_output(self): return_wwn_list = [] @@ -210,6 +241,31 @@ class TestCiscoFCZoneClientCLI(cli.CiscoFCZoneClientCLI, test.TestCase): self.assertEqual(expected_wwn_list, return_wwn_list) +class TestCiscoFCZoneClientCLISSH(test.TestCase): + + def setUp(self): + super(TestCiscoFCZoneClientCLISSH, self).setUp() + self.client = cli.CiscoFCZoneClientCLI(None, None, None, None, None) + self.client.sshpool = mock.MagicMock() + self.mock_ssh = self.client.sshpool.item().__enter__() + + @mock.patch('oslo_concurrency.processutils.ssh_execute') + def test__run_ssh(self, mock_execute): + mock_execute.return_value = 'ssh output' + ret = self.client._run_ssh(['cat', 'foo']) + self.assertEqual('ssh output', ret) + mock_execute.assert_called_once_with(self.mock_ssh, + 'cat foo', + check_exit_code=True) + + @mock.patch('oslo_concurrency.processutils.ssh_execute') + def test__run_ssh_with_error(self, mock_execute): + mock_execute.side_effect = processutils.ProcessExecutionError() + self.assertRaises(processutils.ProcessExecutionError, + self.client._run_ssh, + ['cat', 'foo']) + + class Channel(object): def recv_exit_status(self): return 0 diff --git a/cinder/zonemanager/drivers/cisco/cisco_fc_zone_client_cli.py b/cinder/zonemanager/drivers/cisco/cisco_fc_zone_client_cli.py index 705ba1805f3..4cdbda1ed9c 100644 --- a/cinder/zonemanager/drivers/cisco/cisco_fc_zone_client_cli.py +++ b/cinder/zonemanager/drivers/cisco/cisco_fc_zone_client_cli.py @@ -28,7 +28,7 @@ from oslo_utils import excutils import six from cinder import exception -from cinder.i18n import _, _LE, _LI +from cinder.i18n import _, _LE, _LI, _LW from cinder import ssh_utils from cinder import utils import cinder.zonemanager.drivers.cisco.fc_zone_constants as ZoneConstant @@ -313,14 +313,15 @@ class CiscoFCZoneClientCLI(object): return return_list + @utils.retry(processutils.ProcessExecutionError, retries=5) def _cfg_save(self): cmd = ['copy', 'running-config', 'startup-config'] - self._run_ssh(cmd, True, 1) + self._run_ssh(cmd, True) def _get_switch_info(self, cmd_list): stdout, stderr, sw_data = None, None, None try: - stdout, stderr = self._run_ssh(cmd_list, True, 1) + stdout, stderr = self._run_ssh(cmd_list, True) LOG.debug("CLI output from ssh - output: %s", stdout) if (stdout): sw_data = stdout.splitlines() @@ -353,7 +354,7 @@ class CiscoFCZoneClientCLI(object): raise exception.InvalidParameterValue(err=msg) return return_list - def _run_ssh(self, cmd_list, check_exit_code=True, attempts=1): + def _run_ssh(self, cmd_list, check_exit_code=True): command = ' '.join(cmd_list) @@ -365,36 +366,16 @@ class CiscoFCZoneClientCLI(object): self.switch_pwd, min_size=1, max_size=5) - last_exception = None try: with self.sshpool.item() as ssh: - while attempts > 0: - attempts -= 1 - try: - return processutils.ssh_execute( - ssh, - command, - check_exit_code=check_exit_code) - except Exception as e: - msg = _("Exception: %s") % six.text_type(e) - LOG.error(msg) - last_exception = e - greenthread.sleep(random.randint(20, 500) / 100.0) - try: - raise processutils.ProcessExecutionError( - exit_code=last_exception.exit_code, - stdout=last_exception.stdout, - stderr=last_exception.stderr, - cmd=last_exception.cmd) - except AttributeError: - raise processutils.ProcessExecutionError( - exit_code=-1, - stdout="", - stderr="Error running SSH command", - cmd=command) + return processutils.ssh_execute( + ssh, + command, + check_exit_code=check_exit_code) + except Exception: with excutils.save_and_reraise_exception(): - LOG.error(_LE("Error running SSH command: %s"), command) + LOG.warning(_LW("Error running SSH command: %s"), command) def _ssh_execute(self, cmd_list, check_exit_code=True, attempts=1): """Execute cli with status update.