Sheepdog:optimization of connection error handling

This patch optimized the processing method of sheepdog's bug. It
deal with the connection error in __run_dog and __run_qemu_img
functions.

Change-Id: Ia42ee91a720a4347c00146059d48d9b031e7f1a2
Closes-Bug: #1561874

cinder/tests/unit/test_sheepdog.py

@@ -444,6 +444,22 @@ class SheepdogClientTestCase(test.TestCase):
                                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_fail_to_connect_bugcase(self, fake_logger, fake_execute):
+        # NOTE(zhangsong): Sheepdog's bug case.
+        # details are written to Sheepdog driver code.
+        args = ('node', 'list')
+        stdout = ''
+        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.return_value = (stdout, 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):
@@ -498,7 +514,25 @@ class SheepdogClientTestCase(test.TestCase):
 
     @mock.patch.object(utils, 'execute')
     @mock.patch.object(sheepdog, 'LOG')
-    def test_run_qemu_img_execution_error(self, fake_logger, fake_execute):
+    def test_run_qemu_img_fail_to_connect(self, fake_logger, fake_execute):
+        args = ('create', 'dummy')
+        cmd = ('qemu-img', 'create', 'dummy')
+        exit_code = 1
+        stdout = 'stdout dummy'
+        stderr = self.test_data.QEMU_IMG_FAILED_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_qemu_img, *args)
+        self.assertEqual(expected_reason, ex.kwargs['reason'])
+
+    @mock.patch.object(utils, 'execute')
+    @mock.patch.object(sheepdog, 'LOG')
+    def test_run_qemu_img_unknown_execution_error(self, fake_logger,
+                                                  fake_execute):
         args = ('create', 'dummy')
         cmd = ('qemu-img', 'create', 'dummy')
         exit_code = 1
@@ -653,20 +687,6 @@ class SheepdogClientTestCase(test.TestCase):
         self.client.delete(self._vdiname)
         self.assertTrue(fake_logger.warning.called)
 
-    @mock.patch.object(sheepdog.SheepdogClient, '_run_dog')
-    def test_delete_fail_to_connect_bugcase(self, fake_execute):
-        # NOTE(tishizaki): Sheepdog's bug case.
-        # details are written to Sheepdog driver code.
-        stdout = ''
-        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.return_value = (stdout, stderr)
-        ex = self.assertRaises(exception.SheepdogError,
-                               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_unknown_error(self, fake_logger, fake_execute):
@@ -786,23 +806,6 @@ class SheepdogClientTestCase(test.TestCase):
         self.client.delete_snapshot(*args)
         self.assertTrue(fake_logger.warning.called)
 
-    @mock.patch.object(sheepdog.SheepdogClient, '_run_dog')
-    @mock.patch.object(sheepdog, 'LOG')
-    def test_delete_snapshot_fail_to_connect_bugcase(self, fake_logger,
-                                                     fake_execute):
-        # NOTE(tishizaki): Sheepdog's bug case.
-        # details are written to Sheepdog driver code.
-        args = (self._src_vdiname, self._snapname)
-        stdout = ''
-        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.return_value = (stdout, stderr)
-        ex = self.assertRaises(exception.SheepdogError,
-                               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_unknown_error(self, fake_logger, fake_execute):
@@ -836,27 +839,6 @@ class SheepdogClientTestCase(test.TestCase):
         self.client.clone(*args)
         fake_execute.assert_called_once_with(*expected_cmd)
 
-    @mock.patch.object(sheepdog.SheepdogClient, '_run_qemu_img')
-    @mock.patch.object(sheepdog, 'LOG')
-    def test_clone_fail_to_connect(self, fake_logger, fake_execute):
-        args = (self._src_vdiname, self._snapname,
-                self._dst_vdiname, self._dst_vdisize)
-        cmd = self.test_data.cmd_qemuimg_vdi_clone(*args)
-        exit_code = 2
-        stdout = 'stdout_dummy'
-        stderr = self.test_data.QEMU_IMG_FAILED_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.clone,
-                               *args)
-        self.assertTrue(fake_logger.error.called)
-        self.assertEqual(expected_msg, ex.msg)
-
     @mock.patch.object(sheepdog.SheepdogClient, '_run_qemu_img')
     @mock.patch.object(sheepdog, 'LOG')
     def test_clone_dst_vdi_already_exists(self, fake_logger, fake_execute):

cinder/volume/drivers/sheepdog.py

@@ -84,7 +84,20 @@ class SheepdogClient(object):
         cmd = ('env', 'LC_ALL=C', 'LANG=C', 'dog', command, subcommand,
                '-a', self.addr, '-p', self.port) + params
         try:
-            return utils.execute(*cmd)
+            (_stdout, _stderr) = utils.execute(*cmd)
+            if _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR):
+                # NOTE(tishizaki)
+                # Dog command does not return error_code although
+                # dog command cannot connect to sheep process.
+                # That is a Sheepdog's bug.
+                # To avoid a Sheepdog's bug, now we need to check stderr.
+                # If Sheepdog has been fixed, this check logic is needed
+                # by old Sheepdog users.
+                reason = (_('Failed to connect to sheep daemon. '
+                          'addr: %(addr)s, port: %(port)s'),
+                          {'addr': self.addr, 'port': self.port})
+                raise exception.SheepdogError(reason=reason)
+            return (_stdout, _stderr)
         except OSError as e:
             with excutils.save_and_reraise_exception():
                 if e.errno == errno.ENOENT:
@@ -130,6 +143,12 @@ class SheepdogClient(object):
                     msg = _LE('OSError: command is %(cmd)s.')
                 LOG.error(msg, {'cmd': tuple(cmd)})
         except processutils.ProcessExecutionError as e:
+            _stderr = e.stderr
+            if self.QEMU_IMG_RESP_CONNECTION_ERROR in _stderr:
+                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,
@@ -175,18 +194,6 @@ class SheepdogClient(object):
             (_stdout, _stderr) = self._run_dog('vdi', 'delete', vdiname)
             if _stderr.rstrip().endswith(self.DOG_RESP_VDI_NOT_FOUND):
                 LOG.warning(_LW('Volume not found. %s'), vdiname)
-            elif _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR):
-                # NOTE(tishizaki)
-                # Dog command does not return error_code although
-                # dog command cannot connect to sheep process.
-                # That is a Sheepdog's bug.
-                # To avoid a Sheepdog's bug, now we need to check stderr.
-                # If Sheepdog has been fixed, this check logic is needed
-                # by old Sheepdog users.
-                reason = (_('Failed to connect to sheep daemon. '
-                          'addr: %(addr)s, port: %(port)s'),
-                          {'addr': self.addr, 'port': self.port})
-                raise exception.SheepdogError(reason=reason)
         except exception.SheepdogCmdError as e:
             _stderr = e.kwargs['stderr']
             with excutils.save_and_reraise_exception():
@@ -220,18 +227,6 @@ class SheepdogClient(object):
                 LOG.warning(_LW('Snapshot "%s" not found.'), snapname)
             elif _stderr.rstrip().endswith(self.DOG_RESP_VDI_NOT_FOUND):
                 LOG.warning(_LW('Volume "%s" not found.'), vdiname)
-            elif _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR):
-                # NOTE(tishizaki)
-                # Dog command does not return error_code although
-                # dog command cannot connect to sheep process.
-                # That is a Sheepdog's bug.
-                # To avoid a Sheepdog's bug, now we need to check stderr.
-                # If Sheepdog has been fixed, this check logic is needed
-                # by old Sheepdog users.
-                reason = (_('Failed to connect to sheep daemon. '
-                          'addr: %(addr)s, port: %(port)s'),
-                          {'addr': self.addr, 'port': self.port})
-                raise exception.SheepdogError(reason=reason)
         except exception.SheepdogCmdError as e:
             cmd = e.kwargs['cmd']
             _stderr = e.kwargs['stderr']
@@ -250,11 +245,7 @@ class SheepdogClient(object):
             cmd = e.kwargs['cmd']
             _stderr = e.kwargs['stderr']
             with excutils.save_and_reraise_exception():
-                if self.QEMU_IMG_RESP_CONNECTION_ERROR in _stderr:
+                if self.QEMU_IMG_RESP_ALREADY_EXISTS in _stderr:
-                    LOG.error(_LE('Failed to connect to sheep daemon. '
-                                  'addr: %(addr)s, port: %(port)s'),
-                              {'addr': self.addr, 'port': self.port})
-                elif self.QEMU_IMG_RESP_ALREADY_EXISTS in _stderr:
                     LOG.error(_LE('Clone volume "%s" already exists. '
                               'Please check the results of "dog vdi list".'),
                               dst_vdiname)