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

Add json output for status #5611

Merged
merged 4 commits into from
Oct 22, 2019

Conversation

woodcockjosh
Copy link
Contributor

@woodcockjosh woodcockjosh commented Oct 13, 2019

Adds json output for minikube status command
Adds shorthand argument for format option

Before

Command:

minikube status -o json

Output:

Error: unknown shorthand flag: 'o' in -o


Options:
      --format='host: {{.Host}}
kubelet: {{.Kubelet}}
apiserver: {{.APIServer}}
kubectl: {{.Kubeconfig}}
': Go template format string for the status output.  The format for Go templates can be found here: https://golang.org/pkg/text/template/
For the list accessible variables for the template, see the struct values here: https://godoc.org/k8s.io/minikube/cmd/minikube/cmd#Status

Usage:
  minikube status [flags] [options]

Use "minikube status options" for a list of global command-line options (applies to all commands).

After

Adds json output

Command:

minikube status -o json

Output:

{"APIServer":"Running","Host":"Running","Kubeconfig":{"Correct":true,"IP":"192.168.99.189"},"Kubelet":"Running"}

Adds shorthand for format option

Command:

minikube status --help

Output:

Gets the status of a local kubernetes cluster.
        Exit status contains the status of minikube's VM, cluster and kubernetes encoded on it's bits in this order from right
to left.
        Eg: 7 meaning: 1 (for minikube NOK) + 2 (for cluster NOK) + 4 (for kubernetes NOK)

Options:
  -f, --format='host: {{.Host}}
kubelet: {{.Kubelet}}
apiserver: {{.APIServer}}
kubectl: {{.Kubeconfig}}
': Go template format string for the status output.  The format for Go templates can be found here:
https://golang.org/pkg/text/template/
For the list accessible variables for the template, see the struct values here:
https://godoc.org/k8s.io/minikube/cmd/minikube/cmd#Status
  -o, --output='text': minikube status --output OUTPUT. json, text

Usage:
  minikube status [flags] [options]

Use "minikube status options" for a list of global command-line options (applies to all commands).

Output and Format options cannot both be used at the same time

Command:

minikube status -o json -f "hello"

Output:

💡  Cannot use both --output and --format options

Fixes #5080

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 13, 2019
@k8s-ci-robot
Copy link
Contributor

Hi @woodcockjosh. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 13, 2019
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@woodcockjosh
Copy link
Contributor Author

/assign @medyagh

@TravisBuddy
Copy link

Travis tests have failed

Hey @woodcockjosh,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

1st Build

View build log

make test
which go-bindata || GO111MODULE=off GOBIN=/home/travis/gopath/bin go get github.com/jteeuwen/go-bindata/...
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.12.9.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.0/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/assets/assets.go -pkg assets deploy/addons/...
gofmt -s -w pkg/minikube/assets/assets.go
which go-bindata || GO111MODULE=off GOBIN=/home/travis/gopath/bin go get github.com/jteeuwen/go-bindata/...
/home/travis/gopath/bin/go-bindata
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.12.9.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.0/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/translate/translations.go -pkg translate translations/...
gofmt -s -w pkg/minikube/translate/translations.go
./test.sh
= make lint =============================================================
golangci/golangci-lint info checking GitHub for tag 'v1.20.0'
golangci/golangci-lint info found version: 1.20.0 for v1.20.0/linux/amd64
golangci/golangci-lint info installed out/linters/golangci-lint
cmd/minikube/cmd/status.go:194: File is not `goimports`-ed (goimports)
	}else{
Makefile:335: recipe for target 'lint-ci' failed
make[1]: *** [lint-ci] Error 1
= go mod ================================================================
ok
Makefile:232: recipe for target 'test' failed
make: *** [test] Error 4
TravisBuddy Request Identifier: b27b8ce0-edd1-11e9-b065-29d3c1d45cd6

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: woodcockjosh
To complete the pull request process, please assign medyagh
You can assign the PR to them by writing /assign @medyagh in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@codecov-io
Copy link

codecov-io commented Oct 13, 2019

Codecov Report

Merging #5611 into master will decrease coverage by 0.06%.
The diff coverage is 10.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5611      +/-   ##
==========================================
- Coverage   37.23%   37.17%   -0.07%     
==========================================
  Files         103      103              
  Lines        7528     7546      +18     
==========================================
+ Hits         2803     2805       +2     
- Misses       4352     4368      +16     
  Partials      373      373
Impacted Files Coverage Δ
cmd/minikube/cmd/status.go 7.69% <10.71%> (+1.02%) ⬆️

Copy link
Contributor

@tstromberg tstromberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, just some minor naming nits. Thank you for writing an integration test!

cmd/minikube/cmd/status.go Outdated Show resolved Hide resolved
cmd/minikube/cmd/status.go Outdated Show resolved Hide resolved
cmd/minikube/cmd/status.go Outdated Show resolved Hide resolved
@RA489
Copy link

RA489 commented Oct 16, 2019

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 16, 2019
@woodcockjosh
Copy link
Contributor Author

@tstromberg I please check this update. From my perspective the current output doesn't make sense.

  1. In the code the kubeconfig status is referenced as kubeconfig but in the output it is referenced as kubectl
  2. In all the other statuses there is a single word status but in the kubeconfig status it is more of a message.
  3. Do we really need the ip address that kubeconfig is configured for? I don't think so.

For this reason I think it makes sense to modify the current output while adding json output. The JSON output needs to be simple and doesn't need to have user messages. We also want the json output to be somewhat congruent with the default output. I have separated the kubeconfig status from the warning message in case the kubeconfig is misconfigured.

I think this is cleaner and less confusing.

@tstromberg
Copy link
Contributor

tstromberg commented Oct 17, 2019

PR looks good, but the status integration test is failing:

       functional_test.go:179: (dbg) Run:  out/minikube-linux-amd64 status
            functional_test.go:179: (dbg) Non-zero exit: out/minikube-linux-amd64 status: exit status 1 (98.606406ms)
                -- stdout --
                host: 
                kubelet: 
                apiserver: 
                kubeconfig: 
                
                -- /stdout --
            functional_test.go:181: [out/minikube-linux-amd64 status] failed: exit status 1
            functional_test.go:185: (dbg) Run:  out/minikube-linux-amd64 status -f host:{{.Host}},kublet:{{.Kubelet}},apiserver:{{.APIServer}},kubectl:{{.Kubeconfig}}
            functional_test.go:185: (dbg) Non-zero exit: out/minikube-linux-amd64 status -f host:{{.Host}},kublet:{{.Kubelet}},apiserver:{{.APIServer}},kubectl:{{.Kubeconfig}}: exit status 1 (88.157569ms)
                -- stdout --
                host:,kublet:,apiserver:,kubectl:
                -- /stdout --
            functional_test.go:187: [out/minikube-linux-amd64 status -f host:{{.Host}},kublet:{{.Kubelet}},apiserver:{{.APIServer}},kubectl:{{.Kubeconfig}}] failed: exit status 1
            functional_test.go:191: [out/minikube-linux-amd64 status -f host:{{.Host}},kublet:{{.Kubelet}},apiserver:{{.APIServer}},kubectl:{{.Kubeconfig}}] failed: exit status 1. Output for custom format did not match
            functional_test.go:195: (dbg) Run:  out/minikube-linux-amd64 status -o json
            functional_test.go:195: (dbg) Non-zero exit: out/minikube-linux-amd64 status -o json: exit status 1 (79.102917ms)
                -- stdout --
                {"Host":"","Kubelet":"","APIServer":"","Kubeconfig":""}
                -- /stdout --
            functional_test.go:197: [out/minikube-linux-amd64 status -o json] failed: exit status 1

NOTE: This may be an unrelated error and fixed by #5649

@woodcockjosh
Copy link
Contributor Author

woodcockjosh commented Oct 17, 2019

@tstromberg works on my machine :-D

make integration -e TEST_ARGS="-test.parallel=1 -test.run TestFunctional/parallel/StatusCmd --profile=minikube --cleanup=false"
go test -v -test.timeout=60m ./test/integration --tags="integration container_image_ostree_stub containers_image_openpgp" -test.parallel=1 -test.run TestFunctional/parallel/StatusCmd --profile=minikube --cleanup=false
=== RUN   TestFunctional
=== PAUSE TestFunctional
=== CONT  TestFunctional
=== RUN   TestFunctional/parallel
=== RUN   TestFunctional/parallel/StatusCmd
=== PAUSE TestFunctional/parallel/StatusCmd
=== CONT  TestFunctional/parallel/StatusCmd
--- PASS: TestFunctional (1.28s)
    --- PASS: TestFunctional/parallel (0.00s)
        --- PASS: TestFunctional/parallel/StatusCmd (1.28s)
            functional_test.go:179: (dbg) Run:  out/minikube status
            functional_test.go:185: (dbg) Run:  out/minikube status -f host:{{.Host}},kublet:{{.Kubelet}},apiserver:{{.APIServer}},kubeconfig:{{.Kubeconfig}}
            functional_test.go:195: (dbg) Run:  out/minikube status -o json
    helpers.go:170: skipping cleanup of minikube (--cleanup=false)
PASS
ok      k8s.io/minikube/test/integration        1.316s

I did rebase off master but I'm not sure it helped

@tstromberg
Copy link
Contributor

Looks good. Thank you!

@tstromberg tstromberg merged commit fbc1630 into kubernetes:master Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

status command should support json output
8 participants