Bug 1397995 - diagnostics coloring and indentation
Summary: diagnostics coloring and indentation
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: OpenShift Container Platform
Classification: Red Hat
Component: oc
Version: 3.4.0
Hardware: Unspecified
OS: Unspecified
medium
low
Target Milestone: ---
: ---
Assignee: Fabiano Franz
QA Contact: Xia Zhao
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-11-23 18:52 UTC by Luke Meyer
Modified: 2017-07-24 14:11 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Cause: the new responsive terminal wraps long lines in the output of CLI commands. Consequence: 'oadm diagnostics' indentation is not great, and no longer has color in its output. Fix: bypass the responsive terminal in 'oadm diagnostics' (basically now only being used in CLI help output). Result: 'oadm diagnostics' has proper indentation and colorized output.
Clone Of:
Environment:
Last Closed: 2017-04-12 19:17:17 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2017:0884 0 normal SHIPPED_LIVE Red Hat OpenShift Container Platform 3.5 RPM Release Advisory 2017-04-12 22:50:07 UTC

Description Luke Meyer 2016-11-23 18:52:48 UTC
With the introduction of NewResponsiveWriter, oadm diagnostics no longer has color in its output, and long lines are wrapped which ruins the indentation scheme.

I would prefer to just bypass the new writer for actual diagnostic output and continue using the old one. I'm not familiar enough with the change to be sure but it sounds like from https://github.com/openshift/origin/pull/11680 I can just pass os.Stdout instead of the writer that's used for the command. The command can continue to pass the new writer to cobra and just use os.Stdout for intended output.

BTW, the reason color is turned off is that the new writer is not a terminal, according to IsTerminal https://github.com/openshift/origin/blob/65a46c858582ee166ad183dda06069be1a40db07/pkg/diagnostics/log/text.go#L29-L32 - in case that matters anywhere else, you might want to have NewResponsiveWriter implement whatever it needs to in order to be accurately considered a terminal based on the stdout.

Comment 1 Fabiano Franz 2016-11-24 18:44:47 UTC
Fixed in https://github.com/openshift/origin/pull/12023

Comment 2 Fabiano Franz 2016-11-24 18:45:17 UTC
Fixed in https://github.com/openshift/origin/pull/12023

Comment 3 Xia Zhao 2016-11-25 08:26:41 UTC
Verify passed on origin, the cli is shown with colors. 

# openshift version
openshift v1.5.0-alpha.0+e115e25-81
kubernetes v1.4.0+776c994
etcd 3.1.0-rc.0

However, the fix was not included by the latest puddle of OCP, repro on:

# openshift version
openshift v3.4.0.29+ca980ba
kubernetes v1.4.0+776c994
etcd 3.1.0-rc.0

Keep this bz ON_QA until PR is merged in OCP.

Comment 4 Fabiano Franz 2016-11-25 19:18:01 UTC
We're not planning a backport of this fix to OCP 3.4 given the "low" severity. You should only expect for this in 3.5 as specified in "Target Release".

Dan and/or Luke, feel free to disagree.

Comment 5 Xia Zhao 2016-11-29 02:54:05 UTC
Set to modified according to comment #3.

Comment 6 Troy Dawson 2017-01-20 23:23:01 UTC
This has been merged into ocp and is in OCP v3.5.0.7 or newer.

Comment 7 Xia Zhao 2017-01-22 02:24:36 UTC
Verified passed on OCP 3.5.0 , the cli is shown with colors:
# openshift version
openshift v3.5.0.7+390ef18
kubernetes v1.5.2+43a9be4
etcd 3.1.0-rc.0

Comment 9 errata-xmlrpc 2017-04-12 19:17:17 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2017:0884


Note You need to log in before you can comment on or make changes to this bug.