From 6e0acc0768371106735df2ff3ca599cb060cea13 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Thu, 27 Jan 2022 15:50:13 -0500 Subject: [PATCH] mgr/nfs: limit dependency of NFSRados object Previously, the NFSRados object accepted the "Module" as the first argument but only used the rados attribute (type rados.Rados). It's better to limit the scope of types when reasonably possible so we can see what the true dependencies are. So we restrict NFSRados to accepting a rados.Rados as the argument. Signed-off-by: John Mulligan (cherry picked from commit d94b63830d94f21ba276452844a46d21e084fb3f) --- src/pybind/mgr/nfs/cluster.py | 14 +++++++++----- src/pybind/mgr/nfs/export.py | 28 ++++++++++++++++------------ 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/pybind/mgr/nfs/cluster.py b/src/pybind/mgr/nfs/cluster.py index 721eda2bdfe72..bebfd9973ba4c 100644 --- a/src/pybind/mgr/nfs/cluster.py +++ b/src/pybind/mgr/nfs/cluster.py @@ -91,11 +91,11 @@ class NFSCluster: def create_empty_rados_obj(self, cluster_id: str) -> None: common_conf = self._get_common_conf_obj_name(cluster_id) - NFSRados(self.mgr, cluster_id).write_obj('', self._get_common_conf_obj_name(cluster_id)) + self._rados(cluster_id).write_obj('', self._get_common_conf_obj_name(cluster_id)) log.info("Created empty object:%s", common_conf) def delete_config_obj(self, cluster_id: str) -> None: - NFSRados(self.mgr, cluster_id).remove_all_obj() + self._rados(cluster_id).remove_all_obj() log.info("Deleted %s object and all objects in %s", self._get_common_conf_obj_name(cluster_id), cluster_id) @@ -215,7 +215,7 @@ class NFSCluster: def get_nfs_cluster_config(self, cluster_id: str) -> Tuple[int, str, str]: try: if cluster_id in available_clusters(self.mgr): - rados_obj = NFSRados(self.mgr, cluster_id) + rados_obj = self._rados(cluster_id) conf = rados_obj.read_obj(self._get_user_conf_obj_name(cluster_id)) return 0, conf or "", "" raise ClusterNotFound() @@ -225,7 +225,7 @@ class NFSCluster: def set_nfs_cluster_config(self, cluster_id: str, nfs_config: str) -> Tuple[int, str, str]: try: if cluster_id in available_clusters(self.mgr): - rados_obj = NFSRados(self.mgr, cluster_id) + rados_obj = self._rados(cluster_id) if rados_obj.check_user_config(): return 0, "", "NFS-Ganesha User Config already exists" rados_obj.write_obj(nfs_config, self._get_user_conf_obj_name(cluster_id), @@ -243,7 +243,7 @@ class NFSCluster: def reset_nfs_cluster_config(self, cluster_id: str) -> Tuple[int, str, str]: try: if cluster_id in available_clusters(self.mgr): - rados_obj = NFSRados(self.mgr, cluster_id) + rados_obj = self._rados(cluster_id) if not rados_obj.check_user_config(): return 0, "", "NFS-Ganesha User Config does not exist" rados_obj.remove_obj(self._get_user_conf_obj_name(cluster_id), @@ -256,3 +256,7 @@ class NFSCluster: "(Manual Restart of NFS PODS required)", "" except Exception as e: return exception_handler(e, f"Resetting NFS-Ganesha Config failed for {cluster_id}") + + def _rados(self, cluster_id: str) -> NFSRados: + """Return a new NFSRados object for the given cluster id.""" + return NFSRados(self.mgr.rados, cluster_id) diff --git a/src/pybind/mgr/nfs/export.py b/src/pybind/mgr/nfs/export.py index e864c7e366087..96a4863da622f 100644 --- a/src/pybind/mgr/nfs/export.py +++ b/src/pybind/mgr/nfs/export.py @@ -4,7 +4,7 @@ import logging from typing import List, Any, Dict, Tuple, Optional, TYPE_CHECKING, TypeVar, Callable, cast from os.path import normpath -from rados import TimedOut, ObjectNotFound +from rados import TimedOut, ObjectNotFound, Rados from mgr_module import NFS_POOL_NAME as POOL_NAME, NFS_GANESHA_SUPPORTED_FSALS @@ -46,8 +46,8 @@ def exception_handler( class NFSRados: - def __init__(self, mgr: 'Module', namespace: str) -> None: - self.mgr = mgr + def __init__(self, rados: 'Rados', namespace: str) -> None: + self.rados = rados self.pool = POOL_NAME self.namespace = namespace @@ -58,7 +58,7 @@ class NFSRados: return RawBlock('%url', values={'value': self._make_rados_url(obj_name)}) def write_obj(self, conf_block: str, obj: str, config_obj: str = '') -> None: - with self.mgr.rados.open_ioctx(self.pool) as ioctx: + with self.rados.open_ioctx(self.pool) as ioctx: ioctx.set_namespace(self.namespace) ioctx.write_full(obj, conf_block.encode('utf-8')) if not config_obj: @@ -74,7 +74,7 @@ class NFSRados: log.debug("Added %s url to %s", obj, config_obj) def read_obj(self, obj: str) -> Optional[str]: - with self.mgr.rados.open_ioctx(self.pool) as ioctx: + with self.rados.open_ioctx(self.pool) as ioctx: ioctx.set_namespace(self.namespace) try: return ioctx.read(obj, 1048576).decode() @@ -82,7 +82,7 @@ class NFSRados: return None def update_obj(self, conf_block: str, obj: str, config_obj: str) -> None: - with self.mgr.rados.open_ioctx(self.pool) as ioctx: + with self.rados.open_ioctx(self.pool) as ioctx: ioctx.set_namespace(self.namespace) ioctx.write_full(obj, conf_block.encode('utf-8')) log.debug("write configuration into rados object %s/%s/%s", @@ -91,7 +91,7 @@ class NFSRados: log.debug("Update export %s in %s", obj, config_obj) def remove_obj(self, obj: str, config_obj: str) -> None: - with self.mgr.rados.open_ioctx(self.pool) as ioctx: + with self.rados.open_ioctx(self.pool) as ioctx: ioctx.set_namespace(self.namespace) export_urls = ioctx.read(config_obj) url = '%url "{}"\n\n'.format(self._make_rados_url(obj)) @@ -102,13 +102,13 @@ class NFSRados: log.debug("Object deleted: %s", url) def remove_all_obj(self) -> None: - with self.mgr.rados.open_ioctx(self.pool) as ioctx: + with self.rados.open_ioctx(self.pool) as ioctx: ioctx.set_namespace(self.namespace) for obj in ioctx.list_objects(): obj.remove() def check_user_config(self) -> bool: - with self.mgr.rados.open_ioctx(self.pool) as ioctx: + with self.rados.open_ioctx(self.pool) as ioctx: ioctx.set_namespace(self.namespace) for obj in ioctx.list_objects(): if obj.key.startswith("userconf-nfs"): @@ -256,7 +256,7 @@ class ExportMgr: def _save_export(self, cluster_id: str, export: Export) -> None: self.exports[cluster_id].append(export) - NFSRados(self.mgr, cluster_id).write_obj( + self._rados(cluster_id).write_obj( GaneshaConfParser.write_block(export.to_export_block()), f'export-{export.export_id}', f'conf-nfs.{export.cluster_id}' @@ -277,7 +277,7 @@ class ExportMgr: if export: if pseudo_path: - NFSRados(self.mgr, cluster_id).remove_obj( + self._rados(cluster_id).remove_obj( f'export-{export.export_id}', f'conf-nfs.{cluster_id}') self.exports[cluster_id].remove(export) self._delete_export_user(export) @@ -306,7 +306,7 @@ class ExportMgr: def _update_export(self, cluster_id: str, export: Export) -> None: self.exports[cluster_id].append(export) - NFSRados(self.mgr, cluster_id).update_obj( + self._rados(cluster_id).update_obj( GaneshaConfParser.write_block(export.to_export_block()), f'export-{export.export_id}', f'conf-nfs.{export.cluster_id}') @@ -748,3 +748,7 @@ class ExportMgr: restart_nfs_service(self.mgr, new_export.cluster_id) return 0, f"Updated export {new_export.pseudo}", "" + + def _rados(self, cluster_id: str) -> NFSRados: + """Return a new NFSRados object for the given cluster id.""" + return NFSRados(self.mgr.rados, cluster_id) -- 2.39.5