From 7e75c380a67cd270082402149b6007f1c8fc50a9 Mon Sep 17 00:00:00 2001 From: Csaba Henk Date: Tue, 7 Apr 2015 18:39:49 +0200 Subject: [PATCH] glusterfs, glusterfs_native: perform version checks - add gluster_version method to GlusterManager class - gluster: check if version of the GlusterFS server is at least 3.5 - gluster_native: check if version of the GlusterFS server is at least 3.6 - gluster_native: on snaphot creation failure, interpret errno only for GlusterFS strictly later than 3.6 Change-Id: I242ea83c3a31670eb6a13c11e39d0c2228170c50 Closes-Bug: #1417352 --- manila/share/drivers/glusterfs.py | 60 +++++++++++++++++++ manila/share/drivers/glusterfs_native.py | 58 +++++++++++++++--- manila/tests/share/drivers/test_glusterfs.py | 53 ++++++++++++++++ .../share/drivers/test_glusterfs_native.py | 53 ++++++++++++++-- 4 files changed, 210 insertions(+), 14 deletions(-) diff --git a/manila/share/drivers/glusterfs.py b/manila/share/drivers/glusterfs.py index 3fbb4856cd..ee9c4eba97 100644 --- a/manila/share/drivers/glusterfs.py +++ b/manila/share/drivers/glusterfs.py @@ -85,6 +85,8 @@ CONF.register_opts(GlusterfsManilaShare_opts) NFS_EXPORT_DIR = 'nfs.export-dir' NFS_EXPORT_VOL = 'nfs.export-volumes' +GLUSTERFS_VERSION_MIN = (3, 5) + class GlusterManager(object): """Interface with a GlusterFS volume.""" @@ -162,6 +164,63 @@ class GlusterManager(object): if o == option: return v + def get_gluster_version(self): + """Retrieve GlusterFS version. + + :returns: version (as tuple of strings, example: ('3', '6', '0beta2')) + """ + try: + out, err = self.gluster_call('--version') + except exception.ProcessExecutionError as exc: + raise exception.GlusterfsException( + _("'gluster version' failed on server " + "%(server)s: %(message)s") % + {'server': self.host, 'message': exc.message}) + try: + owords = out.split() + if owords[0] != 'glusterfs': + raise RuntimeError + vers = owords[1].split('.') + # provoke an exception if vers does not start with two numerals + int(vers[0]) + int(vers[1]) + except Exception: + raise exception.GlusterfsException( + _("Cannot parse version info obtained from server " + "%(server)s, version info: %(info)s") % + {'server': self.host, 'info': out}) + return vers + + def check_gluster_version(self, minvers): + """Retrieve and check GlusterFS version. + + :param minvers: minimum version to require + (given as tuple of integers, example: (3, 6)) + """ + vers = self.get_gluster_version() + if self.numreduct(vers) < minvers: + raise exception.GlusterfsException(_( + "Unsupported GlusterFS version %(version)s on server " + "%(server)s, minimum requirement: %(minvers)s") % { + 'server': self.host, + 'version': '.'.join(vers), + 'minvers': '.'.join(six.text_type(c) for c in minvers)}) + + @staticmethod + def numreduct(vers): + """The numeric reduct of a tuple of strings. + + That is, applying an integer conversion map on the longest + initial segment of vers which consists of numerals. + """ + numvers = [] + for c in vers: + try: + numvers.append(int(c)) + except ValueError: + break + return tuple(numvers) + class GlusterfsShareDriver(driver.ExecuteMixin, driver.GaneshaMixin, driver.ShareDriver,): @@ -189,6 +248,7 @@ class GlusterfsShareDriver(driver.ExecuteMixin, driver.GaneshaMixin, self.configuration.glusterfs_path_to_private_key, self.configuration.glusterfs_server_password, ) + self.gluster_manager.check_gluster_version(GLUSTERFS_VERSION_MIN) try: self._execute('mount.glusterfs', check_exit_code=False) except OSError as exc: diff --git a/manila/share/drivers/glusterfs_native.py b/manila/share/drivers/glusterfs_native.py index d653813c58..830e1bf9e3 100644 --- a/manila/share/drivers/glusterfs_native.py +++ b/manila/share/drivers/glusterfs_native.py @@ -102,6 +102,8 @@ SERVER_SSL = 'server.ssl' # Currently we handle only #{size}. PATTERN_DICT = {'size': {'pattern': '(?P\d+)', 'trans': int}} +GLUSTERFS_VERSION_MIN = (3, 6) + class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver): """GlusterFS native protocol (glusterfs) share driver. @@ -133,6 +135,7 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver): glusterfs_servers[srvaddr] = self._glustermanager( srvaddr, has_volume=False) self.glusterfs_servers = glusterfs_servers + self.glusterfs_versions = {} def _compile_volume_pattern(self): """Compile a RegexObject from the config specified regex template. @@ -159,6 +162,42 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver): # We don't use a service mount as its not necessary for us. # Do some sanity checks. + glusterfs_versions, exceptions = {}, {} + for srvaddr, gluster_mgr in six.iteritems(self.glusterfs_servers): + try: + glusterfs_versions[srvaddr] = gluster_mgr.get_gluster_version() + except exception.GlusterfsException as exc: + exceptions[srvaddr] = exc.message + if exceptions: + for srvaddr, excmsg in six.iteritems(exceptions): + LOG.error(_LE("'gluster version' failed on server " + "%(server)s with: %(message)s"), + {'server': srvaddr, 'message': excmsg}) + raise exception.GlusterfsException(_( + "'gluster version' failed on servers %s") % ( + ','.join(exceptions.keys()))) + notsupp_servers = [] + for srvaddr, vers in six.iteritems(glusterfs_versions): + if glusterfs.GlusterManager.numreduct( + vers) < GLUSTERFS_VERSION_MIN: + notsupp_servers.append(srvaddr) + if notsupp_servers: + gluster_version_min_str = '.'.join( + six.text_type(c) for c in GLUSTERFS_VERSION_MIN) + for srvaddr in notsupp_servers: + LOG.error(_LE("GlusterFS version %(version)s on server " + "%(server)s is not supported, " + "minimum requirement: %(minvers)s"), + {'server': srvaddr, + 'version': '.'.join(glusterfs_versions[srvaddr]), + 'minvers': gluster_version_min_str}) + raise exception.GlusterfsException(_( + "Unsupported GlusterFS version on servers %(servers)s, " + "minimum requirement: %(minvers)s") % { + 'servers': ','.join(notsupp_servers), + 'minvers': gluster_version_min_str}) + self.glusterfs_versions = glusterfs_versions + gluster_volumes_initial = set(self._fetch_gluster_volumes()) if not gluster_volumes_initial: # No suitable volumes are found on the Gluster end. @@ -606,14 +645,17 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver): operrno = int(outxml.find('opErrno').text) operrstr = outxml.find('opErrstr').text - if opret == -1 and operrno == 0: - self.gluster_nosnap_vols_dict[vol] = operrstr - msg = _("Share %(share_id)s does not support snapshots: " - "%(errstr)s.") % {'share_id': snapshot['share_id'], - 'errstr': operrstr} - LOG.error(msg) - raise exception.ShareSnapshotNotSupported(msg) - elif operrno: + if opret == -1: + vers = self.glusterfs_versions[vol] + if glusterfs.GlusterManager.numreduct(vers) > (3, 6): + # This logic has not yet been implemented in GlusterFS 3.6 + if operrno == 0: + self.gluster_nosnap_vols_dict[vol] = operrstr + msg = _("Share %(share_id)s does not support snapshots: " + "%(errstr)s.") % {'share_id': snapshot['share_id'], + 'errstr': operrstr} + LOG.error(msg) + raise exception.ShareSnapshotNotSupported(msg) raise exception.GlusterfsException( _("Creating snapshot for share %(share_id)s failed " "with %(errno)d: %(errstr)s") % { diff --git a/manila/tests/share/drivers/test_glusterfs.py b/manila/tests/share/drivers/test_glusterfs.py index 0eea3d5755..7eb4ca3abc 100644 --- a/manila/tests/share/drivers/test_glusterfs.py +++ b/manila/tests/share/drivers/test_glusterfs.py @@ -238,6 +238,57 @@ class GlusterManagerTestCase(test.TestCase): self.assertEqual('/foo(10.0.0.1|10.0.0.2),/bar(10.0.0.1)', ret) self._gluster_manager.gluster_call.assert_called_once_with(*args) + def test_get_gluster_version(self): + self.mock_object(self._gluster_manager, 'gluster_call', + mock.Mock(return_value=('glusterfs 3.6.2beta3', ''))) + ret = self._gluster_manager.get_gluster_version() + self.assertEqual(['3', '6', '2beta3'], ret) + self._gluster_manager.gluster_call.assert_called_once_with( + '--version') + + @ddt.data("foo 1.1.1", "glusterfs 3-6", "glusterfs 3.6beta3") + def test_get_gluster_version_exception(self, versinfo): + self.mock_object(self._gluster_manager, 'gluster_call', + mock.Mock(return_value=(versinfo, ''))) + self.assertRaises(exception.GlusterfsException, + self._gluster_manager.get_gluster_version) + self._gluster_manager.gluster_call.assert_called_once_with( + '--version') + + def test_get_gluster_version_process_error(self): + def raise_exception(*args, **kwargs): + raise exception.ProcessExecutionError() + + self.mock_object(self._gluster_manager, 'gluster_call', + mock.Mock(side_effect=raise_exception)) + self.assertRaises(exception.GlusterfsException, + self._gluster_manager.get_gluster_version) + self._gluster_manager.gluster_call.assert_called_once_with( + '--version') + + def test_check_gluster_version(self): + self.mock_object(self._gluster_manager, 'get_gluster_version', + mock.Mock(return_value=('3', '6'))) + + ret = self._gluster_manager.check_gluster_version((3, 5, 2)) + self.assertEqual(None, ret) + self._gluster_manager.get_gluster_version.assert_called_once_with() + + def test_check_gluster_version_unmet(self): + self.mock_object(self._gluster_manager, 'get_gluster_version', + mock.Mock(return_value=('3', '5', '2'))) + + self.assertRaises(exception.GlusterfsException, + self._gluster_manager.check_gluster_version, (3, 6)) + self._gluster_manager.get_gluster_version.assert_called_once_with() + + @ddt.data(('3', '6'), + ('3', '6', '2beta'), + ('3', '6', '2beta', '4')) + def test_numreduct(self, vers): + ret = glusterfs.GlusterManager.numreduct(vers) + self.assertEqual((3, 6), ret) + class GlusterfsShareDriverTestCase(test.TestCase): """Tests GlusterfsShareDriver.""" @@ -270,6 +321,8 @@ class GlusterfsShareDriverTestCase(test.TestCase): def test_do_setup(self): fake_gluster_manager = mock.Mock(**fake_gluster_manager_attrs) + self.mock_object(fake_gluster_manager, 'get_gluster_version', + mock.Mock(return_value=('3', '5'))) methods = ('_ensure_gluster_vol_mounted', '_setup_helpers') for method in methods: self.mock_object(self._driver, method) diff --git a/manila/tests/share/drivers/test_glusterfs_native.py b/manila/tests/share/drivers/test_glusterfs_native.py index abeba849f6..dbb9551f61 100644 --- a/manila/tests/share/drivers/test_glusterfs_native.py +++ b/manila/tests/share/drivers/test_glusterfs_native.py @@ -174,24 +174,53 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): self.assertTrue(glusterfs_native.LOG.error.called) def test_do_setup(self): + self._driver.glusterfs_servers = {self.glusterfs_server1: self.gmgr1} + self.mock_object(self.gmgr1, 'get_gluster_version', + mock.Mock(return_value=('3', '6'))) self.mock_object(self._driver, '_fetch_gluster_volumes', mock.Mock(return_value=self.glusterfs_volumes_dict)) self.mock_object(self._driver, '_update_gluster_vols_dict') self._driver.gluster_used_vols_dict = self.glusterfs_volumes_dict self.mock_object(glusterfs_native.LOG, 'warn') + expected_exec = ['mount.glusterfs'] self._driver.do_setup(self._context) self._driver._fetch_gluster_volumes.assert_called_once_with() self.assertEqual(expected_exec, fake_utils.fake_execute_get_log()) + self.gmgr1.get_gluster_version.assert_once_called_with() self._driver._update_gluster_vols_dict.assert_called_once_with( self._context) glusterfs_native.LOG.warn.assert_called_once_with(mock.ANY) + def test_do_setup_unsupported_glusterfs_version(self): + self._driver.glusterfs_servers = {self.glusterfs_server1: self.gmgr1} + self.mock_object(self.gmgr1, 'get_gluster_version', + mock.Mock(return_value=('3', '5'))) + + self.assertRaises(exception.GlusterfsException, + self._driver.do_setup, self._context) + self.gmgr1.get_gluster_version.assert_once_called_with() + + @ddt.data(exception.GlusterfsException, RuntimeError) + def test_do_setup_get_gluster_version_fails(self, exc): + def raise_exception(*args, **kwargs): + raise exc + + self._driver.glusterfs_servers = {self.glusterfs_server1: self.gmgr1} + self.mock_object(self.gmgr1, 'get_gluster_version', + mock.Mock(side_effect=raise_exception)) + self.assertRaises(exc, self._driver.do_setup, self._context) + self.gmgr1.get_gluster_version.assert_once_called_with() + def test_do_setup_glusterfs_no_volumes_provided_by_backend(self): + self._driver.glusterfs_servers = {self.glusterfs_server1: self.gmgr1} + self.mock_object(self.gmgr1, 'get_gluster_version', + mock.Mock(return_value=('3', '6'))) self.mock_object(self._driver, '_fetch_gluster_volumes', mock.Mock(return_value={})) + self.assertRaises(exception.GlusterfsException, self._driver.do_setup, self._context) self._driver._fetch_gluster_volumes.assert_called_once_with() @@ -748,6 +777,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): def test_create_snapshot(self): self._db.share_get = mock.Mock(return_value=self.share1) self._driver.gluster_nosnap_vols_dict = {} + self._driver.glusterfs_versions = {self.glusterfs_target1: ('3', '6')} gmgr = glusterfs.GlusterManager gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None) @@ -766,6 +796,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): def test_create_snapshot_error(self): self._db.share_get = mock.Mock(return_value=self.share1) self._driver.gluster_nosnap_vols_dict = {} + self._driver.glusterfs_versions = {self.glusterfs_target1: ('3', '6')} gmgr = glusterfs.GlusterManager gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None) @@ -782,9 +813,15 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): snapshot) gmgr1.gluster_call.assert_called_once_with(*args) - def test_create_snapshot_no_snap(self): + @ddt.data({"vers_minor": '6', "exctype": exception.GlusterfsException}, + {"vers_minor": '7', + "exctype": exception.ShareSnapshotNotSupported}) + @ddt.unpack + def test_create_snapshot_no_snap(self, vers_minor, exctype): self._db.share_get = mock.Mock(return_value=self.share1) self._driver.gluster_nosnap_vols_dict = {} + self._driver.glusterfs_versions = { + self.glusterfs_target1: ('3', vers_minor)} gmgr = glusterfs.GlusterManager gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None) @@ -796,20 +833,24 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): gmgr1.volume) self.mock_object(gmgr1, 'gluster_call', mock.Mock(side_effect=GlusterXMLOut(ret=-1, errno=0))) - self.assertRaises(exception.ShareSnapshotNotSupported, - self._driver.create_snapshot, self._context, + self.assertRaises(exctype, self._driver.create_snapshot, self._context, snapshot) gmgr1.gluster_call.assert_called_once_with(*args) - def test_create_snapshot_no_snap_cached(self): + @ddt.data({"vers_minor": '6', "exctype": exception.GlusterfsException}, + {"vers_minor": '7', + "exctype": exception.ShareSnapshotNotSupported}) + @ddt.unpack + def test_create_snapshot_no_snap_cached(self, vers_minor, exctype): self._db.share_get = mock.Mock(return_value=self.share1) self._driver.gluster_nosnap_vols_dict = { self.share1['export_location']: 'fake error'} + self._driver.glusterfs_versions = { + self.glusterfs_target1: ('3', vers_minor)} snapshot = {'id': 'fake_snap_id', 'share_id': self.share1['id']} - self.assertRaises(exception.ShareSnapshotNotSupported, - self._driver.create_snapshot, self._context, + self.assertRaises(exctype, self._driver.create_snapshot, self._context, snapshot) def test_delete_snapshot(self):