From b4af5221b463bb4bdbd3150e483b25e781d5ef88 Mon Sep 17 00:00:00 2001 From: zhangsong Date: Fri, 25 Mar 2016 15:39:01 +0800 Subject: [PATCH] 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 | 88 ++++++++++++------------------ cinder/volume/drivers/sheepdog.py | 51 +++++++---------- 2 files changed, 56 insertions(+), 83 deletions(-) diff --git a/cinder/tests/unit/test_sheepdog.py b/cinder/tests/unit/test_sheepdog.py index 81419e05340..358337e2a76 100644 --- a/cinder/tests/unit/test_sheepdog.py +++ b/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): diff --git a/cinder/volume/drivers/sheepdog.py b/cinder/volume/drivers/sheepdog.py index 51529ca7504..c5e91210f7d 100644 --- a/cinder/volume/drivers/sheepdog.py +++ b/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: - 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: + if 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)