From 547f107867619a8d0d670e86959addf2f41c6de0 Mon Sep 17 00:00:00 2001 From: zhangsong Date: Mon, 2 Nov 2015 18:28:22 +0800 Subject: [PATCH] SheepdogDriver: Improve get_volume_stats operation This patch improves get_volume_stats method to invoke dog command in SheepdogClient Class method instead of in SheepdogDriver class method directly. Here are two benefits we can realize as a result: 1.The current implementation can only get volume status by local sheepdog node, but the SheepdogClient Class also supports the method to run dog command with remote sheepdog node. 2.SheepdogClient Class methods are implemented to run dog command with fine grained Error handling. So it can improve the robustness and is more readable to run dog command in SheepdogClient Class method. Change-Id: I64ca193cd50e6914d0d5fb6cf711c760dfb65b8c Closes-Bug: #1512287 --- cinder/tests/unit/test_sheepdog.py | 72 +++++++++++++++++++++--------- cinder/volume/drivers/sheepdog.py | 29 ++++++++---- 2 files changed, 72 insertions(+), 29 deletions(-) diff --git a/cinder/tests/unit/test_sheepdog.py b/cinder/tests/unit/test_sheepdog.py index dbbd53bb4..3b909e61b 100644 --- a/cinder/tests/unit/test_sheepdog.py +++ b/cinder/tests/unit/test_sheepdog.py @@ -106,6 +106,10 @@ class SheepdogDriverTestDataGenerator(object): return ('env', 'LC_ALL=C', 'LANG=C', 'dog', 'vdi', 'resize', name, size, '-a', SHEEP_ADDR, '-p', SHEEP_PORT) + def cmd_dog_node_info(self): + return ('env', 'LC_ALL=C', 'LANG=C', 'dog', 'node', 'info', + '-a', SHEEP_ADDR, '-p', SHEEP_PORT, '-r') + CMD_DOG_CLUSTER_INFO = ('env', 'LC_ALL=C', 'LANG=C', 'dog', 'cluster', 'info', '-a', SHEEP_ADDR, '-p', SHEEP_PORT) @@ -1137,6 +1141,51 @@ class SheepdogClientTestCase(test.TestCase): self.assertTrue(fake_logger.error.called) self.assertEqual(expected_msg, ex.msg) + @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') + def test_get_volume_stats_success(self, fake_execute): + expected_cmd = ('node', 'info', '-r') + fake_execute.return_value = (self.test_data.COLLIE_NODE_INFO, '') + 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): + cmd = self.test_data.cmd_dog_node_info() + 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.get_volume_stats) + self.assertTrue(fake_logger.error.called) + self.assertEqual(expected_msg, ex.msg) + class SheepdogDriverTestCase(test.TestCase): def setUp(self): @@ -1173,10 +1222,9 @@ class SheepdogDriverTestCase(test.TestCase): 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, '' - self.stubs.Set(self.driver, '_execute', fake_stats) + @mock.patch.object(sheepdog.SheepdogClient, 'get_volume_stats') + def test_update_volume_stats(self, fake_execute): + fake_execute.return_value = self.test_data.COLLIE_NODE_INFO expected = dict( volume_backend_name='sheepdog', vendor_name='Open Source', @@ -1189,22 +1237,6 @@ class SheepdogDriverTestCase(test.TestCase): actual = self.driver.get_volume_stats(True) self.assertDictMatch(expected, actual) - def test_update_volume_stats_error(self): - def fake_stats(*args): - raise processutils.ProcessExecutionError() - self.stubs.Set(self.driver, '_execute', fake_stats) - expected = dict( - volume_backend_name='sheepdog', - vendor_name='Open Source', - driver_version=self.driver.VERSION, - storage_protocol='sheepdog', - total_capacity_gb='unknown', - free_capacity_gb='unknown', - reserved_percentage=0, - QoS_support=False) - actual = self.driver.get_volume_stats(True) - self.assertDictMatch(expected, actual) - @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') def test_copy_image_to_volume(self, fake_run_dog): @contextlib.contextmanager diff --git a/cinder/volume/drivers/sheepdog.py b/cinder/volume/drivers/sheepdog.py index 21956cdd5..b41c1b26e 100644 --- a/cinder/volume/drivers/sheepdog.py +++ b/cinder/volume/drivers/sheepdog.py @@ -320,6 +320,20 @@ class SheepdogClient(object): 'vdi: %(vdiname)s new size: %(size)s'), {'vdiname': vdiname, 'size': size}) + def get_volume_stats(self): + 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. ')) + return _stdout + class SheepdogIOWrapper(io.RawIOBase): """File-like object with Sheepdog backend.""" @@ -606,15 +620,12 @@ class SheepdogDriver(driver.VolumeDriver): stats['reserved_percentage'] = 0 stats['QoS_support'] = False - try: - stdout, _err = self._execute('collie', 'node', 'info', '-r') - m = self.stats_pattern.match(stdout) - total = float(m.group(1)) - used = float(m.group(2)) - stats['total_capacity_gb'] = total / units.Gi - stats['free_capacity_gb'] = (total - used) / units.Gi - except processutils.ProcessExecutionError: - LOG.exception(_LE('error refreshing volume stats')) + stdout = self.client.get_volume_stats() + m = self.stats_pattern.match(stdout) + total = float(m.group(1)) + used = float(m.group(2)) + stats['total_capacity_gb'] = total / units.Gi + stats['free_capacity_gb'] = (total - used) / units.Gi self._stats = stats -- 2.45.2