qa/tasks/ceph_manager: use StringIO for capturing COT output

there are couple factors we should consider when choosing between
BytesIO and StringIO:

- if the producer is producing binary
- if we are expecting binary
- if the layers in between them are doing the decoding/encoding
  automatically.

in our case, the producer is either the ChannelFile instances returned
by paramiko.SSHClient or subprocess.CompletedProcess insances returned
by subprocess.run(). the former are file-like objects opened in "r" mode,
but their contents are decoded with utf-8 when reading if
ChannelFile.FLAG_BINARY is not specified. that's why we always try to
add this flag in orchestra/run.py when collecting the stdout and stderr
from paramiko.SSHClient after executing a command.

back in python2, this works just fine. as we don't differentiate bytes
from str by then.

but in python3, we have to make a decision. in the case of
ceph-objectstore-tool (COT for short), it does not produce binary and
we don't check its output with binary, so, if neither Remote.run() nor
LocalRemote.run() decodes/encodes for us, it's fine.

so it boils down to `copy_to_log()`:

i think we we should respect the consumer's expectation, and only decode
the output if a StringIO is passed in as stdout or stderr.

as we always log the output with logging we could either set
`ChannelFile.FLAG_BINARY` depending on the type of `capture` or not.
if it's not set, paramiko will return str (bytes) on python2, and str on
python3. if it's not set paramiko will return str (bytes) on python2,
and bytes on python3.

if there is non-ASCII in the output, logging will bail fail with
`UnicodeDecodeError` exception. and paramiko throws the same exception
when trying to decode for us if `ChannelFile.FLAG_BINARY` is not
specified.

so to ensure that we always have logging messages no matter if the
producer follows the rule of "use StringIO if you only emit text" or
not, we have to use `ChannelFile.FLAG_BINARY`, and force paramiko
to send us the bytes. but we still have the luxury to use StringIO
and do the decode when the caller asks for str explicitly. that'd save
the pain of using `str.decode()` or `six.ensure_str()` everywhere
even if we can assure that the program does not write binary.

Signed-off-by: Kefu Chai <kchai@redhat.com>

diff --git a/qa/tasks/ceph_manager.py b/qa/tasks/ceph_manager.py
index f8afa98835d6e56ec8fb23530f75045d7dabed93..36d44259c9cdd941b93bd4c4fd4f61b042cf8f78 100644 (file)
--- a/qa/tasks/ceph_manager.py
+++ b/qa/tasks/ceph_manager.py
@@ -13,6 +13,7 @@ import logging
 import threading
 import traceback
 import os
+import six
 
 from io import BytesIO
 from six import StringIO
@@ -24,7 +25,6 @@ from teuthology.contextutil import safe_while
 from teuthology.orchestra.remote import Remote
 from teuthology.orchestra import run
 from teuthology.exceptions import CommandFailedError
-import six
 
 try:
     from subprocess import DEVNULL # py3k
@@ -193,6 +193,13 @@ class Thrasher:
                 return False;
         return True;
 
+    def run_ceph_objectstore_tool(self, remote, osd, cmd):
+        return remote.run(
+            args=['sudo', 'adjust-ulimits', 'ceph-objectstore-tool'] + cmd,
+            wait=True, check_status=False,
+            stdout=StringIO(),
+            stderr=StringIO())
+
     def kill_osd(self, osd=None, mark_down=False, mark_out=False):
         """
         :param osd: Osd to be killed.
@@ -259,7 +266,7 @@ class Thrasher:
                     if proc.exitstatus == 0:
                         break
                     elif (proc.exitstatus == 1 and
-                          six.ensure_str(proc.stderr.getvalue()) == "OSD has the store locked"):
+                          proc.stderr.getvalue() == "OSD has the store locked"):
                         continue
                     else:
                         raise Exception("ceph-objectstore-tool: "
@@ -326,7 +333,7 @@ class Thrasher:
                                   stderr=BytesIO())
             if proc.exitstatus == 1:
                 bogosity = "The OSD you are using is older than the exported PG"
-                if bogosity in six.ensure_str(proc.stderr.getvalue()):
+                if bogosity in proc.stderr.getvalue():
                     self.log("OSD older than exported PG"
                              "...ignored")
             elif proc.exitstatus == 10: