From: Kefu Chai <kchai@redhat.com>
Date: Sun, 8 Mar 2020 06:00:53 +0000 (+0800)
Subject: qa/tasks/ceph_manager: use StringIO for capturing COT output
X-Git-Tag: v15.2.8~12^2~2^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F37863%2Fhead;p=ceph.git

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>
(cherry picked from commit d8d44ed1566b19eec055e07da2a0fed88fed4152)
---

diff --git a/qa/tasks/ceph_manager.py b/qa/tasks/ceph_manager.py
index 6a49bcb86689..dc7ff02e50c6 100644
--- 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, StringIO
 from teuthology import misc as teuthology
@@ -305,7 +306,7 @@ class OSDThrasher(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: "
@@ -417,7 +418,7 @@ class OSDThrasher(Thrasher):
                 ])
             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:
@@ -454,8 +455,8 @@ class OSDThrasher(Thrasher):
                            + " --op apply-layout-settings --pool " + pool).format(id=osd)
                     proc = imp_remote.run(args=cmd,
                                           wait=True, check_status=False,
-                                          stderr=BytesIO)
-                    if 'Couldn\'t find pool' in six.ensure_str(proc.stderr.getvalue()):
+                                          stderr=StringIO())
+                    if 'Couldn\'t find pool' in proc.stderr.getvalue():
                         continue
                     if proc.exitstatus:
                         raise Exception("ceph-objectstore-tool apply-layout-settings"