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 3b909e61bae4fd641c1b652e6961080bd8a6df35..0a8e89aa3cae14c16cca8dc98496524102fc1da7 100644 (file)
--- 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 f29a61acb8af42014ed87c400d9304a224aa4ef1..fef1e873c322ff7349df24e845eaf539e4e9b2b0 100644 (file)
--- 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