From 247b2bf485608b81d67a2999240055fa56e5e34a Mon Sep 17 00:00:00 2001 From: Teruaki Ishizaki Date: Tue, 30 Jun 2015 15:38:46 +0900 Subject: [PATCH] Sheepdog: improve create and delete operation This patch changes create_volume and delete_volume to use SheepdogClient Class method for detailed Error logs. SheepdogClient Class methods are implemented to enable fine grained Error handling. So, the administrators are enabled to do error recovery more easily using those logs. Change-Id: I54d320475c143c5f96d70abd7314320ef79f55bf Depends-On: I738c23b9213ebd781ab399a3198551c8b8dfe382 --- cinder/tests/unit/test_sheepdog.py | 165 ++++++++++++++++++++++++++++- cinder/volume/drivers/sheepdog.py | 61 +++++++++-- 2 files changed, 212 insertions(+), 14 deletions(-) diff --git a/cinder/tests/unit/test_sheepdog.py b/cinder/tests/unit/test_sheepdog.py index 56de00218..74f0c474c 100644 --- a/cinder/tests/unit/test_sheepdog.py +++ b/cinder/tests/unit/test_sheepdog.py @@ -59,6 +59,14 @@ class SheepdogDriverTestDataGenerator(object): return fake_volume.fake_volume_obj(context.get_admin_context(), **volume_data) + def cmd_dog_vdi_create(self, name, size): + return ('env', 'LC_ALL=C', 'LANG=C', 'dog', 'vdi', 'create', name, + '%sG' % size, '-a', SHEEP_ADDR, '-p', str(SHEEP_PORT)) + + def cmd_dog_vdi_delete(self, name): + return ('env', 'LC_ALL=C', 'LANG=C', 'dog', 'vdi', 'delete', name, + '-a', SHEEP_ADDR, '-p', str(SHEEP_PORT)) + CMD_DOG_CLUSTER_INFO = ('env', 'LC_ALL=C', 'LANG=C', 'dog', 'cluster', 'info', '-a', SHEEP_ADDR, '-p', str(SHEEP_PORT)) @@ -123,6 +131,14 @@ Epoch Time Version [Host:Port:V-Nodes,,,] DOG_CLUSTER_INFO_SHUTTING_DOWN = """\ Cluster status: System is shutting down +""" + + DOG_VDI_CREATE_VDI_ALREADY_EXISTS = """\ +Failed to create VDI %(vdiname)s: VDI exists already +""" + + DOG_VDI_DELETE_VDI_NOT_EXISTS = """\ +Failed to open VDI %(vdiname)s (snapshot id: 0 snapshot tag: ): No VDI found """ DOG_COMMAND_ERROR_FAIL_TO_CONNECT = """\ @@ -249,6 +265,8 @@ class SheepdogClientTestCase(test.TestCase): self.driver.do_setup(None) self.test_data = SheepdogDriverTestDataGenerator() self.client = self.driver.client + self._vdiname = self.test_data.TEST_VOLUME.name + self._vdisize = self.test_data.TEST_VOLUME.size @mock.patch.object(utils, 'execute') def test_run_dog_success(self, fake_execute): @@ -396,6 +414,134 @@ class SheepdogClientTestCase(test.TestCase): self.client.check_cluster_status) self.assertEqual(expected_msg, ex.msg) + @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') + def test_create_success(self, fake_execute): + expected_cmd = ('vdi', 'create', self._vdiname, '%sG' % self._vdisize) + fake_execute.return_value = ('', '') + 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): + cmd = self.test_data.cmd_dog_vdi_create(self._vdiname, self._vdisize) + exit_code = 1 + stdout = '' + stderr = (self.test_data.DOG_VDI_CREATE_VDI_ALREADY_EXISTS % + {'vdiname': self._vdiname}) + 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_unknown_error(self, fake_logger, fake_execute): + cmd = self.test_data.cmd_dog_vdi_create(self._vdiname, self._vdisize) + exit_code = 1 + stdout = 'stdout_dummy' + stderr = 'stderr_dummy' + 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') + def test_delete_success(self, fake_execute): + expected_cmd = ('vdi', 'delete', self._vdiname) + fake_execute.return_value = ('', '') + self.client.delete(self._vdiname) + fake_execute.assert_called_once_with(*expected_cmd) + + @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') + @mock.patch.object(sheepdog, 'LOG') + def test_delete_vdi_not_found(self, fake_logger, fake_execute): + stdout = '' + stderr = (self.test_data.DOG_VDI_DELETE_VDI_NOT_EXISTS % + {'vdiname': self._vdiname}) + fake_execute.return_value = (stdout, stderr) + 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_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): + cmd = self.test_data.cmd_dog_vdi_delete(self._vdiname) + exit_code = 2 + stdout = 'stdout_dummy' + stderr = 'stderr_dummy' + 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) + class SheepdogDriverTestCase(test.TestCase): def setUp(self): @@ -410,12 +556,24 @@ class SheepdogDriverTestCase(test.TestCase): self.driver.do_setup(None) self.test_data = SheepdogDriverTestDataGenerator() self.client = self.driver.client + self._vdiname = self.test_data.TEST_VOLUME.name + self._vdisize = self.test_data.TEST_VOLUME.size @mock.patch.object(sheepdog.SheepdogClient, 'check_cluster_status') def test_check_for_setup_error(self, fake_execute): self.driver.check_for_setup_error() fake_execute.assert_called_once_with() + @mock.patch.object(sheepdog.SheepdogClient, 'create') + def test_create_volume(self, fake_execute): + self.driver.create_volume(self.test_data.TEST_VOLUME) + fake_execute.assert_called_once_with(self._vdiname, self._vdisize) + + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + def test_delete_volume(self, fake_execute): + self.driver.delete_volume(self.test_data.TEST_VOLUME) + fake_execute.assert_called_once_with(self._vdiname) + def test_update_volume_stats(self): def fake_stats(*args): return self.test_data.COLLIE_NODE_INFO, '' @@ -448,7 +606,8 @@ class SheepdogDriverTestCase(test.TestCase): actual = self.driver.get_volume_stats(True) self.assertDictMatch(expected, actual) - def test_copy_image_to_volume(self): + @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') + def test_copy_image_to_volume(self, fake_run_dog): @contextlib.contextmanager def fake_temp_file(): class FakeTmp(object): @@ -467,8 +626,8 @@ class SheepdogDriverTestCase(test.TestCase): self.stubs.Set(sheepdog.SheepdogDriver, '_try_execute', fake_try_execute) - self.driver.copy_image_to_volume(None, {'name': 'test', - 'size': 1}, + fake_run_dog.return_value = ('fake_stdout', 'fake_stderr') + self.driver.copy_image_to_volume(None, self.test_data.TEST_VOLUME, FakeImageService(), None) def test_copy_volume_to_image(self): diff --git a/cinder/volume/drivers/sheepdog.py b/cinder/volume/drivers/sheepdog.py index ff54f6e37..66443a306 100644 --- a/cinder/volume/drivers/sheepdog.py +++ b/cinder/volume/drivers/sheepdog.py @@ -31,7 +31,7 @@ from oslo_utils import excutils from oslo_utils import units from cinder import exception -from cinder.i18n import _, _LE +from cinder.i18n import _, _LE, _LW from cinder.image import image_utils from cinder import utils from cinder.volume import driver @@ -62,6 +62,8 @@ class SheepdogClient(object): 'Waiting for cluster to be formatted') DOG_RESP_CLUSTER_WAITING = ('Cluster status: ' 'Waiting for other nodes to join cluster') + DOG_RESP_VDI_ALREADY_EXISTS = ': VDI exists already' + DOG_RESP_VDI_NOT_FOUND = ': No VDI found' def __init__(self, addr, port): self.addr = addr @@ -95,7 +97,7 @@ class SheepdogClient(object): _stderr = e.kwargs['stderr'] with excutils.save_and_reraise_exception(): if _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR): - msg = _LE('Failed to connect sheep daemon. ' + msg = _LE('Failed to connect to sheep daemon. ' 'addr: %(addr)s, port: %(port)s') LOG.error(msg, {'addr': self.addr, 'port': self.port}) else: @@ -115,6 +117,49 @@ class SheepdogClient(object): 'Ensure all sheep daemons are running.') raise exception.SheepdogError(reason=reason) + def create(self, vdiname, size): + try: + self._run_dog('vdi', 'create', vdiname, '%sG' % size) + 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( + self.DOG_RESP_VDI_ALREADY_EXISTS): + LOG.error(_LE('Volume already exists. %s'), vdiname) + else: + LOG.error(_LE('Failed to create volume. %s'), vdiname) + + def delete(self, vdiname): + try: + (_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(): + 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) + class SheepdogIOWrapper(io.RawIOBase): """File-like object with Sheepdog backend.""" @@ -303,9 +348,7 @@ class SheepdogDriver(driver.VolumeDriver): def create_volume(self, volume): """Create a sheepdog volume.""" - self._try_execute('qemu-img', 'create', - "sheepdog:%s" % volume['name'], - '%sG' % volume['size']) + self.client.create(volume.name, volume.size) def create_volume_from_snapshot(self, volume, snapshot): """Create a sheepdog volume from a snapshot.""" @@ -317,7 +360,7 @@ class SheepdogDriver(driver.VolumeDriver): def delete_volume(self, volume): """Delete a logical volume.""" - self._delete(volume) + self.client.delete(volume.name) def _resize(self, volume, size=None): if not size: @@ -326,10 +369,6 @@ class SheepdogDriver(driver.VolumeDriver): self._try_execute('collie', 'vdi', 'resize', volume['name'], size) - def _delete(self, volume): - self._try_execute('collie', 'vdi', 'delete', - volume['name']) - def copy_image_to_volume(self, context, volume, image_service, image_id): with image_utils.temporary_file() as tmp: # (wenhao): we don't need to convert to raw for sheepdog. @@ -338,7 +377,7 @@ class SheepdogDriver(driver.VolumeDriver): # remove the image created by import before this function. # see volume/drivers/manager.py:_create_volume - self._delete(volume) + self.client.delete(volume.name) # convert and store into sheepdog image_utils.convert_image(tmp, 'sheepdog:%s' % volume['name'], 'raw') -- 2.45.2