From: Rishabh Dave <ridave@redhat.com>
Date: Fri, 30 Jul 2021 16:00:02 +0000 (+0530)
Subject: qa/vstart_runner: don't use "shell=False" in run_ceph_w()
X-Git-Tag: v17.1.0~995^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=047c90f881d362d349dc813691962665c918a83f;p=ceph.git

qa/vstart_runner: don't use "shell=False" in run_ceph_w()

Instead prepend "exec sudo" to the command arguments of
LocalCephManager.run_ceph_w(). This makes the default parameter
"shell=False" redundant in case of
ceph_manager.CephManager.run_ceph_w(), so get rid of it too and update
calls to run_ceph_w() accordingly.

The reason behind using any of these workarounds is that running "ceph
-w" with "shell" set to True leads to crash for Ceph API CI job. See
this ticket for more details: https://tracker.ceph.com/issues/49644.

The reason behind switching the workaround is that in the following
commits to reduce duplication LocalCephManager.run_ceph_w() will be
deleted and CephManager.run_ceph_w() will be used by LocalCephManager
via inheritance. However, due to the issue described above, Ceph API
test will fail since "shell" is set to "True" for the command issued by
CephManager.run_ceph_w(). Prepending "exec sudo" to the command when it
is used in LocalCephManager makes this duplication unnecessary and also
prevents Ceph API test from failing.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
---

diff --git a/qa/tasks/ceph_manager.py b/qa/tasks/ceph_manager.py
index 15ed5d08b97..3dca41832ed 100644
--- a/qa/tasks/ceph_manager.py
+++ b/qa/tasks/ceph_manager.py
@@ -1594,14 +1594,7 @@ class CephManager:
         kwargs['check_status'] = False
         return self.run_cluster_cmd(**kwargs).exitstatus
 
-    # XXX: Setting "shell" to True for LocalCephManager.run_ceph_w(), doesn't
-    # work with vstart_runner.py; see https://tracker.ceph.com/issues/49644.
-    # shell=False as default parameter is just to maintain compatibility
-    # between interfaces of CephManager.run_ceph_w() and
-    # LocalCephManager.run_ceph_w(). This doesn't affect how "ceph -w" process
-    # is launched by this method since this parameters remains unused in
-    # this method.
-    def run_ceph_w(self, watch_channel=None, shell=False):
+    def run_ceph_w(self, watch_channel=None):
         """
         Execute "ceph -w" in the background with stdout connected to a BytesIO,
         and return the RemoteProcess.
diff --git a/qa/tasks/ceph_test_case.py b/qa/tasks/ceph_test_case.py
index 2ca17b34ce3..7040853dfb4 100644
--- a/qa/tasks/ceph_test_case.py
+++ b/qa/tasks/ceph_test_case.py
@@ -110,10 +110,7 @@ class CephTestCase(unittest.TestCase):
                 return found
 
             def __enter__(self):
-                # XXX: For reason behind setting "shell" to False, see
-                # https://tracker.ceph.com/issues/49644.
-                self.watcher_process = ceph_manager.run_ceph_w(watch_channel,
-                                                               shell=False)
+                self.watcher_process = ceph_manager.run_ceph_w(watch_channel)
 
             def __exit__(self, exc_type, exc_val, exc_tb):
                 if not self.watcher_process.finished:
diff --git a/qa/tasks/vstart_runner.py b/qa/tasks/vstart_runner.py
index 851816b2c3d..5b68167b0b6 100644
--- a/qa/tasks/vstart_runner.py
+++ b/qa/tasks/vstart_runner.py
@@ -795,20 +795,27 @@ class LocalCephManager(CephManager):
         """
         return LocalRemote()
 
-    # XXX: For reason behind setting "shell" to False, see
-    # https://tracker.ceph.com/issues/49644.
-    def run_ceph_w(self, watch_channel=None, shell=False):
+    def run_ceph_w(self, watch_channel=None):
         """
         :param watch_channel: Specifies the channel to be watched.
                               This can be 'cluster', 'audit', ...
         :type watch_channel: str
         """
-        args = [CEPH_CMD, "-w"]
+        # XXX: Ceph API test CI job crashes because "ceph -w" process launched
+        # by run_ceph_w() crashes when shell is set to True.
+        # See https://tracker.ceph.com/issues/49644.
+        #
+        # The 2 possible workaround this are either setting "shell" to "False"
+        # when command "ceph -w" is executed or to prepend "exec sudo" to
+        # command arguments. We are going with latter since former would make
+        # it necessary to pass "shell" parameter to run() method. This leads
+        # to incompatibility with the method teuthology.orchestra.run's run()
+        # since it doesn't accept "shell" as parameter.
+        args = ['exec', 'sudo', CEPH_CMD, "-w"]
         if watch_channel is not None:
             args.append("--watch-channel")
             args.append(watch_channel)
-        proc = self.controller.run(args=args, wait=False, stdout=StringIO(),
-                                   shell=shell)
+        proc = self.controller.run(args=args, wait=False, stdout=StringIO())
         return proc
 
     def run_cluster_cmd(self, **kwargs):