Bug 1397995

Summary:	diagnostics coloring and indentation
Product:	OpenShift Container Platform	Reporter:	Luke Meyer <lmeyer>
Component:	oc	Assignee:	Fabiano Franz <ffranz>
Status:	CLOSED ERRATA	QA Contact:	Xia Zhao <xiazhao>
Severity:	low	Docs Contact:
Priority:	medium
Version:	3.4.0	CC:	aos-bugs, jokerman, mmccomas, tdawson
Target Milestone:	---
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
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.	Story Points:	---
Clone Of:		Environment:
Last Closed:	2017-04-12 19:17:17 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:

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