kubecolor
kubectl's terminal line wrap skipped when output via kubecolor
kubectl output wraps lines to the lesser of terminal width or 120 chars, also properly managing indent of the wrapped lines.
However when invoked via kubecolor, this wrapping is not present. All lines are infinite width, which makes the output "pretty" (with colors) but unwieldy and difficult to read, defeating the purpose. kubecolor should only add colorization, not remove wrapping.
This is easily seen with kubectl drain --help, compare raw kubectl output to kubecolor and you will see it.
$ kubecolor version --client
Client Version: v1.34.1
Kustomize Version: v5.7.1
Kubecolor Version: 0.5.1
I'm guessing that the kubectl output is connected to kubectl with a file descriptor which is not a terminal, so kubectl does ioctl(TCGETS) (ie like isatty(3) in C), which fails when wrapped by kubecolor, meaning "this isn't a terminal" so it does no wrapping (it could not get terminal width, etc).
To test this I traced it with $prog as both kubecolor and kubectl like so:
strace -f -qqq -o $prog.out -e 's=!all' -e ioctl $prog drain --help
and got:
$ tail -n +1 kube*.out | fmt -t | grep -P '^\S'
==> kubecolor.out <==
3263723 ioctl(1, TCGETS, {c_iflag=ICRNL,
3263729 ioctl(2, TCGETS, 0xc00016d320) = -1 ENOTTY (Inappropriate ioctl
3263729 ioctl(2, TCGETS, 0xc00016d380) = -1 ENOTTY (Inappropriate ioctl
3263729 ioctl(1, TCGETS, 0xc00016cf00) = -1 ENOTTY (Inappropriate ioctl
3263729 ioctl(1, TCGETS, 0xc000862000) = -1 ENOTTY (Inappropriate ioctl
==> kubectl.out <==
3263767 ioctl(2, TCGETS, {c_iflag=ICRNL,
3263767 ioctl(2, TCGETS, {c_iflag=ICRNL,
3263772 ioctl(1, TCGETS, {c_iflag=ICRNL,
3263772 ioctl(1, TIOCGWINSZ, {ws_row=44, ws_col=159, ws_xpixel=3816,
3263772 ioctl(1, TCGETS, {c_iflag=ICRNL,
3263772 ioctl(1, TIOCGWINSZ, {ws_row=44, ws_col=159, ws_xpixel=3816,
So, kubectl is testing whether stdout is a terminal, and if so, gets the window size using TIOCGWINSZ and wraps accordingly.
A solution for this might be for kubecolor to connect to kubectl using a pty master/slave pair and then kubectl's ioctl() calls would succeed and it would do the wrap. Whether that would mess up the "parsing" in kubecolor, I don't know, as I don't know the mechanism used (are they regex tables? a grammar lex?), but certainly single lines are a lot simpler to colorize I think, so it might be tricky depending on the current implementation.
Nice debugging.
Its somewhat common to also listen for the COLUMNS env var, so that might be a decent workaround, where kubecolor could get the term width and set the env var accordingly
Ah damn, no they don't have a fallback to env vars. Shame...
Call stack for reference:
- https://github.com/kubernetes/kubectl/blob/d1b399db575d799c1e4be20ee9a293d7110fb7d8/pkg/util/templates/templater.go#L234
- https://github.com/kubernetes/kubectl/blob/d1b399db575d799c1e4be20ee9a293d7110fb7d8/pkg/util/term/term_writer.go#L85-L88
- https://github.com/moby/term/blob/6c1b69fecbac2753dcaf18718a7e9f9093c3760d/term.go#L44-L45
- https://github.com/moby/term/blob/6c1b69fecbac2753dcaf18718a7e9f9093c3760d/term_unix.go#L53-L54
- https://github.com/moby/term/blob/6c1b69fecbac2753dcaf18718a7e9f9093c3760d/term_unix.go#L88-L89
- https://cs.opensource.google/go/x/sys/+/refs/tags/v0.37.0:unix/ioctl_unsigned.go;l=65-67
I don't think kubecolor cleans the environment, it would get copied by execve(), and $COLUMNS is likely already available to kubectl. The latter is using an ioctl to directly get the terminal size. It would be good if we could set the wrap width for kubectl manually with a --width flag or something, but that doesn't appear to be implemented that I can find.
Since kubectl acts differently depending on whether the output is a terminal, presuming the output is post-processed programmatically if output isn't a tty, kubecolor maybe should do the same (since it is a wrapper) and use pty slave descriptors as the stdio for its kubectl invocation, if its own stdout is a tty.
Personally I've not done that before so if you want to take the lead then our PRs are open
I have done it in C and Python but rather new to Golang. I also couldn't do it for quite some time. Maybe eventually... we can leave this open. At least we know what's going on...
@applejag can I ask though, if kubecolor starts seeing multiple input lines for things that got wrapped, but are logically part of the same "virtual line," is this going to mess up its parsing code, which at the moment can safely assume everything is on one line? Implementing the pty itself is somewhat easy but teaching the parser about multiple line "records" would be more difficult.
The parser used on help text checks the indentation and groups by that. It's also the same parser we use for the describe output, which is also indentation based, as well as having some other finicky formats.
As long as the text is indented then it should be fine. Otherwise we can just modify the parsers too. It's our own parsers.
If you could post a mockup/proof-of-concept of how to handle the pty in C or Python then I that's a great start and then I can help look into porting it to our code base
If you could post a mockup/proof-of-concept of how to handle the pty in C or Python
Python apparently doesn't copy the window or termios attributes to the pty slave. When I had used it previously, the window size wasn't in play, so it never came up. Even using os.openpty you cannot pass the termios/winsize, but it can probably be done in the fork manually. python/cpython#85984 will expose it in os.openpty and even the simpler pty.spawn(), but that isn't merged yet.
I looked into doing it in kubecolor but noticed that in your command/runner.go function execWithReaders() you are using exec.Command().StdoutPipe() which probably means it's using pipe(2) syscall and that's definitely not going to be a terminal, it's just a simple half-duplex byte channel with no such thing as termios or winsize. I think it still might work to insert here, because we'd only have to capture the output from the pty complex, after kubectl had already written to the slave pty. However I wanted to prove the concept first as you suggested.
To prove the concept I wrote a C wrapper and the binary works fine when I use it as the kubectl value in ~/.kube/color.yml. I get full terminal wrapping preserved from kubectl and colorized by kubecolor, best of both worlds. The C interface is documented in man 7 pty and man 3 openpty. Here's the code:
/*
* wrap kubectl with pty, for use in pipeline with terminal formatting
*/
#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <termios.h>
#include <string.h>
#include <errno.h>
#include <pty.h>
#include <sys/ioctl.h>
#include <sys/wait.h>
extern char **environ;
static char *kubectlbin = "/bin/kubectl";
int
main(int argc, char **argv)
{
int fd, status;
char buf[BUFSIZ];
struct termios termattrs;
struct winsize winattrs;
ssize_t len;
pid_t pid;
int testfd = STDIN_FILENO;
if (tcgetattr(testfd, &termattrs) == -1) {
perror("tcgetattr");
goto fail;
}
if (ioctl(testfd, TIOCGWINSZ, &winattrs) == -1) {
perror("ioctl");
goto fail;
}
if ((pid = forkpty(&fd, NULL, &termattrs, &winattrs)) == -1) {
perror("forkpty");
goto fail;
}
if (!pid) {
argv[0] = strrchr(kubectlbin, '/') + 1;
execve(kubectlbin, argv, environ);
perror("execvp");
goto fail;
}
while ((len = read(fd, buf, sizeof(buf))) > 0) {
if (write(STDOUT_FILENO, buf, len) != len) {
perror("write");
close(fd);
goto fail;
}
}
if (len == -1 && errno != EIO)
perror("read");
close(fd);
waitpid(pid, &status, 0);
if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
return WEXITSTATUS(status);
if (WIFSIGNALED(status))
return 128 + WTERMSIG(status);
exit(EXIT_SUCCESS);
fail:
exit(EXIT_FAILURE);
}
I think the right way is probably to use https://github.com/creack/pty, that seems to be how pty stuff is done in golang, weird that it's not in standard golang libraries and need a third party lib for what is pretty standard stuff, but apparently that's the case.
Anyways I have a workaround now using this C-wrapper method. We can leave this issue open if you want; I won't be able to try implementing it myself for some time, but would probably get to it eventually. I think it's worthwhile, personally, but I also think you're right that kubectl should provide an override flag to force wrap (defaulting to $COLUMNS).
P.S. I had to use stdin, rather than stdout, to determine if invoked on a tty (unlike kubectl, which is testing stdout). If using stdout, the ioctl() failed. I think because stdout is a pipe when kubecolor invokes it. But for example if I made kubectlbin to be /bin/ls and passed --color=auto then I could make testfd to use stdout instead and it worked fine.
[In Python], even using os.openpty you cannot pass the termios/winsize, but it can probably be done in the fork manually
Just for completeness, I tried it. This does work too as kubectl bin in color.yml:
#!/usr/bin/env python3
import termios, sys, os, pty
from shutil import which
from os.path import basename
exepath = which('kubectl')
spawnv = [basename(exepath)] + sys.argv[1:]
stdinfd = sys.stdin.fileno()
winsize = termios.tcgetwinsize(stdinfd)
tattrs = termios.tcgetattr(stdinfd)
if not os.isatty(stdinfd):
os.execv(exepath, spawnv)
pid, masterfd = pty.fork()
if pid == pty.CHILD:
termios.tcsetwinsize(stdinfd, winsize)
termios.tcsetattr(stdinfd, termios.TCSANOW, tattrs)
os.execv(exepath, spawnv)
pty._copy(masterfd, pty._read, pty._read)
os.close(masterfd)
os.waitpid(pid, 0)[1]
but loading the python interpreter every time has a noticeable latency compared to the C wrapper.