From: zhangsong Date: Sun, 15 Nov 2015 15:46:23 +0000 (+0800) Subject: Sheepdog: Optimization of error handling X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=a1c10609896ce6b3a3afe7b0ebf74c92cadfe5de;p=openstack-build%2Fcinder-build.git Sheepdog: Optimization of error handling Since each place where _run_dog() is called needs to handle a connection error, it would be more efficient to handle it inside the _run_dog() method. Change-Id: Ib1eb3044e583b46a8a42f820bf780600415122e8 --- diff --git a/cinder/tests/unit/test_sheepdog.py b/cinder/tests/unit/test_sheepdog.py index 3b909e61b..0a8e89aa3 100644 --- a/cinder/tests/unit/test_sheepdog.py +++ b/cinder/tests/unit/test_sheepdog.py @@ -424,6 +424,23 @@ class SheepdogClientTestCase(test.TestCase): self.assertRaises(OSError, self.client._run_dog, *args) self.assertTrue(fake_logger.error.called) + @mock.patch.object(utils, 'execute') + @mock.patch.object(sheepdog, 'LOG') + def test_run_dog_fail_to_connect(self, fake_logger, fake_execute): + args = ('cluster', 'info') + cmd = self.test_data.CMD_DOG_CLUSTER_INFO + exit_code = 2 + stdout = 'stdout dummy' + stderr = self.test_data.DOG_COMMAND_ERROR_FAIL_TO_CONNECT + expected_reason = (_('Failed to connect to sheep daemon. ' + 'addr: %(addr)s, port: %(port)s'), + {'addr': SHEEP_ADDR, 'port': SHEEP_PORT}) + fake_execute.side_effect = processutils.ProcessExecutionError( + cmd=cmd, exit_code=exit_code, stdout=stdout, stderr=stderr) + ex = self.assertRaises(exception.SheepdogError, + self.client._run_dog, *args) + self.assertEqual(expected_reason, ex.kwargs['reason']) + @mock.patch.object(utils, 'execute') @mock.patch.object(sheepdog, 'LOG') def test_run_dog_unknown_error(self, fake_logger, fake_execute): @@ -556,26 +573,6 @@ class SheepdogClientTestCase(test.TestCase): self.client.check_cluster_status) self.assertEqual(expected_reason, ex.kwargs['reason']) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') - @mock.patch.object(sheepdog, 'LOG') - def test_check_cluster_status_fail_to_connect(self, fake_logger, - fake_execute): - cmd = self.test_data.CMD_DOG_CLUSTER_INFO - exit_code = 2 - stdout = 'stdout_dummy' - stderr = self.test_data.DOG_COMMAND_ERROR_FAIL_TO_CONNECT - expected_msg = self.test_data.sheepdog_cmd_error(cmd=cmd, - exit_code=exit_code, - stdout=stdout, - stderr=stderr) - fake_execute.side_effect = exception.SheepdogCmdError( - cmd=cmd, exit_code=exit_code, stdout=stdout.replace('\n', '\\n'), - stderr=stderr.replace('\n', '\\n')) - ex = self.assertRaises(exception.SheepdogCmdError, - self.client.check_cluster_status) - self.assertEqual(expected_msg, ex.msg) - self.assertTrue(fake_logger.error.called) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') @mock.patch.object(sheepdog, 'LOG') def test_check_cluster_status_unknown_error(self, fake_logger, @@ -601,23 +598,6 @@ class SheepdogClientTestCase(test.TestCase): self.client.create(self._vdiname, self._vdisize) fake_execute.assert_called_once_with(*expected_cmd) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') - @mock.patch.object(sheepdog, 'LOG') - def test_create_fail_to_connect(self, fake_logger, fake_execute): - cmd = self.test_data.cmd_dog_vdi_create(self._vdiname, self._vdisize) - exit_code = 2 - stdout = '' - stderr = self.test_data.DOG_COMMAND_ERROR_FAIL_TO_CONNECT - expected_msg = self.test_data.sheepdog_cmd_error( - cmd=cmd, exit_code=exit_code, stdout=stdout, stderr=stderr) - fake_execute.side_effect = exception.SheepdogCmdError( - cmd=cmd, exit_code=exit_code, stdout=stdout.replace('\n', '\\n'), - stderr=stderr.replace('\n', '\\n')) - ex = self.assertRaises(exception.SheepdogCmdError, self.client.create, - self._vdiname, self._vdisize) - self.assertTrue(fake_logger.error.called) - self.assertEqual(expected_msg, ex.msg) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') @mock.patch.object(sheepdog, 'LOG') def test_create_vdi_already_exists(self, fake_logger, fake_execute): @@ -684,25 +664,6 @@ class SheepdogClientTestCase(test.TestCase): self.client.delete, self._vdiname) self.assertEqual(expected_reason, ex.kwargs['reason']) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') - @mock.patch.object(sheepdog, 'LOG') - def test_delete_fail_to_connect(self, fake_logger, fake_execute): - cmd = self.test_data.cmd_dog_vdi_delete(self._vdiname) - exit_code = 2 - stdout = 'stdout_dummy' - stderr = self.test_data.DOG_COMMAND_ERROR_FAIL_TO_CONNECT - expected_msg = self.test_data.sheepdog_cmd_error(cmd=cmd, - exit_code=exit_code, - stdout=stdout, - stderr=stderr) - fake_execute.side_effect = exception.SheepdogCmdError( - cmd=cmd, exit_code=exit_code, stdout=stdout.replace('\n', '\\n'), - stderr=stderr.replace('\n', '\\n')) - ex = self.assertRaises(exception.SheepdogCmdError, - self.client.delete, self._vdiname) - self.assertTrue(fake_logger.error.called) - self.assertEqual(expected_msg, ex.msg) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') @mock.patch.object(sheepdog, 'LOG') def test_delete_unknown_error(self, fake_logger, fake_execute): @@ -731,26 +692,6 @@ class SheepdogClientTestCase(test.TestCase): self.client.create_snapshot(*args) fake_execute.assert_called_once_with(*expected_cmd) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') - @mock.patch.object(sheepdog, 'LOG') - def test_create_snapshot_fail_to_connect(self, fake_logger, fake_execute): - args = (self._src_vdiname, self._snapname) - cmd = self.test_data.cmd_dog_vdi_create_snapshot(*args) - exit_code = 2 - stdout = '' - stderr = self.test_data.DOG_COMMAND_ERROR_FAIL_TO_CONNECT - expected_msg = self.test_data.sheepdog_cmd_error(cmd=cmd, - exit_code=exit_code, - stdout=stdout, - stderr=stderr) - fake_execute.side_effect = exception.SheepdogCmdError( - cmd=cmd, exit_code=exit_code, stdout=stdout.replace('\n', '\\n'), - stderr=stderr.replace('\n', '\\n')) - ex = self.assertRaises(exception.SheepdogCmdError, - self.client.create_snapshot, *args) - self.assertTrue(fake_logger.error.called) - self.assertEqual(expected_msg, ex.msg) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') @mock.patch.object(sheepdog, 'LOG') def test_create_snapshot_vdi_not_found(self, fake_logger, fake_execute): @@ -859,26 +800,6 @@ class SheepdogClientTestCase(test.TestCase): self.client.delete_snapshot, *args) self.assertEqual(expected_reason, ex.kwargs['reason']) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') - @mock.patch.object(sheepdog, 'LOG') - def test_delete_snapshot_fail_to_connect(self, fake_logger, fake_execute): - args = (self._src_vdiname, self._snapname) - cmd = self.test_data.cmd_dog_vdi_delete_snapshot(*args) - exit_code = 2 - stdout = '' - stderr = self.test_data.DOG_COMMAND_ERROR_FAIL_TO_CONNECT - expected_msg = self.test_data.sheepdog_cmd_error(cmd=cmd, - exit_code=exit_code, - stdout=stdout, - stderr=stderr) - fake_execute.side_effect = exception.SheepdogCmdError( - cmd=cmd, exit_code=exit_code, stdout=stdout.replace('\n', '\\n'), - stderr=stderr.replace('\n', '\\n')) - ex = self.assertRaises(exception.SheepdogCmdError, - self.client.delete_snapshot, *args) - self.assertTrue(fake_logger.error.called) - self.assertEqual(expected_msg, ex.msg) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') @mock.patch.object(sheepdog, 'LOG') def test_delete_snapshot_unknown_error(self, fake_logger, fake_execute): @@ -1045,25 +966,6 @@ class SheepdogClientTestCase(test.TestCase): self.client.resize(self._vdiname, 10) fake_execute.assert_called_once_with(*expected_cmd) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') - @mock.patch.object(sheepdog, 'LOG') - def test_resize_fail_to_connect(self, fake_logger, fake_execute): - cmd = self.test_data.cmd_dog_vdi_resize(self._vdiname, 10 * 1024 ** 3) - exit_code = 2 - stdout = 'stdout_dummy' - stderr = self.test_data.DOG_COMMAND_ERROR_FAIL_TO_CONNECT - expected_msg = self.test_data.sheepdog_cmd_error(cmd=cmd, - exit_code=exit_code, - stdout=stdout, - stderr=stderr) - fake_execute.side_effect = exception.SheepdogCmdError( - cmd=cmd, exit_code=exit_code, stdout=stdout.replace('\n', '\\n'), - stderr=stderr.replace('\n', '\\n')) - ex = self.assertRaises(exception.SheepdogCmdError, - self.client.resize, self._vdiname, 10) - self.assertTrue(fake_logger.error.called) - self.assertEqual(expected_msg, ex.msg) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') @mock.patch.object(sheepdog, 'LOG') def test_resize_vdi_not_found(self, fake_logger, fake_execute): @@ -1148,25 +1050,6 @@ class SheepdogClientTestCase(test.TestCase): self.client.get_volume_stats() fake_execute.assert_called_once_with(*expected_cmd) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') - @mock.patch.object(sheepdog, 'LOG') - def test_get_volume_stats_fail_to_connect(self, fake_logger, fake_execute): - cmd = self.test_data.cmd_dog_node_info() - exit_code = 2 - stdout = 'stdout_dummy' - stderr = self.test_data.DOG_COMMAND_ERROR_FAIL_TO_CONNECT - expected_msg = self.test_data.sheepdog_cmd_error(cmd=cmd, - exit_code=exit_code, - stdout=stdout, - stderr=stderr) - fake_execute.side_effect = exception.SheepdogCmdError( - cmd=cmd, exit_code=exit_code, stdout=stdout.replace('\n', '\\n'), - stderr=stderr.replace('\n', '\\n')) - ex = self.assertRaises(exception.SheepdogCmdError, - self.client.get_volume_stats) - self.assertTrue(fake_logger.error.called) - self.assertEqual(expected_msg, ex.msg) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') @mock.patch.object(sheepdog, 'LOG') def test_get_volume_stats_unknown_error(self, fake_logger, fake_execute): diff --git a/cinder/volume/drivers/sheepdog.py b/cinder/volume/drivers/sheepdog.py index f29a61acb..fef1e873c 100644 --- a/cinder/volume/drivers/sheepdog.py +++ b/cinder/volume/drivers/sheepdog.py @@ -94,6 +94,12 @@ class SheepdogClient(object): msg = _LE('OSError: command is %s.') LOG.error(msg, cmd) except processutils.ProcessExecutionError as e: + _stderr = e.stderr + if _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR): + reason = (_('Failed to connect to sheep daemon. ' + 'addr: %(addr)s, port: %(port)s'), + {'addr': self.addr, 'port': self.port}) + raise exception.SheepdogError(reason=reason) raise exception.SheepdogCmdError( cmd=e.cmd, exit_code=e.exit_code, @@ -135,15 +141,9 @@ class SheepdogClient(object): (_stdout, _stderr) = self._run_dog('cluster', 'info') except exception.SheepdogCmdError as e: cmd = e.kwargs['cmd'] - _stderr = e.kwargs['stderr'] with excutils.save_and_reraise_exception(): - if _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR): - msg = _LE('Failed to connect to sheep daemon. ' - 'addr: %(addr)s, port: %(port)s') - LOG.error(msg, {'addr': self.addr, 'port': self.port}) - else: - LOG.error(_LE('Failed to check cluster status.' - '(command: %s)'), cmd) + LOG.error(_LE('Failed to check cluster status.' + '(command: %s)'), cmd) if _stdout.startswith(self.DOG_RESP_CLUSTER_RUNNING): LOG.debug('Sheepdog cluster is running.') @@ -164,11 +164,7 @@ class SheepdogClient(object): except exception.SheepdogCmdError as e: _stderr = e.kwargs['stderr'] with excutils.save_and_reraise_exception(): - if _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR): - LOG.error(_LE("Failed to connect to sheep daemon. " - "addr: %(addr)s, port: %(port)s"), - {'addr': self.addr, 'port': self.port}) - elif _stderr.rstrip('\\n').endswith( + if _stderr.rstrip('\\n').endswith( self.DOG_RESP_VDI_ALREADY_EXISTS): LOG.error(_LE('Volume already exists. %s'), vdiname) else: @@ -194,12 +190,7 @@ class SheepdogClient(object): except exception.SheepdogCmdError as e: _stderr = e.kwargs['stderr'] with excutils.save_and_reraise_exception(): - if _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR): - LOG.error(_LE('Failed to connect to sheep daemon. ' - 'addr: %(addr)s, port: %(port)s'), - {'addr': self.addr, 'port': self.port}) - else: - LOG.error(_LE('Failed to delete volume. %s'), vdiname) + LOG.error(_LE('Failed to delete volume. %s'), vdiname) def create_snapshot(self, vdiname, snapname): try: @@ -208,11 +199,7 @@ class SheepdogClient(object): cmd = e.kwargs['cmd'] _stderr = e.kwargs['stderr'] with excutils.save_and_reraise_exception(): - if _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR): - LOG.error(_LE('Failed to connect to sheep daemon. ' - 'addr: %(addr)s, port: %(port)s'), - {'addr': self.addr, 'port': self.port}) - elif _stderr.rstrip('\\n').endswith( + if _stderr.rstrip('\\n').endswith( self.DOG_RESP_SNAPSHOT_VDI_NOT_FOUND): LOG.error(_LE('Volume "%s" not found. Please check the ' 'results of "dog vdi list".'), @@ -249,13 +236,8 @@ class SheepdogClient(object): cmd = e.kwargs['cmd'] _stderr = e.kwargs['stderr'] with excutils.save_and_reraise_exception(): - if _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR): - msg = _LE('Failed to connect to sheep daemon. ' - 'addr: %(addr)s, port: %(port)s') - LOG.error(msg, {'addr': self.addr, 'port': self.port}) - else: - LOG.error(_LE('Failed to delete snapshot. (command: %s)'), - cmd) + LOG.error(_LE('Failed to delete snapshot. (command: %s)'), + cmd) def clone(self, src_vdiname, src_snapname, dst_vdiname, size): try: @@ -296,11 +278,7 @@ class SheepdogClient(object): except exception.SheepdogCmdError as e: _stderr = e.kwargs['stderr'] with excutils.save_and_reraise_exception(): - if _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR): - LOG.error(_LE('Failed to connect to sheep daemon. ' - 'addr: %(addr)s, port: %(port)s'), - {'addr': self.addr, 'port': self.port}) - elif _stderr.rstrip('\\n').endswith( + if _stderr.rstrip('\\n').endswith( self.DOG_RESP_VDI_NOT_FOUND): LOG.error(_LE('Failed to resize vdi. vdi not found. %s'), vdiname) @@ -323,14 +301,8 @@ class SheepdogClient(object): try: (_stdout, _stderr) = self._run_dog('node', 'info', '-r') except exception.SheepdogCmdError as e: - _stderr = e.kwargs['stderr'] with excutils.save_and_reraise_exception(): - if _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR): - LOG.error(_LE('Failed to connect to sheep daemon. ' - 'addr: %(addr)s, port: %(port)s'), - {'addr': self.addr, 'port': self.port}) - else: - LOG.error(_LE('Failed to get volume status. ')) + LOG.error(_LE('Failed to get volume status. %s'), e) return _stdout