Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve and standardize low-fi output prefixes (MINIKUBE_IN_STYLE=0) #4157

Closed
jakebasile opened this issue Apr 25, 2019 · 10 comments
Closed
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@jakebasile
Copy link

Moved from #3724 (comment)

The change to logging has cluttered output with worthless characters, or even worse emoji.

Please add an environment variable to shut this off.

Example:

$ minikube start --container-runtime=cri-o --cache-images
o   minikube v0.34.1 on linux (amd64)
$   Caching images in the background ...
>   Creating virtualbox VM (CPUs=2, Memory=2048MB, Disk=20000MB) ...
-   "minikube" IP address is 192.168.99.100
-   Configuring CRI-O as the container runtime ...
-   Preparing Kubernetes environment ...
:   Waiting for image caching to complete ...
-   Pulling images required by Kubernetes v1.13.3 ...
-   Launching Kubernetes v1.13.3 using kubeadm ... 
-   Configuring cluster permissions ...
-   Verifying component health .....
+   kubectl is now configured to use "minikube"
=   Done! Thank you for using minikube!

should be:

$ export MINIKUBE_REASONABLE_OUTPUT=true
$ minikube start --container-runtime=cri-o --cache-images
minikube v0.34.1 on linux (amd64)
Caching images in the background ...
Creating virtualbox VM (CPUs=2, Memory=2048MB, Disk=20000MB) ...
"minikube" IP address is 192.168.99.100
Configuring CRI-O as the container runtime ...
Preparing Kubernetes environment ...
Waiting for image caching to complete ...
Pulling images required by Kubernetes v1.13.3 ...
Launching Kubernetes v1.13.3 using kubeadm ... 
Configuring cluster permissions ...
Verifying component health .....
kubectl is now configured to use "minikube"
Done! Thank you for using minikube!

These symbols provide no useful information and clutter the output.

@tstromberg tstromberg self-assigned this Apr 25, 2019
@tstromberg tstromberg changed the title Remove Extraneous Output Improve and standardize low-fi output prefixes (MINIKUBE_IN_STYLE=0) Apr 25, 2019
@tstromberg
Copy link
Contributor

I agree that the ASCII output currently is not very good. Here's some features that I think should be improved and/or preserved:

  • A visible difference between two separate events and a single multi-line string (kubeadm errors, for instance)
  • Clear difference between between info, errors, and warning messages
  • Support for indented items

Here's a straw-man proposal, though I'm definitely flexible on what characters are used so long as we can preserve those goals.

Failed start:

* minikube v1.0.0 on linux (amd64)
* Downloading Kubernetes v1.14.1 images in the background ...
* Creating virtualbox VM (CPUs=2, Memory=2,048MB, Disk=20,000MB) ...

X Unable to start VM
* Error:         [VBOX_NOT_FOUND] create: precreate: VBoxManage not found. Make sure VirtualBox is installed and VBoxManage is in the path
* Advice:        Install VirtualBox, ensure that VBoxManage is executable and in path, or select an alternative value for --vm-driver
* Documentation: https://www.virtualbox.org/wiki/Downloads
* Related issues:
  - https://github.com/kubernetes/minikube/issues/3,784
  - https://github.com/kubernetes/minikube/issues/3,776

* If the above advice does not help, please let us know: 
  - https://github.com/kubernetes/minikube/issues/new

Successful start:

* minikube v1.0.0 on linux (amd64)
* Downloading Kubernetes v1.14.1 images in the background ...
* Creating kvm2 VM (CPUs=2, Memory=2,048MB, Disk=20,000MB) ...
* "minikube" IP address is 192.168.39.141
* Configuring Docker as the container runtime ...
* Version of container runtime is 18.06.2-ce
* Waiting for image downloads to complete ...
* Preparing Kubernetes environment ...
* Pulling images required by Kubernetes v1.14.1 ...
* Launching Kubernetes v1.14.1 using kubeadm ... 
* Waiting for pods: apiserver proxy etcd scheduler controller dns
* Configuring cluster permissions ...
* Verifying component health .....
* kubectl is now configured to use "minikube"
* Done! Thank you for using minikube!

Any thoughts?

@Th3G4mbl3r
Copy link

looks like a much more workable option as compared to current emjoi based output...

@afbjorklund
Copy link
Collaborator

Should info/warning/error be an explicit severity argument, and then have style as secondary ? (perhaps grouped into category)

@jakebasile
Copy link
Author

Personally, I prefer no prefixes whatsoever. They break up my ability to scan output. However, if that is not an option #4157 (comment) would be better than what we have now.

@tstromberg
Copy link
Contributor

Thanks for the feedback!

I've mocked up 4 example styles for the 1.1 release. Could folks interested in this issue please vote on their top two styles? https://forms.gle/KjFG9ahgqURNr6k78

@jakebasile / @Th3G4mbl3r / @afbjorklund - I'd really love to get your feedback in particular.

@afbjorklund
Copy link
Collaborator

@tstromberg The election seems somewhat rigged, but I voted for the simplest formats.

My preference is either go all in on the Mac emojis (color and all), or just drop the prefix.

I think my feedback was the $MINIKUBE_NO_STYLE, which also avoided using booleans.

But I can live with the black-and-white emoji or with the bullets, it's not super-important...


Could still see color being used for errors (e.g. red) or a category (like in our google logging)

Then again those also frequently act up, and you are left with garbage. (See $NO_COLOR)

@gbraad
Copy link
Contributor

gbraad commented May 1, 2019 via email

@afbjorklund
Copy link
Collaborator

Sorry, what part of the ASCII output got fixed by libmachine/driver ?

@tstromberg tstromberg added kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels May 2, 2019
@jakebasile
Copy link
Author

jakebasile commented May 4, 2019

I voted for the "dash" option, but as @afbjorklund stated it's kind of a rigged vote, since my preference is nothing at all, not even indentation.

Any prefix symbol makes it harder for me to read because it is so unlike all other terminal tools I use. As for the indents, they seem to just be a way to split what should be in one atomic line into multiple.

What value is provided by these prefixes? The fact that it is a new line is already obvious since it is a new line. It'd be like me wearing a nametag that says "I am a human" when you can just look at me and see that I am a human. A new output line is indicated by a new output line.

@tstromberg
Copy link
Contributor

Closed by #4162 - will be part of next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests

5 participants