From 2718b7ac6ca5d009d42d86eb7d0a55aa828fe976 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 8 Mar 2020 14:00:53 +0800 Subject: [PATCH] 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 (cherry picked from commit d8d44ed1566b19eec055e07da2a0fed88fed4152) --- qa/tasks/ceph_manager.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) 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" -- 2.47.3