From: Sage Weil <sage@newdream.net>
Date: Fri, 29 Oct 2021 15:27:50 +0000 (-0400)
Subject: mgr/nfs/export: use tool_exec() instead of private _exec()
X-Git-Tag: v17.1.0~557^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=8dcdde71512f951e048dff5cd75cca5d0e4b5641;p=ceph.git

mgr/nfs/export: use tool_exec() instead of private _exec()

Fixes: https://tracker.ceph.com/issues/53096
Signed-off-by: Sage Weil <sage@newdream.net>
Signed-off-by: Michael Fritch <mfritch@suse.com>
---

diff --git a/src/pybind/mgr/nfs/export.py b/src/pybind/mgr/nfs/export.py
index 7c2666bf19550..b23a95e04632d 100644
--- a/src/pybind/mgr/nfs/export.py
+++ b/src/pybind/mgr/nfs/export.py
@@ -1,7 +1,6 @@
 import errno
 import json
 import logging
-import subprocess
 from typing import List, Any, Dict, Tuple, Optional, TYPE_CHECKING, TypeVar, Callable, cast
 from os.path import normpath
 
@@ -134,26 +133,6 @@ class ExportMgr:
         except TimedOut:
             log.exception("Ganesha timed out")
 
-    def _exec(self, args: List[str]) -> Tuple[int, str, str]:
-        try:
-            util = args.pop(0)
-            cmd = [
-                util,
-                '-k', str(self.mgr.get_ceph_option('keyring')),
-                '-n', f'mgr.{self.mgr.get_mgr_id()}',
-            ] + args
-            log.debug('exec: ' + ' '.join(cmd))
-            p = subprocess.run(
-                cmd,
-                stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                timeout=10,
-            )
-        except subprocess.CalledProcessError as ex:
-            log.error('Error executing <<%s>>: %s', ex.cmd, ex.output)
-        except subprocess.TimeoutExpired:
-            log.error('timeout (10s) executing <<%s>>', cmd)
-        return p.returncode, p.stdout.decode(), p.stderr.decode()
-
     @property
     def exports(self) -> Dict[str, List[Export]]:
         if self._exports is None:
@@ -226,14 +205,15 @@ class ExportMgr:
 
         elif isinstance(export.fsal, RGWFSAL):
             rgwfsal = cast(RGWFSAL, export.fsal)
-            ret, out, err = self._exec(['radosgw-admin', 'bucket', 'stats', '--bucket',
-                                        export.path])
+            ret, out, err = self.mgr.tool_exec(
+                ['radosgw-admin', 'bucket', 'stats', '--bucket', export.path]
+            )
             if ret:
                 raise NFSException(f'Failed to fetch owner for bucket {export.path}')
             j = json.loads(out)
             owner = j.get('owner', '')
             rgwfsal.user_id = owner
-            ret, out, err = self._exec([
+            ret, out, err = self.mgr.tool_exec([
                 'radosgw-admin', 'user', 'info', '--uid', owner
             ])
             if ret:
@@ -585,7 +565,8 @@ class ExportMgr:
         ex_dict["cluster_id"] = cluster_id
         export = Export.from_dict(ex_id, ex_dict)
         export.validate(self.mgr)
-        log.debug("Successfully created %s export-%s from dict for cluster %s", fsal_type, ex_id, cluster_id)
+        log.debug("Successfully created %s export-%s from dict for cluster %s",
+                  fsal_type, ex_id, cluster_id)
         return export
 
     def create_cephfs_export(self,
diff --git a/src/pybind/mgr/nfs/tests/test_nfs.py b/src/pybind/mgr/nfs/tests/test_nfs.py
index d6f73ae456e65..6fc4246e95880 100644
--- a/src/pybind/mgr/nfs/tests/test_nfs.py
+++ b/src/pybind/mgr/nfs/tests/test_nfs.py
@@ -6,7 +6,7 @@ from typing import Optional, Tuple, Iterator, List, Any
 from contextlib import contextmanager
 from unittest import mock
 from unittest.mock import MagicMock
-from mgr_module import NFS_POOL_NAME
+from mgr_module import MgrModule, NFS_POOL_NAME
 
 from rados import ObjectNotFound
 
@@ -222,7 +222,7 @@ EXPORT {
                            return_value=[self.cluster_id]), \
                 mock.patch('nfs.export.restart_nfs_service'), \
                 mock.patch('nfs.cluster.restart_nfs_service'), \
-                mock.patch('nfs.export.ExportMgr._exec', mock_exec), \
+                mock.patch.object(MgrModule, 'tool_exec', mock_exec), \
                 mock.patch('nfs.export.check_fs', return_value=True), \
                 mock.patch('nfs.export_utils.check_fs', return_value=True), \
                 mock.patch('nfs.export.ExportMgr._create_user_key',