diff --git a/manila/share/drivers/glusterfs/__init__.py b/manila/share/drivers/glusterfs/__init__.py index 8134d2f02d..8d5c0efa68 100644 --- a/manila/share/drivers/glusterfs/__init__.py +++ b/manila/share/drivers/glusterfs/__init__.py @@ -110,13 +110,10 @@ class GlusterfsShareDriver(driver.ExecuteMixin, driver.GaneshaMixin, # to not to defeat access control setting = [NFS_EXPORT_VOL, 'off'] args = ['volume', 'set', gluster_manager.volume] + setting - try: - gluster_manager.gluster_call(*args) - except exception.ProcessExecutionError as exc: - LOG.error(_LE("Error in tuning GlusterFS volume to prevent " - "exporting the entire volume: %s"), exc.stderr) - raise exception.GlusterfsException("gluster %s failed" % - ' '.join(args)) + gluster_manager.gluster_call( + *args, + log=_LE("Tuning the GlusterFS volume to prevent exporting the " + "entire volume")) return self.nfs_helper(self._execute, self.configuration, gluster_manager=gluster_manager).get_export( share_manager['share']) @@ -227,11 +224,9 @@ class GlusterNFSHelper(ganesha.NASHelperBase): else: args = ('volume', 'reset', self.gluster_manager.volume, NFS_EXPORT_DIR) - try: - self.gluster_manager.gluster_call(*args) - except exception.ProcessExecutionError as exc: - LOG.error(_LE("Error in gluster volume set: %s"), exc.stderr) - raise + self.gluster_manager.gluster_call( + *args, + log=_LE("Tuning GlusterFS volume options")) def allow_access(self, base, share, access): """Allow access to a share.""" @@ -307,12 +302,10 @@ class GlusterNFSVolHelper(GlusterNFSHelper): NFS_RPC_AUTH_ALLOW), ('volume', 'set', self.gluster_manager.volume, NFS_RPC_AUTH_REJECT, '*')) - try: - for args in argseq: - self.gluster_manager.gluster_call(*args) - except exception.ProcessExecutionError as exc: - LOG.error(_LE("Error in gluster volume set: %s"), exc.stderr) - raise + for args in argseq: + self.gluster_manager.gluster_call( + *args, + log=_LE("Tuning GlusterFS volume options")) def allow_access(self, base, share, access): """Allow access to a share.""" diff --git a/manila/share/drivers/glusterfs/common.py b/manila/share/drivers/glusterfs/common.py index 97310a89c2..8f021b1d17 100644 --- a/manila/share/drivers/glusterfs/common.py +++ b/manila/share/drivers/glusterfs/common.py @@ -133,18 +133,30 @@ class GlusterManager(object): privatekey=self.path_to_private_key) else: gluster_execf = ganesha_utils.RootExecutor(execf) - return lambda *args, **kwargs: gluster_execf(*(('gluster',) + args), - **kwargs) + + def _gluster_call(*args, **kwargs): + logmsg = kwargs.pop('log', None) + raw_error = kwargs.pop('raw_error', False) + try: + return gluster_execf(*(('gluster',) + args), **kwargs) + except exception.ProcessExecutionError as exc: + if raw_error: + raise + if logmsg: + LOG.error(_LE("%s: GlusterFS instrumentation failed.") % + logmsg) + raise exception.GlusterfsException( + _("GlusterFS management command '%(cmd)s' failed " + "with details as follows:\n%(details)s.") % { + 'cmd': ' '.join(args), + 'details': exc.args[0]}) + + return _gluster_call def get_gluster_vol_option(self, option): """Get the value of an option set on a GlusterFS volume.""" args = ('--xml', 'volume', 'info', self.volume) - try: - out, err = self.gluster_call(*args) - except exception.ProcessExecutionError as exc: - LOG.error(_LE("Error retrieving volume info: %s"), exc.stderr) - raise exception.GlusterfsException("gluster %s failed" % - ' '.join(args)) + out, err = self.gluster_call(*args, log=_LE("retrieving volume info")) if not out: raise exception.GlusterfsException( @@ -165,13 +177,8 @@ class GlusterManager(object): :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': six.text_type(exc)}) + out, err = self.gluster_call('--version', + log=_LE("GlusterFS version query")) try: owords = out.split() if owords[0] != 'glusterfs': @@ -262,28 +269,16 @@ def _restart_gluster_vol(gluster_mgr): :param gluster_mgr: GlusterManager instance """ - try: - # TODO(csaba): '--mode=script' ensures that the Gluster CLI runs in - # script mode. This seems unnecessary as the Gluster CLI is - # expected to run in non-interactive mode when the stdin is not - # a terminal, as is the case below. But on testing, found the - # behaviour of Gluster-CLI to be the contrary. Need to investigate - # this odd-behaviour of Gluster-CLI. - gluster_mgr.gluster_call( - 'volume', 'stop', gluster_mgr.volume, '--mode=script') - except exception.ProcessExecutionError as exc: - msg = (_("Error stopping gluster volume. " - "Volume: %(volname)s, Error: %(error)s") % - {'volname': gluster_mgr.volume, 'error': exc.stderr}) - LOG.error(msg) - raise exception.GlusterfsException(msg) + # TODO(csaba): '--mode=script' ensures that the Gluster CLI runs in + # script mode. This seems unnecessary as the Gluster CLI is + # expected to run in non-interactive mode when the stdin is not + # a terminal, as is the case below. But on testing, found the + # behaviour of Gluster-CLI to be the contrary. Need to investigate + # this odd-behaviour of Gluster-CLI. + gluster_mgr.gluster_call( + 'volume', 'stop', gluster_mgr.volume, '--mode=script', + log=_LE("stopping GlusterFS volume %s") % gluster_mgr.volume) - try: - gluster_mgr.gluster_call( - 'volume', 'start', gluster_mgr.volume) - except exception.ProcessExecutionError as exc: - msg = (_("Error starting gluster volume. " - "Volume: %(volname)s, Error: %(error)s") % - {'volname': gluster_mgr.volume, 'error': exc.stderr}) - LOG.error(msg) - raise exception.GlusterfsException(msg) + gluster_mgr.gluster_call( + 'volume', 'start', gluster_mgr.volume, + log=_LE("starting GlusterFS volume %s") % gluster_mgr.volume) diff --git a/manila/share/drivers/glusterfs/layout_directory.py b/manila/share/drivers/glusterfs/layout_directory.py index 5936eb63ea..64bdc55810 100644 --- a/manila/share/drivers/glusterfs/layout_directory.py +++ b/manila/share/drivers/glusterfs/layout_directory.py @@ -80,13 +80,12 @@ class GlusterfsDirectoryMappedLayout(layout.GlusterfsShareLayoutBase): args = ('volume', 'quota', self.gluster_manager.volume, 'enable') try: self.gluster_manager.gluster_call(*args) - except exception.ProcessExecutionError as exc: + except exception.GlusterfsException: if (self.gluster_manager. get_gluster_vol_option('features.quota')) != 'on': LOG.error(_LE("Error in tuning GlusterFS volume to enable " - "creation of shares of specific size: %s"), - exc.stderr) - raise exception.GlusterfsException(exc) + "creation of shares of specific size.")) + raise self._ensure_gluster_vol_mounted() @@ -149,10 +148,13 @@ class GlusterfsDirectoryMappedLayout(layout.GlusterfsShareLayoutBase): try: self.driver._execute(*cmd, run_as_root=True) self.gluster_manager.gluster_call(*args) - except exception.ProcessExecutionError as exc: - self._cleanup_create_share(local_share_path, share['name']) - LOG.error(_LE('Unable to create share %s'), share['name']) - raise exception.GlusterfsException(exc) + except Exception as exc: + if isinstance(exc, exception.ProcessExecutionError): + exc = exception.GlusterfsException(exc) + if isinstance(exc, exception.GlusterfsException): + self._cleanup_create_share(local_share_path, share['name']) + LOG.error(_LE('Unable to create share %s'), share['name']) + raise exc comp_share = self.gluster_manager.components.copy() comp_share['path'] = '/' + share['name'] diff --git a/manila/share/drivers/glusterfs/layout_volume.py b/manila/share/drivers/glusterfs/layout_volume.py index adbff2cb22..23f96dd06f 100644 --- a/manila/share/drivers/glusterfs/layout_volume.py +++ b/manila/share/drivers/glusterfs/layout_volume.py @@ -204,16 +204,12 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase): volumes_dict = {} for srvaddr in self.configuration.glusterfs_servers: gluster_mgr = self._glustermanager(srvaddr, False) - try: - out, err = gluster_mgr.gluster_call('volume', 'list') - except exception.ProcessExecutionError as exc: - msgdict = {'err': exc.stderr, 'hostinfo': ''} - if gluster_mgr.user: - msgdict['hostinfo'] = ' on host %s' % gluster_mgr.host - LOG.error(_LE("Error retrieving volume list%(hostinfo)s: " - "%(err)s") % msgdict) - raise exception.GlusterfsException( - _('gluster volume list failed')) + if gluster_mgr.user: + logmsg = _LE("Retrieving volume list " + "on host %s") % gluster_mgr.host + else: + logmsg = _LE("Retrieving volume list") + out, err = gluster_mgr.gluster_call('volume', 'list', log=logmsg) for volname in out.split("\n"): patmatch = self.volume_pattern.match(volname) if not patmatch: @@ -403,12 +399,7 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase): self.private_storage.update(share['id'], {'volume': vol}) args = ('volume', 'set', gmgr.volume, USER_MANILA_SHARE, share['id']) - try: - gmgr.gluster_call(*args) - except exception.ProcessExecutionError: - raise exception.GlusterfsException( - _("gluster %(cmd)s failed on %(vol)s") % - {'cmd': ' '.join(args), 'vol': gmgr.qualified}) + gmgr.gluster_call(*args) # TODO(deepakcs): Enable quota and set it to the share size. @@ -438,13 +429,7 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase): self._wipe_gluster_vol(gmgr) args = ('volume', 'set', gmgr.volume, USER_MANILA_SHARE, 'NONE') - try: - gmgr.gluster_call(*args) - except exception.ProcessExecutionError as exc: - LOG.error(_LE("Gluster command failed: %s"), exc.stderr) - raise exception.GlusterfsException( - _("gluster %(cmd)s failed on %(vol)s") % - {'cmd': ' '.join(args), 'vol': gmgr.qualified}) + gmgr.gluster_call(*args) self._push_gluster_vol(gmgr.qualified) except exception.GlusterfsException: @@ -459,12 +444,9 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase): @staticmethod def _find_actual_backend_snapshot_name(gluster_mgr, snapshot): args = ('snapshot', 'list', gluster_mgr.volume, '--mode=script') - try: - out, err = gluster_mgr.gluster_call(*args) - except exception.ProcessExecutionError as exc: - LOG.error(_LE("Error retrieving snapshot list: %s"), exc.stderr) - raise exception.GlusterfsException(_("gluster %s failed") % - ' '.join(args)) + out, err = gluster_mgr.gluster_call( + *args, + log=_LE("Retrieving snapshot list")) snapgrep = list(filter(lambda x: snapshot['id'] in x, out.split("\n"))) if len(snapgrep) != 1: msg = (_("Failed to identify backing GlusterFS object " @@ -505,17 +487,10 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase): args_tuple = (('snapshot', 'activate', backend_snapshot_name, 'force', '--mode=script'), ('snapshot', 'clone', volume, backend_snapshot_name)) - try: - for args in args_tuple: - out, err = old_gmgr.gluster_call(*args) - except exception.ProcessExecutionError as exc: - LOG.error(_LE("Error creating share from snapshot " - "%(snap)s of share %(share)s: %(err)s"), - {'snap': snapshot['id'], - 'share': snapshot['share_instance']['id'], - 'err': exc.stderr}) - raise exception.GlusterfsException(_("gluster %s failed") % - ' '.join(args)) + for args in args_tuple: + out, err = old_gmgr.gluster_call( + *args, + log=_LE("Creating share from snapshot")) # Get a manager for the the new volume/share. comp_vol = old_gmgr.components.copy() @@ -531,16 +506,7 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase): ('start', [])) for op, opargs in argseq: args = ['volume', op, gmgr.volume] + opargs - try: - gmgr.gluster_call(*args) - except exception.ProcessExecutionError as exc: - LOG.error(_LE("Error creating share from snapshot " - "%(snap)s of share %(share)s: %(err)s."), - {'snap': snapshot['id'], - 'share': snapshot['share_instance']['id'], - 'err': exc.stderr}) - raise exception.GlusterfsException(_("gluster %s failed.") % - ' '.join(args)) + gmgr.gluster_call(*args, log=_LE("Creating share from snapshot")) self.gluster_used_vols.add(gmgr.qualified) self.private_storage.update(share['id'], {'volume': gmgr.qualified}) @@ -557,12 +523,9 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase): else: args = ('--xml', 'snapshot', 'create', 'manila-' + snapshot['id'], gluster_mgr.volume) - try: - out, err = gluster_mgr.gluster_call(*args) - except exception.ProcessExecutionError as exc: - LOG.error(_LE("Error retrieving volume info: %s"), exc.stderr) - raise exception.GlusterfsException("gluster %s failed" % - ' '.join(args)) + out, err = gluster_mgr.gluster_call( + *args, + log=_LE("Retrieving volume info")) if not out: raise exception.GlusterfsException( @@ -602,12 +565,9 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase): gluster_mgr, snapshot) args = ('--xml', 'snapshot', 'delete', backend_snapshot_name, '--mode=script') - try: - out, err = gluster_mgr.gluster_call(*args) - except exception.ProcessExecutionError as exc: - LOG.error(_LE("Error deleting snapshot: %s"), exc.stderr) - raise exception.GlusterfsException(_("gluster %s failed") % - ' '.join(args)) + out, err = gluster_mgr.gluster_call( + *args, + log=_LE("Error deleting snapshot")) if not out: raise exception.GlusterfsException( @@ -635,12 +595,7 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase): self.gluster_used_vols.add(gmgr.qualified) args = ('volume', 'set', gmgr.volume, USER_MANILA_SHARE, share['id']) - try: - gmgr.gluster_call(*args) - except exception.ProcessExecutionError: - raise exception.GlusterfsException( - _("gluster %(cmd)s failed on %(vol)s") % - {'cmd': ' '.join(args), 'vol': gmgr.qualified}) + gmgr.gluster_call(*args) # Debt... diff --git a/manila/share/drivers/glusterfs_native.py b/manila/share/drivers/glusterfs_native.py index beaf9c945d..3e8b3bd933 100644 --- a/manila/share/drivers/glusterfs_native.py +++ b/manila/share/drivers/glusterfs_native.py @@ -31,6 +31,7 @@ from oslo_log import log from manila import exception from manila.i18n import _ +from manila.i18n import _LE from manila.i18n import _LW from manila.share import driver from manila.share.drivers.glusterfs import common @@ -123,29 +124,33 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, gluster_actions.append((AUTH_SSL_ALLOW, access_to)) for option, value in ( - (NFS_EXPORT_VOL, 'off'), (CLIENT_SSL, 'on'), (SERVER_SSL, 'on'), - (DYNAMIC_AUTH, 'on') + (NFS_EXPORT_VOL, 'off'), (CLIENT_SSL, 'on'), (SERVER_SSL, 'on') ): gluster_actions.append((option, value)) for action in gluster_actions: - try: - gluster_mgr.gluster_call( - 'volume', 'set', gluster_mgr.volume, *action) - except exception.ProcessExecutionError as exc: - if action[0] == DYNAMIC_AUTH and exc.exit_code == 1: - # 'dynamic-auth' is not supported by gluster backend, - # that's OK - pass - else: - msg = (_("Error in gluster volume set during volume " - "setup. volume: %(volname)s, option: %(option)s, " - "value: %(value)s, error: %(error)s") % - {'volname': gluster_mgr.volume, - 'option': option, 'value': value, - 'error': exc.stderr}) - LOG.error(msg) - raise exception.GlusterfsException(msg) + gluster_mgr.gluster_call( + 'volume', 'set', gluster_mgr.volume, *action, + log=_LE('Setting up GlusterFS volume')) + + try: + gluster_mgr.gluster_call( + 'volume', 'set', gluster_mgr.volume, DYNAMIC_AUTH, 'on', + raw_error=True) + except exception.ProcessExecutionError as exc: + if exc.exit_code == 1: + # 'dynamic-auth' is not supported by gluster backend, + # that's OK + pass + else: + msg = (_("Error in gluster volume set during volume " + "setup. volume: %(volname)s, option: %(option)s, " + "value: %(value)s, error: %(error)s") % + {'volname': gluster_mgr.volume, + 'option': option, 'value': value, + 'error': exc.stderr}) + LOG.error(msg) + raise exception.GlusterfsException(msg) # SSL enablement requires a fresh volume start # to take effect @@ -188,19 +193,10 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, ssl_allow.append(access_to) ssl_allow_opt = ','.join(ssl_allow) - try: - gluster_mgr.gluster_call( - 'volume', 'set', gluster_mgr.volume, - AUTH_SSL_ALLOW, ssl_allow_opt) - except exception.ProcessExecutionError as exc: - msg = (_("Error in gluster volume set during allow access. " - "Volume: %(volname)s, Option: %(option)s, " - "access_to: %(access_to)s, Error: %(error)s") % - {'volname': gluster_mgr.volume, - 'option': AUTH_SSL_ALLOW, 'access_to': access_to, - 'error': exc.stderr}) - LOG.error(msg) - raise exception.GlusterfsException(msg) + gluster_mgr.gluster_call( + 'volume', 'set', gluster_mgr.volume, + AUTH_SSL_ALLOW, ssl_allow_opt, + log=_LE("Tuning GlusterFS volume in allow-access")) @utils.synchronized("glusterfs_native_access", external=False) def _deny_access_via_manager(self, gluster_mgr, context, share, access, @@ -228,30 +224,12 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, ssl_allow.remove(access_to) ssl_allow_opt = ','.join(ssl_allow) - try: - gluster_mgr.gluster_call( - 'volume', 'set', gluster_mgr.volume, - AUTH_SSL_ALLOW, ssl_allow_opt) - except exception.ProcessExecutionError as exc: - msg = (_("Error in gluster volume set during deny access. " - "Volume: %(volname)s, Option: %(option)s, " - "access_to: %(access_to)s, Error: %(error)s") % - {'volname': gluster_mgr.volume, - 'option': AUTH_SSL_ALLOW, 'access_to': access_to, - 'error': exc.stderr}) - LOG.error(msg) - raise exception.GlusterfsException(msg) + gluster_mgr.gluster_call( + 'volume', 'set', gluster_mgr.volume, + AUTH_SSL_ALLOW, ssl_allow_opt, + log=_LE("Tuning GlusterFS volume in deny-access")) - try: - dynauth = gluster_mgr.get_gluster_vol_option(DYNAMIC_AUTH) or 'off' - except exception.ProcessExecutionError as exc: - msg = (_("Error in querying gluster volume. " - "Volume: %(volname)s, Option: %(option)s, " - "Error: %(error)s") % - {'volname': gluster_mgr.volume, - 'option': DYNAMIC_AUTH, 'error': exc.stderr}) - LOG.error(msg) - raise exception.GlusterfsException(msg) + dynauth = gluster_mgr.get_gluster_vol_option(DYNAMIC_AUTH) or 'off' # TODO(csaba): boolean option processing shoud be done in common if dynauth.lower() not in ('on', '1', 'true', 'yes', 'enable'): common._restart_gluster_vol(gluster_mgr) diff --git a/manila/tests/share/drivers/glusterfs/test_common.py b/manila/tests/share/drivers/glusterfs/test_common.py index b61453be69..471965f5c6 100644 --- a/manila/tests/share/drivers/glusterfs/test_common.py +++ b/manila/tests/share/drivers/glusterfs/test_common.py @@ -159,6 +159,36 @@ class GlusterManagerTestCase(test.TestCase): fake_obj.assert_called_once_with( *(('gluster',) + fake_args), **fake_kwargs) + @ddt.data({'trouble': exception.ProcessExecutionError, + '_exception': exception.GlusterfsException, 'xkw': {}}, + {'trouble': exception.ProcessExecutionError, + '_exception': exception.GlusterfsException, + 'xkw': {'raw_error': False}}, + {'trouble': exception.ProcessExecutionError, + '_exception': exception.ProcessExecutionError, + 'xkw': {'raw_error': True}}, + {'trouble': RuntimeError, '_exception': RuntimeError, 'xkw': {}}) + @ddt.unpack + def test_gluster_manager_make_gluster_call_error(self, trouble, + _exception, xkw): + fake_obj = mock.Mock(side_effect=trouble) + fake_execute = mock.Mock() + kwargs = fake_kwargs.copy() + kwargs.update(xkw) + with mock.patch.object(common.ganesha_utils, 'RootExecutor', + mock.Mock(return_value=fake_obj)): + gluster_manager = common.GlusterManager( + '127.0.0.1:/testvol', self.fake_execf) + + self.assertRaises(_exception, + gluster_manager.make_gluster_call(fake_execute), + *fake_args, **kwargs) + + common.ganesha_utils.RootExecutor.assert_called_with( + fake_execute) + fake_obj.assert_called_once_with( + *(('gluster',) + fake_args), **fake_kwargs) + def test_get_gluster_vol_option_empty_volinfo(self): args = ('--xml', 'volume', 'info', self._gluster_manager.volume) self.mock_object(self._gluster_manager, 'gluster_call', @@ -167,26 +197,7 @@ class GlusterManagerTestCase(test.TestCase): self._gluster_manager.get_gluster_vol_option, NFS_EXPORT_DIR) self._gluster_manager.gluster_call.assert_called_once_with( - *args) - - @ddt.data({'inner_excp': RuntimeError, 'outer_excp': RuntimeError}, - {'inner_excp': exception.ProcessExecutionError, - 'outer_excp': exception.GlusterfsException}) - @ddt.unpack - def test_get_gluster_vol_option_failing_volinfo(self, inner_excp, - outer_excp): - - def raise_exception(*ignore_args, **ignore_kwargs): - raise inner_excp('fake error') - - args = ('--xml', 'volume', 'info', self._gluster_manager.volume) - self.mock_object(self._gluster_manager, 'gluster_call', - mock.Mock(side_effect=raise_exception)) - self.assertRaises(outer_excp, - self._gluster_manager.get_gluster_vol_option, - NFS_EXPORT_DIR) - self._gluster_manager.gluster_call.assert_called_once_with( - *args) + *args, log=mock.ANY) def test_get_gluster_vol_option_ambiguous_volinfo(self): @@ -206,7 +217,8 @@ class GlusterManagerTestCase(test.TestCase): self.assertRaises(exception.InvalidShare, self._gluster_manager.get_gluster_vol_option, NFS_EXPORT_DIR) - self._gluster_manager.gluster_call.assert_called_once_with(*args) + self._gluster_manager.gluster_call.assert_called_once_with( + *args, log=mock.ANY) def test_get_gluster_vol_option_trivial_volinfo(self): @@ -227,7 +239,8 @@ class GlusterManagerTestCase(test.TestCase): mock.Mock(side_effect=xml_output)) ret = self._gluster_manager.get_gluster_vol_option(NFS_EXPORT_DIR) self.assertIsNone(ret) - self._gluster_manager.gluster_call.assert_called_once_with(*args) + self._gluster_manager.gluster_call.assert_called_once_with( + *args, log=mock.ANY) def test_get_gluster_vol_option(self): @@ -254,7 +267,8 @@ class GlusterManagerTestCase(test.TestCase): mock.Mock(side_effect=xml_output)) ret = self._gluster_manager.get_gluster_vol_option(NFS_EXPORT_DIR) 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) + self._gluster_manager.gluster_call.assert_called_once_with( + *args, log=mock.ANY) def test_get_gluster_version(self): self.mock_object(self._gluster_manager, 'gluster_call', @@ -262,7 +276,7 @@ class GlusterManagerTestCase(test.TestCase): ret = self._gluster_manager.get_gluster_version() self.assertEqual(['3', '6', '2beta3'], ret) self._gluster_manager.gluster_call.assert_called_once_with( - '--version') + '--version', log=mock.ANY) @ddt.data("foo 1.1.1", "glusterfs 3-6", "glusterfs 3.6beta3") def test_get_gluster_version_exception(self, versinfo): @@ -271,18 +285,7 @@ class GlusterManagerTestCase(test.TestCase): 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') + '--version', log=mock.ANY) def test_check_gluster_version(self): self.mock_object(self._gluster_manager, 'get_gluster_version', @@ -387,45 +390,11 @@ class GlusterFSCommonTestCase(test.TestCase): def test_restart_gluster_vol(self): gmgr = common.GlusterManager(fakeexport, self._execute, None, None) - test_args = [('volume', 'stop', fakevol, '--mode=script'), - ('volume', 'start', fakevol)] + test_args = [(('volume', 'stop', fakevol, '--mode=script'), + {'log': mock.ANY}), + (('volume', 'start', fakevol), {'log': mock.ANY})] common._restart_gluster_vol(gmgr) self.assertEqual( - [mock.call(*test_args[0]), mock.call(*test_args[1])], - gmgr.gluster_call.call_args_list) - - def test_restart_gluster_vol_excp1(self): - gmgr = common.GlusterManager(fakeexport, self._execute, None, None) - test_args = ('volume', 'stop', fakevol, '--mode=script') - - def raise_exception(*args, **kwargs): - if(args == test_args): - raise exception.ProcessExecutionError() - - self.mock_object(gmgr, 'gluster_call', - mock.Mock(side_effect=raise_exception)) - - self.assertRaises(exception.GlusterfsException, - common._restart_gluster_vol, gmgr) - self.assertEqual( - [mock.call(*test_args)], - gmgr.gluster_call.call_args_list) - - def test_restart_gluster_vol_excp2(self): - gmgr = common.GlusterManager(fakeexport, self._execute, None, None) - test_args = [('volume', 'stop', fakevol, '--mode=script'), - ('volume', 'start', fakevol)] - - def raise_exception(*args, **kwargs): - if(args == test_args[1]): - raise exception.ProcessExecutionError() - - self.mock_object(gmgr, 'gluster_call', - mock.Mock(side_effect=raise_exception)) - - self.assertRaises(exception.GlusterfsException, - common._restart_gluster_vol, gmgr) - self.assertEqual( - [mock.call(*test_args[0]), mock.call(*test_args[1])], + [mock.call(*arg[0], **arg[1]) for arg in test_args], gmgr.gluster_call.call_args_list) diff --git a/manila/tests/share/drivers/glusterfs/test_layout_directory.py b/manila/tests/share/drivers/glusterfs/test_layout_directory.py index a308969eb8..3a5e2eaf52 100644 --- a/manila/tests/share/drivers/glusterfs/test_layout_directory.py +++ b/manila/tests/share/drivers/glusterfs/test_layout_directory.py @@ -110,7 +110,7 @@ class GlusterfsDirectoryMappedLayoutTestCase(test.TestCase): def test_do_setup_error_enabling_creation_share_specific_size(self): attrs = {'volume': 'testvol', - 'gluster_call.side_effect': exception.ProcessExecutionError, + 'gluster_call.side_effect': exception.GlusterfsException, 'get_gluster_vol_option.return_value': 'off'} fake_gluster_manager = mock.Mock(**attrs) self.mock_object(layout_directory.LOG, 'error') @@ -133,13 +133,13 @@ class GlusterfsDirectoryMappedLayoutTestCase(test.TestCase): 'volume', 'quota', 'testvol', 'enable') (self._layout.gluster_manager.get_gluster_vol_option. assert_called_once_with('features.quota')) - layout_directory.LOG.error.assert_called_once_with(mock.ANY, mock.ANY) + layout_directory.LOG.error.assert_called_once_with(mock.ANY) self._layout._check_mount_glusterfs.assert_called_once_with() self.assertFalse(self._layout._ensure_gluster_vol_mounted.called) def test_do_setup_error_already_enabled_creation_share_specific_size(self): attrs = {'volume': 'testvol', - 'gluster_call.side_effect': exception.ProcessExecutionError, + 'gluster_call.side_effect': exception.GlusterfsException, 'get_gluster_vol_option.return_value': 'on'} fake_gluster_manager = mock.Mock(**attrs) self.mock_object(layout_directory.LOG, 'error') @@ -260,9 +260,10 @@ class GlusterfsDirectoryMappedLayoutTestCase(test.TestCase): {'share': self.share, 'manager': gmgr}) self.assertEqual(expected_ret, ret) - def test_create_share_unable_to_create_share(self): + @ddt.data(exception.ProcessExecutionError, exception.GlusterfsException) + def test_create_share_unable_to_create_share(self, trouble): def exec_runner(*ignore_args, **ignore_kw): - raise exception.ProcessExecutionError + raise trouble self.mock_object( self._layout, '_get_local_share_path', @@ -283,6 +284,26 @@ class GlusterfsDirectoryMappedLayoutTestCase(test.TestCase): layout_directory.LOG.error.assert_called_once_with( mock.ANY, mock.ANY) + def test_create_share_unable_to_create_share_weird(self): + def exec_runner(*ignore_args, **ignore_kw): + raise RuntimeError + + self.mock_object( + self._layout, '_get_local_share_path', + mock.Mock(return_value=fake_local_share_path)) + self.mock_object(self._layout, '_cleanup_create_share') + self.mock_object(layout_directory.LOG, 'error') + expected_exec = ['mkdir %s' % fake_local_share_path] + fake_utils.fake_execute_set_repliers([(expected_exec[0], + exec_runner)]) + + self.assertRaises( + RuntimeError, self._layout.create_share, + self._context, self.share) + + self._layout._get_local_share_path.called_once_with(self.share) + self.assertFalse(self._layout._cleanup_create_share.called) + def test_cleanup_create_share_local_share_path_exists(self): expected_exec = ['rm -rf %s' % fake_local_share_path] self.mock_object(os.path, 'exists', mock.Mock(return_value=True)) diff --git a/manila/tests/share/drivers/glusterfs/test_layout_volume.py b/manila/tests/share/drivers/glusterfs/test_layout_volume.py index 2d527d72aa..bfcbad2a16 100644 --- a/manila/tests/share/drivers/glusterfs/test_layout_volume.py +++ b/manila/tests/share/drivers/glusterfs/test_layout_volume.py @@ -192,8 +192,10 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): ret = self._layout._fetch_gluster_volumes() test_args = ('volume', 'list') - self.gmgr1.gluster_call.assert_called_once_with(*test_args) - self.gmgr2.gluster_call.assert_called_once_with(*test_args) + self.gmgr1.gluster_call.assert_called_once_with(*test_args, + log=mock.ANY) + self.gmgr2.gluster_call.assert_called_once_with(*test_args, + log=mock.ANY) gmgr_vol1.get_gluster_vol_option.assert_called_once_with( 'user.manila-share') gmgr_vol2.get_gluster_vol_option.assert_called_once_with( @@ -221,8 +223,10 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): ret = self._layout._fetch_gluster_volumes(filter_used=False) test_args = ('volume', 'list') - self.gmgr1.gluster_call.assert_called_once_with(*test_args) - self.gmgr2.gluster_call.assert_called_once_with(*test_args) + self.gmgr1.gluster_call.assert_called_once_with(*test_args, + log=mock.ANY) + self.gmgr2.gluster_call.assert_called_once_with(*test_args, + log=mock.ANY) self.assertFalse(gmgr_vol1.get_gluster_vol_option.called) self.assertFalse(gmgr_vol2.get_gluster_vol_option.called) self.assertEqual(expected_output, ret) @@ -245,7 +249,8 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): ret = self._layout._fetch_gluster_volumes() test_args = ('volume', 'list') - self.gmgr1.gluster_call.assert_called_once_with(*test_args) + self.gmgr1.gluster_call.assert_called_once_with(*test_args, + log=mock.ANY) self.assertEqual(expected_output, ret) def test_fetch_gluster_volumes_error(self): @@ -253,7 +258,7 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): def raise_exception(*args, **kwargs): if(args == test_args): - raise exception.ProcessExecutionError() + raise exception.GlusterfsException() self._layout.configuration.glusterfs_servers = [self.glusterfs_server1] self.mock_object(self.gmgr1, 'gluster_call', @@ -265,8 +270,8 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): self.assertRaises(exception.GlusterfsException, self._layout._fetch_gluster_volumes) - self.gmgr1.gluster_call.assert_called_once_with(*test_args) - self.assertTrue(layout_volume.LOG.error.called) + self.gmgr1.gluster_call.assert_called_once_with(*test_args, + log=mock.ANY) def test_do_setup(self): self._layout.configuration.glusterfs_servers = [self.glusterfs_server1] @@ -357,26 +362,6 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): gmgr1.gluster_call.assert_called_once_with( 'volume', 'set', 'gv1', 'user.manila-share', share['id']) - @ddt.data({'trouble': exception.ProcessExecutionError, - '_exception': exception.GlusterfsException}, - {'trouble': RuntimeError, '_exception': RuntimeError}) - @ddt.unpack - def test_ensure_share_gluster_call_error(self, trouble, _exception): - share = self.share1 - gmgr1 = common.GlusterManager(self.glusterfs_target1, self._execute, - None, None) - gmgr1.gluster_call = mock.Mock(side_effect=trouble) - self.mock_object(self._layout, '_share_manager', - mock.Mock(return_value=gmgr1)) - - self.assertRaises(_exception, self._layout.ensure_share, - self._context, share) - - self._layout._share_manager.assert_called_once_with(share) - self.assertIn(self.glusterfs_target1, self._layout.gluster_used_vols) - gmgr1.gluster_call.assert_called_once_with( - 'volume', 'set', 'gv1', 'user.manila-share', share['id']) - @ddt.data({"voldict": {"host:/share2G": {"size": 2}}, "used_vols": set(), "size": 1, "expected": "host:/share2G"}, {"voldict": {"host:/share2G": {"size": 2}}, "used_vols": set(), @@ -574,32 +559,6 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): 'volume', 'set', 'gv1', 'user.manila-share', share['id']) self.assertEqual('host1:/gv1', exp_locn) - @ddt.data({'trouble': exception.ProcessExecutionError, - '_exception': exception.GlusterfsException}, - {'trouble': RuntimeError, '_exception': RuntimeError}) - @ddt.unpack - def test_create_share_gluster_call_error(self, trouble, _exception): - self._layout._pop_gluster_vol = mock.Mock( - return_value=self.glusterfs_target1) - gmgr1 = common.GlusterManager(self.glusterfs_target1) - gmgr1.gluster_call = mock.Mock(side_effect=trouble) - self.mock_object(self._layout, '_glustermanager', - mock.Mock(return_value=gmgr1)) - self.mock_object(self.fake_driver, '_setup_via_manager', - mock.Mock(return_value='host1:/gv1')) - - share = new_share() - self.assertRaises(_exception, self._layout.create_share, - self._context, share) - - self._layout._pop_gluster_vol.assert_called_once_with(share['size']) - self.fake_driver._setup_via_manager.assert_called_once_with( - {'manager': gmgr1, 'share': share}) - self._layout.private_storage.update.assert_called_once_with( - share['id'], {'volume': self.glusterfs_target1}) - gmgr1.gluster_call.assert_called_once_with( - 'volume', 'set', 'gv1', 'user.manila-share', share['id']) - def test_create_share_error(self): self._layout._pop_gluster_vol = mock.Mock( side_effect=exception.GlusterfsException) @@ -658,58 +617,6 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): gmgr1.gluster_call.assert_called_once_with( 'volume', 'delete', 'gv1') - @ddt.data({'trouble': exception.ProcessExecutionError, - '_exception': exception.GlusterfsException}, - {'trouble': RuntimeError, '_exception': RuntimeError}) - @ddt.unpack - def test_delete_share_gluster_call_error(self, trouble, _exception): - self._layout._push_gluster_vol = mock.Mock() - self._layout._wipe_gluster_vol = mock.Mock() - gmgr = common.GlusterManager - gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None) - gmgr1.gluster_call = mock.Mock(side_effect=trouble) - gmgr1.get_gluster_vol_option = mock.Mock(return_value=None) - self.mock_object(self._layout, '_glustermanager', - mock.Mock(return_value=gmgr1)) - self._layout.gluster_used_vols = set([self.glusterfs_target1]) - - self.assertRaises(_exception, self._layout.delete_share, - self._context, self.share1) - - gmgr1.get_gluster_vol_option.assert_called_once_with( - 'user.manila-cloned-from') - self._layout._wipe_gluster_vol.assert_called_once_with(gmgr1) - self.assertFalse(self._layout._push_gluster_vol.called) - self.assertFalse(self._layout.private_storage.delete.called) - gmgr1.gluster_call.assert_called_once_with( - 'volume', 'set', 'gv1', 'user.manila-share', 'NONE') - - @ddt.data({'trouble': exception.ProcessExecutionError, - '_exception': exception.GlusterfsException}, - {'trouble': RuntimeError, '_exception': RuntimeError}) - @ddt.unpack - def test_delete_share_gluster_call_clone_error(self, trouble, _exception): - self._layout._push_gluster_vol = mock.Mock() - self._layout._wipe_gluster_vol = mock.Mock() - gmgr = common.GlusterManager - gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None) - gmgr1.gluster_call = mock.Mock(side_effect=trouble) - gmgr1.get_gluster_vol_option = mock.Mock(return_value=FAKE_UUID1) - self.mock_object(self._layout, '_glustermanager', - mock.Mock(return_value=gmgr1)) - self._layout.gluster_used_vols = set([self.glusterfs_target1]) - - self.assertRaises(_exception, self._layout.delete_share, - self._context, self.share1) - - gmgr1.get_gluster_vol_option.assert_called_once_with( - 'user.manila-cloned-from') - self.assertFalse(self._layout._wipe_gluster_vol.called) - self.assertFalse(self._layout._push_gluster_vol.called) - self.assertFalse(self._layout.private_storage.delete.called) - gmgr1.gluster_call.assert_called_once_with( - 'volume', 'delete', 'gv1') - def test_delete_share_error(self): self._layout._wipe_gluster_vol = mock.Mock() self._layout._wipe_gluster_vol.side_effect = ( @@ -751,13 +658,10 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): self.assertIsNone(ret) args = ('--xml', 'snapshot', 'create', 'manila-fake_snap_id', gmgr1.volume) - gmgr1.gluster_call.assert_called_once_with(*args) + gmgr1.gluster_call.assert_called_once_with(*args, log=mock.ANY) @ddt.data({'side_effect': (glusterXMLOut(ret=-1, errno=2),), '_exception': exception.GlusterfsException}, - {'side_effect': exception.ProcessExecutionError, - '_exception': exception.GlusterfsException}, - {'side_effect': RuntimeError, '_exception': RuntimeError}, {'side_effect': (('', ''),), '_exception': exception.GlusterfsException}) @ddt.unpack @@ -782,7 +686,7 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): args = ('--xml', 'snapshot', 'create', 'manila-fake_snap_id', gmgr1.volume) - gmgr1.gluster_call.assert_called_once_with(*args) + gmgr1.gluster_call.assert_called_once_with(*args, log=mock.ANY) @ddt.data({"vers_minor": '6', "exctype": exception.GlusterfsException}, {"vers_minor": '7', @@ -811,7 +715,7 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): args = ('--xml', 'snapshot', 'create', 'manila-fake_snap_id', gmgr1.volume) - gmgr1.gluster_call.assert_called_once_with(*args) + gmgr1.gluster_call.assert_called_once_with(*args, log=mock.ANY) @ddt.data({"vers_minor": '6', "exctype": exception.GlusterfsException}, {"vers_minor": '7', @@ -850,25 +754,9 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): ret = self._layout._find_actual_backend_snapshot_name(gmgr1, snapshot) args = ('snapshot', 'list', gmgr1.volume, '--mode=script') - gmgr1.gluster_call.assert_called_once_with(*args) + gmgr1.gluster_call.assert_called_once_with(*args, log=mock.ANY) self.assertEqual('fake_snap_id_xyz', ret) - @ddt.data(exception.ProcessExecutionError, RuntimeError) - def test_find_actual_backend_snapshot_name_gluster_error(self, _exception): - gmgr = common.GlusterManager - gmgr1 = gmgr(self.share1['export_location'], self._execute, None, None) - self.mock_object(gmgr1, 'gluster_call', - mock.Mock(side_effect=_exception)) - - self.assertRaises({exception.ProcessExecutionError: - exception.GlusterfsException}.get(_exception, - _exception), - self._layout._find_actual_backend_snapshot_name, - gmgr1, mock.Mock()) - - args = ('snapshot', 'list', gmgr1.volume, '--mode=script') - gmgr1.gluster_call.assert_called_once_with(*args) - @ddt.data('this is too bad', 'fake_snap_id_xyx\nfake_snap_id_pqr') def test_find_actual_backend_snapshot_name_bad_snap_list(self, snaplist): gmgr = common.GlusterManager @@ -886,7 +774,7 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): gmgr1, snapshot) args = ('snapshot', 'list', gmgr1.volume, '--mode=script') - gmgr1.gluster_call.assert_called_once_with(*args) + gmgr1.gluster_call.assert_called_once_with(*args, log=mock.ANY) @ddt.data({'glusterfs_target': 'root@host1:/gv1', 'glusterfs_server': 'root@host1'}, @@ -933,13 +821,14 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): args = (('snapshot', 'activate', 'fake_snap_id_xyz', 'force', '--mode=script'), ('snapshot', 'clone', volume, 'fake_snap_id_xyz')) - old_gmgr.gluster_call.assert_has_calls([mock.call(*a) for a in args]) + old_gmgr.gluster_call.assert_has_calls( + [mock.call(*a, log=mock.ANY) for a in args]) args = (('volume', 'start', volume), ('volume', 'set', volume, 'user.manila-share', share['id']), ('volume', 'set', volume, 'user.manila-cloned-from', snapshot['share_id'])) - new_gmgr.gluster_call.assert_has_calls([mock.call(*a) for a in args], - any_order=True) + new_gmgr.gluster_call.assert_has_calls( + [mock.call(*a, log=mock.ANY) for a in args], any_order=True) self._layout._share_manager.assert_called_once_with( snapshot['share_instance']) self._layout._glustermanager.assert_called_once_with( @@ -954,114 +843,6 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): self._layout.gluster_used_vols) self.assertEqual('host1:/gv1', ret) - @ddt.data({'trouble': exception.ProcessExecutionError, - '_exception': exception.GlusterfsException}, - {'trouble': RuntimeError, '_exception': RuntimeError}) - @ddt.unpack - def test_create_share_from_snapshot_error_new_gmr_gluster_calls( - self, trouble, _exception): - glusterfs_target = 'root@host1:/gv1' - glusterfs_server = 'root@host1' - share = new_share() - volume = ''.join(['manila-', share['id']]) - new_vol_addr = ':/'.join([glusterfs_server, volume]) - gmgr = common.GlusterManager - old_gmgr = gmgr(glusterfs_target, self._execute, None, None) - new_gmgr = gmgr(new_vol_addr, self._execute, None, None) - self._layout.gluster_used_vols = set([glusterfs_target]) - self._layout.glusterfs_versions = {glusterfs_server: ('3', '7')} - self.mock_object(old_gmgr, 'gluster_call', - mock.Mock(side_effect=[('', ''), ('', '')])) - self.mock_object(new_gmgr, 'gluster_call', - mock.Mock(side_effect=trouble)) - self.mock_object(new_gmgr, 'get_gluster_vol_option', - mock.Mock()) - new_gmgr.get_gluster_vol_option.return_value = ( - 'glusterfs-server-1,client') - self.mock_object(self._layout, '_find_actual_backend_snapshot_name', - mock.Mock(return_value='fake_snap_id_xyz')) - self.mock_object(self._layout, '_share_manager', - mock.Mock(return_value=old_gmgr)) - self.mock_object(self._layout, '_glustermanager', - mock.Mock(return_value=new_gmgr)) - self.mock_object(self.fake_driver, '_setup_via_manager', - mock.Mock(return_value='host1:/gv1')) - - snapshot = { - 'id': 'fake_snap_id', - 'share_instance': new_share(export_location=glusterfs_target), - 'share_id': 'fake_share_id', - } - self.assertRaises(_exception, - self._layout.create_share_from_snapshot, - self._context, share, snapshot) - - (self._layout._find_actual_backend_snapshot_name. - assert_called_once_with(old_gmgr, snapshot)) - args = (('snapshot', 'activate', 'fake_snap_id_xyz', - 'force', '--mode=script'), - ('snapshot', 'clone', volume, 'fake_snap_id_xyz')) - old_gmgr.gluster_call.assert_has_calls([mock.call(*a) for a in args]) - self.assertTrue(new_gmgr.gluster_call.called) - self._layout._share_manager.assert_called_once_with( - snapshot['share_instance']) - self._layout._glustermanager.assert_called_once_with( - gmgr.parse(new_vol_addr)) - self._layout.driver._setup_via_manager.assert_called_once_with( - {'manager': new_gmgr, 'share': share}, - {'manager': old_gmgr, 'share': snapshot['share_instance']}) - - @ddt.data({'trouble': exception.ProcessExecutionError, - '_exception': exception.GlusterfsException}, - {'trouble': RuntimeError, '_exception': RuntimeError}) - @ddt.unpack - def test_create_share_from_snapshot_error_old_gmr_gluster_calls( - self, trouble, _exception): - glusterfs_target = 'root@host1:/gv1' - glusterfs_server = 'root@host1' - share = new_share() - volume = ''.join(['manila-', share['id']]) - new_vol_addr = ':/'.join([glusterfs_server, volume]) - gmgr = common.GlusterManager - old_gmgr = gmgr(glusterfs_target, self._execute, None, None) - new_gmgr = gmgr(new_vol_addr, self._execute, None, None) - self._layout.gluster_used_vols_dict = {glusterfs_target: old_gmgr} - self._layout.glusterfs_versions = {glusterfs_server: ('3', '7')} - self.mock_object( - old_gmgr, 'gluster_call', - mock.Mock(side_effect=[('', ''), trouble])) - self.mock_object(new_gmgr, 'get_gluster_vol_option', - mock.Mock()) - new_gmgr.get_gluster_vol_option.return_value = ( - 'glusterfs-server-1,client') - self.mock_object(self._layout, '_find_actual_backend_snapshot_name', - mock.Mock(return_value='fake_snap_id_xyz')) - self.mock_object(self._layout, '_share_manager', - mock.Mock(return_value=old_gmgr)) - self.mock_object(self._layout, '_glustermanager', - mock.Mock(return_value=new_gmgr)) - - snapshot = { - 'id': 'fake_snap_id', - 'share_instance': new_share(export_location=glusterfs_target) - } - self.assertRaises(_exception, - self._layout.create_share_from_snapshot, - self._context, share, snapshot) - - (self._layout._find_actual_backend_snapshot_name. - assert_called_once_with(old_gmgr, snapshot)) - args = (('snapshot', 'activate', 'fake_snap_id_xyz', - 'force', '--mode=script'), - ('snapshot', 'clone', volume, 'fake_snap_id_xyz')) - old_gmgr.gluster_call.assert_has_calls([mock.call(*a) for a in args]) - self._layout._share_manager.assert_called_once_with( - snapshot['share_instance']) - self.assertFalse(new_gmgr.get_gluster_vol_option.called) - self.assertFalse(new_gmgr.gluster_call.called) - self.assertNotIn(new_vol_addr, - self._layout.glusterfs_versions.keys()) - def test_create_share_from_snapshot_error_unsupported_gluster_version( self): glusterfs_target = 'root@host1:/gv1' @@ -1129,15 +910,12 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): self.assertIsNone(ret) args = ('--xml', 'snapshot', 'delete', 'fake_snap_id_xyz', '--mode=script') - gmgr1.gluster_call.assert_called_once_with(*args) + gmgr1.gluster_call.assert_called_once_with(*args, log=mock.ANY) (self._layout._find_actual_backend_snapshot_name. assert_called_once_with(gmgr1, snapshot)) @ddt.data({'side_effect': (glusterXMLOut(ret=-1, errno=2),), '_exception': exception.GlusterfsException}, - {'side_effect': exception.ProcessExecutionError, - '_exception': exception.GlusterfsException}, - {'side_effect': RuntimeError, '_exception': RuntimeError}, {'side_effect': (('', ''),), '_exception': exception.GlusterfsException}) @ddt.unpack @@ -1164,7 +942,7 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase): self.assertRaises(_exception, self._layout.delete_snapshot, self._context, snapshot) - gmgr1.gluster_call.assert_called_once_with(*args) + gmgr1.gluster_call.assert_called_once_with(*args, log=mock.ANY) (self._layout._find_actual_backend_snapshot_name. assert_called_once_with(gmgr1, snapshot)) diff --git a/manila/tests/share/drivers/test_glusterfs.py b/manila/tests/share/drivers/test_glusterfs.py index 5c2751c4ed..3832b27d22 100644 --- a/manila/tests/share/drivers/test_glusterfs.py +++ b/manila/tests/share/drivers/test_glusterfs.py @@ -97,7 +97,8 @@ class GlusterfsShareDriverTestCase(test.TestCase): share_manager_parent=share_mgr_parent) gmgr.gluster_call.assert_called_once_with( - 'volume', 'set', gmgr.volume, 'nfs.export-volumes', 'off') + 'volume', 'set', gmgr.volume, 'nfs.export-volumes', 'off', + log=mock.ANY) self._driver.nfs_helper.assert_called_once_with( self._execute, self.fake_conf, gluster_manager=gmgr) nfs_helper.get_export.assert_called_once_with(self.share) @@ -130,19 +131,7 @@ class GlusterfsShareDriverTestCase(test.TestCase): else: args = (NFS_EXPORT_VOL, 'off') gmgr.gluster_call.assert_called_once_with( - 'volume', 'set', gmgr.volume, *args) - - @ddt.data(exception.ProcessExecutionError, RuntimeError) - def test_setup_via_manager_exception(self, _exception): - gmgr = mock.Mock() - gmgr.gluster_call = mock.Mock(side_effect=_exception) - gmgr.volume = 'somevol' - - self.assertRaises( - {exception.ProcessExecutionError: - exception.GlusterfsException}.get( - _exception, _exception), self._driver._setup_via_manager, - {'manager': gmgr, 'share': self.share}) + 'volume', 'set', gmgr.volume, *args, log=mock.ANY) @ddt.data('off', 'no', '0', 'false', 'disable', 'foobarbaz') def test_setup_via_manager_export_volumes_on(self, export_vol): @@ -291,39 +280,7 @@ class GlusterNFSHelperTestCase(test.TestCase): self.assertIsNone(ret) self._helper._get_export_dir_dict.assert_called_once_with() self._helper.gluster_manager.gluster_call.assert_called_once_with( - *args) - - def test_manage_access_adding_entry_cmd_fail(self): - - def cbk(d, key, value): - d[key].append(value) - - def raise_exception(*args, **kwargs): - raise exception.ProcessExecutionError() - - access = fake_share.fake_access() - export_dir_dict = { - 'example.com': ['10.0.0.1'], - 'fakename': ['10.0.0.2'], - } - export_str = '/example.com(10.0.0.1),/fakename(10.0.0.2|10.0.0.1)' - args = ('volume', 'set', self._helper.gluster_manager.volume, - NFS_EXPORT_DIR, export_str) - self.mock_object(self._helper, '_get_export_dir_dict', - mock.Mock(return_value=export_dir_dict)) - self.mock_object(self._helper.gluster_manager, 'gluster_call', - mock.Mock(side_effect=raise_exception)) - self.mock_object(glusterfs.LOG, 'error') - - self.assertRaises(exception.ProcessExecutionError, - self._helper._manage_access, - fake_share_name, access['access_type'], - access['access_to'], cbk) - - self._helper._get_export_dir_dict.assert_called_once_with() - self._helper.gluster_manager.gluster_call.assert_called_once_with( - *args) - glusterfs.LOG.error.assert_called_once_with(mock.ANY, mock.ANY) + *args, log=mock.ANY) def test_manage_access_removing_last_entry(self): @@ -344,7 +301,7 @@ class GlusterNFSHelperTestCase(test.TestCase): self.assertIsNone(ret) self._helper._get_export_dir_dict.assert_called_once_with() self._helper.gluster_manager.gluster_call.assert_called_once_with( - *args) + *args, log=mock.ANY) def test_allow_access_with_share_having_noaccess(self): access = fake_share.fake_access() @@ -360,7 +317,7 @@ class GlusterNFSHelperTestCase(test.TestCase): self._helper._get_export_dir_dict.assert_called_once_with() self._helper.gluster_manager.gluster_call.assert_called_once_with( 'volume', 'set', self._helper.gluster_manager.volume, - NFS_EXPORT_DIR, export_str) + NFS_EXPORT_DIR, export_str, log=mock.ANY) def test_allow_access_with_share_having_access(self): access = fake_share.fake_access() @@ -406,7 +363,7 @@ class GlusterNFSHelperTestCase(test.TestCase): self._helper._get_export_dir_dict.assert_called_once_with() self._helper.gluster_manager.gluster_call.assert_called_once_with( - *args) + *args, log=mock.ANY) @ddt.ddt @@ -479,36 +436,9 @@ class GlusterNFSVolHelperTestCase(test.TestCase): ('volume', 'reset', self._helper.gluster_manager.volume, NFS_RPC_AUTH_REJECT)) self.assertEqual( - [mock.call(*a) for a in argseq], + [mock.call(*a, log=mock.ANY) for a in argseq], self._helper.gluster_manager.gluster_call.call_args_list) - def test_manage_access_adding_entry_cmd_fail(self): - - def cbk(li, v): - li.append(v) - - def raise_exception(*args, **kwargs): - raise exception.ProcessExecutionError() - - access = fake_share.fake_access() - export_list = ['10.0.0.2'] - self.mock_object(self._helper, '_get_vol_exports', - mock.Mock(return_value=export_list)) - self.mock_object(self._helper.gluster_manager, 'gluster_call', - mock.Mock(side_effect=raise_exception)) - - self.assertRaises(exception.ProcessExecutionError, - self._helper._manage_access, - access['access_type'], - access['access_to'], cbk) - - self._helper._get_vol_exports.assert_called_once_with() - export_str = '10.0.0.2,10.0.0.1' - args = ('volume', 'set', self._helper.gluster_manager.volume, - NFS_RPC_AUTH_ALLOW, export_str) - self._helper.gluster_manager.gluster_call.assert_called_once_with( - *args) - def test_manage_access_removing_last_entry(self): def cbk(li, v): @@ -529,7 +459,7 @@ class GlusterNFSVolHelperTestCase(test.TestCase): ('volume', 'set', self._helper.gluster_manager.volume, NFS_RPC_AUTH_REJECT, '*')) self.assertEqual( - [mock.call(*a) for a in argseq], + [mock.call(*a, log=mock.ANY) for a in argseq], self._helper.gluster_manager.gluster_call.call_args_list) def test_allow_access_with_share_having_noaccess(self): @@ -548,7 +478,7 @@ class GlusterNFSVolHelperTestCase(test.TestCase): ('volume', 'reset', self._helper.gluster_manager.volume, NFS_RPC_AUTH_REJECT)) self.assertEqual( - [mock.call(*a) for a in argseq], + [mock.call(*a, log=mock.ANY) for a in argseq], self._helper.gluster_manager.gluster_call.call_args_list) def test_allow_access_with_share_having_access(self): @@ -591,7 +521,7 @@ class GlusterNFSVolHelperTestCase(test.TestCase): ('volume', 'reset', self._helper.gluster_manager.volume, NFS_RPC_AUTH_REJECT)) self.assertEqual( - [mock.call(*a) for a in argseq], + [mock.call(*a, log=mock.ANY) for a in argseq], self._helper.gluster_manager.gluster_call.call_args_list) diff --git a/manila/tests/share/drivers/test_glusterfs_native.py b/manila/tests/share/drivers/test_glusterfs_native.py index d83e3c7a8c..dab8fee4d9 100644 --- a/manila/tests/share/drivers/test_glusterfs_native.py +++ b/manila/tests/share/drivers/test_glusterfs_native.py @@ -98,9 +98,18 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): self.assertEqual(('GLUSTERFS', ), self._driver.supported_protocols) - def test_setup_via_manager(self): + @ddt.data(True, False) + def test_setup_via_manager(self, has_dynauth): gmgr = mock.Mock() - gmgr.gluster_call = mock.Mock() + + if has_dynauth: + _gluster_call = lambda *args, **kwargs: None + else: + def _gluster_call(*args, **kwargs): + if kwargs.get('raw_error'): + raise exception.ProcessExecutionError(exit_code=1) + + gmgr.gluster_call = mock.Mock(side_effect=_gluster_call) gmgr.volume = 'fakevol' gmgr.export = 'fakehost:/fakevol' gmgr.get_gluster_vol_option = mock.Mock( @@ -112,13 +121,18 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): gmgr.get_gluster_vol_option.assert_called_once_with('auth.ssl-allow') args = ( - ('volume', 'set', 'fakevol', 'nfs.export-volumes', 'off'), - ('volume', 'set', 'fakevol', 'client.ssl', 'on'), - ('volume', 'set', 'fakevol', 'server.ssl', 'on'), - ('volume', 'set', 'fakevol', 'server.dynamic-auth', 'on'), - ('volume', 'stop', 'fakevol', '--mode=script'), - ('volume', 'start', 'fakevol')) - gmgr.gluster_call.assert_has_calls([mock.call(*a) for a in args]) + ('volume', 'set', 'fakevol', 'nfs.export-volumes', 'off', + {'log': mock.ANY}), + ('volume', 'set', 'fakevol', 'client.ssl', 'on', + {'log': mock.ANY}), + ('volume', 'set', 'fakevol', 'server.ssl', 'on', + {'log': mock.ANY}), + ('volume', 'set', 'fakevol', 'server.dynamic-auth', 'on', + {'raw_error': True}), + ('volume', 'stop', 'fakevol', '--mode=script', {'log': mock.ANY}), + ('volume', 'start', 'fakevol', {'log': mock.ANY})) + gmgr.gluster_call.assert_has_calls( + [mock.call(*a[:-1], **a[-1]) for a in args]) self.assertEqual(ret, gmgr.export) def test_setup_via_manager_with_parent(self): @@ -141,12 +155,17 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): 'auth.ssl-allow') args = ( ('volume', 'set', 'fakevol', 'auth.ssl-allow', - 'glusterfs-server-name,manila-host.com'), - ('volume', 'set', 'fakevol', 'nfs.export-volumes', 'off'), - ('volume', 'set', 'fakevol', 'client.ssl', 'on'), - ('volume', 'set', 'fakevol', 'server.ssl', 'on'), - ('volume', 'set', 'fakevol', 'server.dynamic-auth', 'on')) - gmgr.gluster_call.assert_has_calls([mock.call(*a) for a in args]) + 'glusterfs-server-name,manila-host.com', {'log': mock.ANY}), + ('volume', 'set', 'fakevol', 'nfs.export-volumes', 'off', + {'log': mock.ANY}), + ('volume', 'set', 'fakevol', 'client.ssl', 'on', + {'log': mock.ANY}), + ('volume', 'set', 'fakevol', 'server.ssl', 'on', + {'log': mock.ANY}), + ('volume', 'set', 'fakevol', 'server.dynamic-auth', 'on', + {'raw_error': True})) + gmgr.gluster_call.assert_has_calls( + [mock.call(*a[:-1], **a[-1]) for a in args]) self.assertEqual(ret, gmgr.export) @ddt.data(True, False) @@ -171,17 +190,26 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): gmgr_queried.get_gluster_vol_option.assert_called_once_with( 'auth.ssl-allow') - @ddt.data(exception.ProcessExecutionError, RuntimeError) - def test_setup_via_manager_exception(self, _exception): + @ddt.data({'trouble': exception.ProcessExecutionError, + 'trouble_kw': {'exit_code': 2}, + '_exception': exception.GlusterfsException}, + {'trouble': RuntimeError, 'trouble_kw': {}, + '_exception': RuntimeError}) + @ddt.unpack + def test_setup_via_manager_exception(self, trouble, trouble_kw, + _exception): share = mock.Mock() gmgr = mock.Mock() - gmgr.gluster_call = mock.Mock(side_effect=_exception) + + def raise_exception(*args, **kwargs): + if kwargs.get('raw_error'): + raise trouble(**trouble_kw) + + gmgr.gluster_call = mock.Mock(side_effect=raise_exception) gmgr.get_gluster_vol_option = mock.Mock() self.assertRaises( - {exception.ProcessExecutionError: - exception.GlusterfsException}.get( - _exception, _exception), self._driver._setup_via_manager, + _exception, self._driver._setup_via_manager, {'share': share, 'manager': gmgr}) def test_snapshots_are_supported(self): @@ -201,7 +229,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): self._driver._allow_access_via_manager(gmgr1, self._context, self.share1, access) gmgr1.get_gluster_vol_option.assert_called_once_with('auth.ssl-allow') - gmgr1.gluster_call.assert_called_once_with(*test_args) + gmgr1.gluster_call.assert_called_once_with(*test_args, log=mock.ANY) def test_allow_access_via_manager_with_share_having_access(self): access = {'access_type': 'cert', 'access_to': 'client.example.com'} @@ -228,30 +256,6 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): self.assertEqual(expected_exec, fake_utils.fake_execute_get_log()) - def test_allow_access_via_manager_excp(self): - access = {'access_type': 'cert', 'access_to': 'client.example.com'} - test_args = ('volume', 'set', 'gv1', 'auth.ssl-allow', - 'some.common.name,' + access['access_to']) - - def raise_exception(*args, **kwargs): - if (args == test_args): - raise exception.ProcessExecutionError() - - gmgr1 = common.GlusterManager(self.glusterfs_target1, self._execute, - None, None) - self.mock_object(gmgr1, 'get_gluster_vol_option', - mock.Mock(return_value='some.common.name')) - self._driver.layout.gluster_used_vols = set([self.glusterfs_target1]) - self.mock_object(gmgr1, 'gluster_call', - mock.Mock(side_effect=raise_exception)) - - self.assertRaises(exception.GlusterfsException, - self._driver._allow_access_via_manager, gmgr1, - self._context, self.share1, access) - - gmgr1.get_gluster_vol_option.assert_called_once_with('auth.ssl-allow') - gmgr1.gluster_call.assert_called_once_with(*test_args) - @ddt.data('on', '1', 'Yes', 'TRUE', 'enable') def test_deny_access_via_manager(self, trueish): self.mock_object(common, '_restart_gluster_vol', mock.Mock()) @@ -277,7 +281,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): [mock.call(a) for a in ('auth.ssl-allow', 'server.dynamic-auth')]) test_args = ('volume', 'set', 'gv1', 'auth.ssl-allow', 'some.common.name') - gmgr1.gluster_call.assert_called_once_with(*test_args) + gmgr1.gluster_call.assert_called_once_with(*test_args, log=mock.ANY) self.assertFalse(common._restart_gluster_vol.called) @ddt.data('off', None, 'strangelove') @@ -305,7 +309,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): [mock.call(a) for a in ('auth.ssl-allow', 'server.dynamic-auth')]) test_args = ('volume', 'set', 'gv1', 'auth.ssl-allow', 'some.common.name') - gmgr1.gluster_call.assert_called_once_with(*test_args) + gmgr1.gluster_call.assert_called_once_with(*test_args, log=mock.ANY) common._restart_gluster_vol.assert_called_once_with(gmgr1) def test_deny_access_via_manager_with_share_having_no_access(self): @@ -334,70 +338,6 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase): self.assertFalse(common._restart_gluster_vol.called) - @ddt.data({'trouble': exception.ProcessExecutionError, - '_exception': exception.GlusterfsException}, - {'trouble': RuntimeError, '_exception': RuntimeError}) - @ddt.unpack - def test_deny_access_via_manager_excp(self, trouble, _exception): - access = {'access_type': 'cert', 'access_to': 'client.example.com'} - test_args = ('volume', 'set', 'gv1', 'auth.ssl-allow', - 'some.common.name') - - def raise_exception(*args, **kwargs): - if (args == test_args): - raise trouble() - - self.mock_object(common, '_restart_gluster_vol', mock.Mock()) - gmgr1 = common.GlusterManager(self.glusterfs_target1, self._execute, - None, None) - self.mock_object( - gmgr1, 'get_gluster_vol_option', - mock.Mock(return_value='some.common.name,' + access['access_to'])) - self._driver.layout.gluster_used_vols = set([self.glusterfs_target1]) - - self.mock_object(gmgr1, 'gluster_call', - mock.Mock(side_effect=raise_exception)) - - self.assertRaises(_exception, - self._driver._deny_access_via_manager, gmgr1, - self._context, self.share1, access) - - gmgr1.get_gluster_vol_option.assert_called_once_with('auth.ssl-allow') - gmgr1.gluster_call.assert_called_once_with(*test_args) - self.assertFalse(common._restart_gluster_vol.called) - - @ddt.data({'trouble': exception.ProcessExecutionError, - '_exception': exception.GlusterfsException}, - {'trouble': RuntimeError, '_exception': RuntimeError}) - @ddt.unpack - def test_deny_access_via_manager_dyn_auth_fail(self, trouble, _exception): - self.mock_object(common, '_restart_gluster_vol', mock.Mock()) - access = {'access_type': 'cert', 'access_to': 'client.example.com'} - gmgr1 = common.GlusterManager(self.glusterfs_target1, self._execute, - None, None) - - def _get_gluster_vol_option(opt): - if opt == 'auth.ssl-allow': - return('some.common.name,' + access['access_to']) - elif opt == 'server.dynamic-auth': - raise trouble - - self.mock_object( - gmgr1, 'get_gluster_vol_option', - mock.Mock(side_effect=_get_gluster_vol_option)) - self._driver.layout.gluster_used_vols = set([self.glusterfs_target1]) - - self.assertRaises(_exception, - self._driver._deny_access_via_manager, gmgr1, - self._context, self.share1, access) - - gmgr1.get_gluster_vol_option.assert_has_calls( - [mock.call(a) for a in ('auth.ssl-allow', 'server.dynamic-auth')]) - test_args = ('volume', 'set', 'gv1', 'auth.ssl-allow', - 'some.common.name') - gmgr1.gluster_call.assert_called_once_with(*test_args) - self.assertFalse(common._restart_gluster_vol.called) - def test_update_share_stats(self): self._driver._update_share_stats()