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

Chore: set CLI verbose output to be hidden by default #6016

Merged
merged 1 commit into from
May 30, 2023

Conversation

Somefive
Copy link
Collaborator

@Somefive Somefive commented May 22, 2023

Description of your changes

Add "--verbosity"/"-V" global flag for vela cli. By default 0.

When 0, all "klog.Info" will be omitted. "klog.Error" will be written to stderr.

image

When greater than 0, "klog.Info" will be written to stdout.

image

When greater than 3, the call file and time will be printed.

image

When greater than 6, all details will be printed.

image

Fixes #5561

I have:

  • Read and followed KubeVela's contribution process.
  • Related Docs updated properly. In a new feature or configuration option, an update to the documentation is necessary.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Special notes for your reviewer

@codecov
Copy link

codecov bot commented May 22, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.85 🎉

Comparison is base (8818f54) 60.12% compared to head (625d6a8) 60.98%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6016      +/-   ##
==========================================
+ Coverage   60.12%   60.98%   +0.85%     
==========================================
  Files         225      225              
  Lines       31349    31349              
==========================================
+ Hits        18849    19118     +269     
+ Misses      10706    10461     -245     
+ Partials     1794     1770      -24     
Flag Coverage Δ
core-unittests 56.19% <ø> (-0.07%) ⬇️
e2e-multicluster-test 25.05% <ø> (-0.02%) ⬇️
e2etests 24.47% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 31 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@chivalryq
Copy link
Member

Consider add a short version. Say -V? WDYT

@Somefive
Copy link
Collaborator Author

Consider add a short version. Say -V? WDYT

The "-v" is used by some commands, like vela install. Upper case is okay for me, but not aligned with the kubectl -v. /cc @wonderflow @wangyikewxgm WDYT

@wonderflow
Copy link
Collaborator

vela --verbose is better? -V is a bit wield

@Somefive
Copy link
Collaborator Author

vela --verbose is better? -V is a bit wield

This is how "KinD" use the flag.

kind creates and manages local Kubernetes clusters using Docker container 'nodes'

Usage:
  kind [command]

Available Commands:
  build       Build one of [node-image]
  completion  Output shell completion code for the specified shell (bash, zsh or fish)
  create      Creates one of [cluster]
  delete      Deletes one of [cluster]
  export      Exports one of [kubeconfig, logs]
  get         Gets one of [clusters, nodes, kubeconfig]
  help        Help about any command
  load        Loads images into nodes
  version     Prints the kind CLI version

Flags:
  -h, --help              help for kind
      --loglevel string   DEPRECATED: see -v instead
  -q, --quiet             silence all stderr output
  -v, --verbosity int32   info log verbosity, higher value produces more output
      --version           version for kind

This is k3d

https://k3d.io/
k3d is a wrapper CLI that helps you to easily create k3s clusters inside docker.
Nodes of a k3d cluster are docker containers running a k3s image.
All Nodes of a k3d cluster are part of the same docker network.

Usage:
  k3d [flags]
  k3d [command]

Available Commands:
  cluster      Manage cluster(s)
  completion   Generate completion scripts for [bash, zsh, fish, powershell | psh]
  config       Work with config file(s)
  help         Help about any command
  image        Handle container images.
  kubeconfig   Manage kubeconfig(s)
  node         Manage node(s)
  registry     Manage registry/registries
  version      Show k3d and default k3s version

Flags:
  -h, --help         help for k3d
      --timestamps   Enable Log timestamps
      --trace        Enable super verbose output (trace logging)
      --verbose      Enable verbose output (debug logging)
      --version      Show k3d and default k3s version

This is kubectl.

    --tls-server-name='':
	Server name to use for server certificate validation. If it is not provided, the hostname used to contact the
	server is used

    --token='':
	Bearer token for authentication to the API server

    --user='':
	The name of the kubeconfig user to use

    --username='':
	Username for basic authentication to the API server

    -v, --v=0:
	number for the log level verbosity

    --vmodule=:
	comma-separated list of pattern=N settings for file-filtered logging (only works for the default text log
	format)

    --warnings-as-errors=false:
	Treat warnings received from the server as errors and exit with a non-zero exit code

So, if we would like to use the verbose level, I think verbosity is the best choice. It would be good if we use -v for short name but that could cause a conflict with other commands using -v for version.

It you prefer --verbose, then it should be a boolean flag and we might also need --trace for it. Less configuration capability and less confusion. The advantage of using --verbosity is that it is aligned with all other Kubernetes project's internal logging behaviour. The advantage of using --verbose is that it is easier for user to understand and use.

@wonderflow @FogDong WDYT

@wonderflow
Copy link
Collaborator

Great, verbosity +1, suppurt --verbosity and -V is fine.

Signed-off-by: Somefive <yd219913@alibaba-inc.com>
@Somefive
Copy link
Collaborator Author

Great, verbosity +1, suppurt --verbosity and -V is fine.

Done

@chivalryq chivalryq merged commit 057441b into kubevela:master May 30, 2023
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide the output of the warning log while the CLI tool is working
3 participants