From ee8af90109cc41d1c22f2e0cd1718f7cfd2fdfe7 Mon Sep 17 00:00:00 2001 From: Goutham Pacha Ravi Date: Wed, 29 Mar 2023 22:19:19 -0700 Subject: [PATCH] Implement Ensure Shares in the CephFS driver This allows switching between NFS protocol helpers that the driver currently supports and avoids unnecessary ensuring of driver resources on restarts unless a small set of config opts are toggled. Use of either NFS protocol helper in the driver will trigger access rules to be reapplied when ensuring shares. Implements: bp use-cephadm-nfs-ganesha Partial-Bug: #2035137 Change-Id: I93c4e8616a1cfb5ab713b420aff69464969f28d5 Signed-off-by: Goutham Pacha Ravi --- manila/share/drivers/cephfs/driver.py | 69 ++++++++++++++++--- manila/share/drivers/ganesha/__init__.py | 6 ++ .../tests/share/drivers/cephfs/test_driver.py | 69 +++++++++++-------- ...upport-ensure-shares-b72fe18381af274a.yaml | 17 +++++ 4 files changed, 126 insertions(+), 35 deletions(-) create mode 100644 releasenotes/notes/bug-2035137-cephfs-support-ensure-shares-b72fe18381af274a.yaml diff --git a/manila/share/drivers/cephfs/driver.py b/manila/share/drivers/cephfs/driver.py index 0e24407e2e..cc891a5dee 100644 --- a/manila/share/drivers/cephfs/driver.py +++ b/manila/share/drivers/cephfs/driver.py @@ -138,6 +138,13 @@ cephfs_opts = [ cfg.StrOpt('cephfs_filesystem_name', help="The name of the filesystem to use, if there are " "multiple filesystems in the cluster."), + cfg.StrOpt('cephfs_ensure_all_shares_salt', + default="manila_cephfs_reef_bobcat", + help="Provide a unique string value to make the driver " + "ensure all of the shares it has created during " + "startup. Ensuring would re-export shares and this " + "action isn't always required, unless something has " + "been administratively modified on CephFS.") ] cephfsnfs_opts = [ @@ -540,15 +547,31 @@ class CephFSDriver(driver.ExecuteMixin, driver.GaneshaMixin, context, share, access_rules, add_rules, delete_rules, share_server=share_server) - def ensure_share(self, context, share, share_server=None): - try: - export_location = self._get_export_locations(share) - except exception.ShareBackendException as e: - if 'does not exist' in str(e).lower(): - raise exception.ShareResourceNotFound(share_id=share['id']) - raise + def get_backend_info(self, context): + return self.protocol_helper.get_backend_info(context) - return export_location + def ensure_shares(self, context, shares): + share_updates = {} + for share in shares: + share_updates[share['id']] = { + 'reapply_access_rules': + self.protocol_helper.reapply_rules_while_ensuring_shares, + } + try: + share_updates[share['id']].update({ + 'export_locations': self._get_export_locations(share), + }) + except exception.ShareBackendException as e: + if 'does not exist' in str(e).lower(): + msg = ("Share instance %(si)s belonging to share " + "%(share)s cannot be found on the backend.") + msg_payload = {'si': share['id'], + 'share': share['share_id']} + LOG.exception(msg, msg_payload) + share_updates[share['id']] = { + 'status': constants.STATUS_ERROR, + } + return share_updates def extend_share(self, share, new_size, share_server=None): # resize FS subvolume/share @@ -778,6 +801,7 @@ class NativeProtocolHelper(ganesha.NASHelperBase): supported_access_types = (CEPHX_ACCESS_TYPE, ) supported_access_levels = (constants.ACCESS_LEVEL_RW, constants.ACCESS_LEVEL_RO) + reapply_rules_while_ensuring_shares = False def __init__(self, execute, config, **kwargs): self.rados_client = kwargs.pop('rados_client') @@ -803,6 +827,12 @@ class NativeProtocolHelper(ganesha.NASHelperBase): return result + def get_backend_info(self, context): + return { + "cephfs_ensure_all_shares_salt": + self.configuration.cephfs_ensure_all_shares_salt, + } + def get_export_locations(self, share, subvolume_path): # To mount this you need to know the mon IPs and the path to the volume mon_addrs = self.get_mon_addrs() @@ -1055,6 +1085,7 @@ class NFSProtocolHelper(NFSProtocolHelperMixin, ganesha.GaneshaNASHelper2): shared_data = {} supported_protocols = ('NFS',) + reapply_rules_while_ensuring_shares = True def __init__(self, execute, config_object, **kwargs): if config_object.cephfs_ganesha_server_is_remote: @@ -1151,12 +1182,22 @@ class NFSProtocolHelper(NFSProtocolHelperMixin, ganesha.GaneshaNASHelper2): return export_ips + def get_backend_info(self, context): + backend_info = { + "cephfs_ganesha_export_ips": self.config.cephfs_ganesha_export_ips, + "cephfs_ganesha_server_ip": self.config.cephfs_ganesha_server_ip, + "cephfs_ensure_all_shares_salt": + self.configuration.cephfs_ensure_all_shares_salt, + } + return backend_info + class NFSClusterProtocolHelper(NFSProtocolHelperMixin, ganesha.NASHelperBase): supported_access_types = ('ip', ) supported_access_levels = (constants.ACCESS_LEVEL_RW, constants.ACCESS_LEVEL_RO) + reapply_rules_while_ensuring_shares = True def __init__(self, execute, config_object, **kwargs): self.rados_client = kwargs.pop('rados_client') @@ -1308,3 +1349,15 @@ class NFSClusterProtocolHelper(NFSProtocolHelperMixin, ganesha.NASHelperBase): self._deny_access(share) return rule_state_map + + def get_backend_info(self, context): + backend_info = { + "cephfs_ganesha_export_ips": + self.configuration.cephfs_ganesha_export_ips, + "cephfs_ganesha_server_ip": + self.configuration.cephfs_ganesha_server_ip, + "cephfs_nfs_cluster_id": self.nfs_clusterid, + "cephfs_ensure_all_shares_salt": + self.configuration.cephfs_ensure_all_shares_salt, + } + return backend_info diff --git a/manila/share/drivers/ganesha/__init__.py b/manila/share/drivers/ganesha/__init__.py index b079472e79..f73939af45 100644 --- a/manila/share/drivers/ganesha/__init__.py +++ b/manila/share/drivers/ganesha/__init__.py @@ -53,6 +53,12 @@ class NASHelperBase(metaclass=abc.ABCMeta): delete_rules, share_server=None): """Update access rules of share.""" + def get_backend_info(self, context): + raise NotImplementedError + + def ensure_shares(self, context, shares): + raise NotImplementedError + class GaneshaNASHelper(NASHelperBase): """Perform share access changes using Ganesha version < 2.4.""" diff --git a/manila/tests/share/drivers/cephfs/test_driver.py b/manila/tests/share/drivers/cephfs/test_driver.py index d9ba75218d..d6f27a9e97 100644 --- a/manila/tests/share/drivers/cephfs/test_driver.py +++ b/manila/tests/share/drivers/cephfs/test_driver.py @@ -235,38 +235,53 @@ class CephFSDriverTestCase(test.TestCase): self._context, self._share, access_rules, add_rules, delete_rules, share_server=None) - def test_ensure_share(self): - expected_exports = { - 'path': '1.2.3.4,5.6.7.8:/foo/bar', - 'is_admin_only': False, - 'metadata': {}, + def test_ensure_shares(self): + self._driver.protocol_helper.reapply_rules_while_ensuring_shares = True + shares = [ + fake_share.fake_share(share_id='123', share_proto='NFS'), + fake_share.fake_share(share_id='456', share_proto='NFS'), + fake_share.fake_share(share_id='789', share_proto='NFS') + ] + export_locations = [ + { + 'path': '1.2.3.4,5.6.7.8:/foo/bar', + 'is_admin_only': False, + 'metadata': {}, + }, + { + 'path': '1.2.3.4,5.6.7.8:/foo/quz', + 'is_admin_only': False, + 'metadata': {}, + }, + + ] + expected_updates = { + shares[0]['id']: { + 'status': constants.STATUS_ERROR, + 'reapply_access_rules': True, + }, + shares[1]['id']: { + 'export_locations': export_locations[0], + 'reapply_access_rules': True, + }, + shares[2]['id']: { + 'export_locations': export_locations[1], + 'reapply_access_rules': True, + } } + err_message = (f"Error ENOENT: subvolume {self._share['id']} does " + f"not exist") + expected_exception = exception.ShareBackendException(err_message) self.mock_object( self._driver, '_get_export_locations', - mock.Mock(return_value=expected_exports)) + mock.Mock(side_effect=[expected_exception] + export_locations)) - returned_exports = self._driver.ensure_share(self._context, - self._share) + actual_updates = self._driver.ensure_shares(self._context, shares) - self._driver._get_export_locations.assert_called_once_with(self._share) - - self.assertEqual(expected_exports, returned_exports) - - def test_ensure_share_subvolume_not_found(self): - message = f"Error ENOENT: subvolume {self._share['id']} does not exist" - expected_exception = exception.ShareBackendException(message) - - self.mock_object( - self._driver, '_get_export_locations', - mock.Mock(side_effect=expected_exception)) - - self.assertRaises( - exception.ShareResourceNotFound, - self._driver.ensure_share, - self._context, - self._share) - - self._driver._get_export_locations.assert_called_once_with(self._share) + self.assertEqual(3, self._driver._get_export_locations.call_count) + self._driver._get_export_locations.assert_has_calls([ + mock.call(shares[0]), mock.call(shares[1]), mock.call(shares[2])]) + self.assertEqual(expected_updates, actual_updates) def test_delete_share(self): clone_status_prefix = "fs clone status" diff --git a/releasenotes/notes/bug-2035137-cephfs-support-ensure-shares-b72fe18381af274a.yaml b/releasenotes/notes/bug-2035137-cephfs-support-ensure-shares-b72fe18381af274a.yaml new file mode 100644 index 0000000000..d33da8430f --- /dev/null +++ b/releasenotes/notes/bug-2035137-cephfs-support-ensure-shares-b72fe18381af274a.yaml @@ -0,0 +1,17 @@ +--- +fixes: + - | + The CephFS backend driver now supports a bulk share recovery mechanism + (``ensure_shares``). At startup time, a combination of driver configuration + options will determine if the driver must re-evaluate export paths of + previously created shares. If these configuration options do not change, + service startup will skip through this recovery stage. + - | + The CephFS backend driver will also reapply access rules when performing + a recovery of pre-existing shares. +upgrade: + - | + A new configuration option called ``cephfs_ensure_all_shares_salt`` has + been introduced to assist cloud administrtrators that would like the CephFS + driver to reconcile export paths of existing shares during service + startup.