Update Cobra from 1.2.1 to 1.3.0

[brianpursley]

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This PR upgrades cobra from version 1.2.1 to 1.3.0.

This will be helpful for kubectl for a few reasons:

• Enables displaying command descriptions in bash completion output like it does for zsh and fish completions currently (bug fixed in cobra 1.3.0 to make this usable Support different bash completion options spf13/cobra#1509)
• Improves flag parsing capabilities which will help with plugin command completions (Shell completion for plugins #105867 (comment))
• Several other minor fixes and enhancements in Cobra 1.3.0 related to completions

Which issue(s) this PR fixes:

Fixes kubernetes/kubectl#1166

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[brianpursley]

/cc @eddiezane
/cc @marckhouzam

[k8s-ci-robot]

@brianpursley: GitHub didn't allow me to request PR reviews from the following users: marckhouzam.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @eddiezane
/cc @marckhouzam

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]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: brianpursley
To complete the pull request process, please ask for approval from liggitt after the PR has been reviewed.

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

Needs approval from an approver in each of these files:

• OWNERS
• staging/src/k8s.io/api/OWNERS
• staging/src/k8s.io/legacy-cloud-providers/OWNERS

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

[brianpursley]

Hmm, unit tests are passing locally. Maybe a flake.

/retest

[fedebongio]

/triage accepted

[liggitt]

I think it is due to cobra's dependency on a newer version of viper, which has a large number of dependencies.

yeah... that might be something to keep on the back burner to investigate a way to isolate / drop ...

[liggitt]

/test pull-kubernetes-dependencies

[liggitt]

what is pulling this in? another proto generation tool increases likelihood of selecting incompatible transitive protobuf libraries

[eddiezane]

This appears to make it's way in from google.golang.org/grpc via github.com/envoyproxy/go-control-plane via github.com/envoyproxy/protoc-gen-validate.

[liggitt]

what is pulling this in?

new transitive deps from this once we get to v3:

• https://github.com/circonus-labs/go-apiclient/blob/master/go.mod
• https://github.com/circonus-labs/circonus-gometrics/blob/master/go.mod
• https://github.com/circonus-labs/circonusllhist/blob/master/go.mod
• https://github.com/hashicorp/go-retryablehttp/blob/master/go.mod
• https://github.com/tv42/httpunix/blob/master/go.mod
• https://github.com/hashicorp/go-hclog/blob/master/go.mod

[liggitt]

new

[liggitt]

new

[liggitt]

same question about what is pulling this in?

[marckhouzam]

I think it is due to cobra's dependency on a newer version of viper, which has a large number of dependencies.

yeah... that might be something to keep on the back burner to investigate a way to isolate / drop ...

So kubernetes doesn't use Viper directly correct? And removing the dependency to Viper would have benefits for kubernetes?

If so, I will discuss this with the Cobra maintainers and see if there's anything that can be done.

[eddiezane]

I think it is due to cobra's dependency on a newer version of viper, which has a large number of dependencies.

yeah... that might be something to keep on the back burner to investigate a way to isolate / drop ...

So kubernetes doesn't use Viper directly correct? And removing the dependency to Viper would have benefits for kubernetes?

If so, I will discuss this with the Cobra maintainers and see if there's anything that can be done.

If that could happen it would cut down on a ton of transitive deps for us.

ref: #102598

[liggitt]

Yeah, we dropped direct dependence on Viper in that PR, but we still have a transitive dependency on it, which adds a lot to our dependency graph. https://github.com/muesli/coral is an interesting proof-of-concept that tracks head of cobra and drops the viper dep. Since some of our deps use cobra though, I think the viper dependency would have to be made optional in cobra itself for it to benefit us:

go mod graph | grep " github.com/spf13/cobra" | grep -v k8s.io
github.com/coredns/corefile-migration@v1.0.14 github.com/spf13/cobra@v1.1.3
github.com/containerd/continuity@v0.1.0 github.com/spf13/cobra@v1.0.0
go.etcd.io/etcd/server/v3@v3.5.0 github.com/spf13/cobra@v1.1.3
go.etcd.io/etcd/pkg/v3@v3.5.0 github.com/spf13/cobra@v1.1.3

[jpmcb]

Hi all - I was pointed to this PR from some people in the cobra community. From what I gather, we need to remove the transient dependencies on Viper so that the dependencies resolve correctly?

[liggitt]

Hi all - I was pointed to this PR from some people in the cobra community. From what I gather, we need to remove the transient dependencies on Viper so that the dependencies resolve correctly?

The dependencies are resolving correctly, but are steadily increasing beyond acceptable levels because of the viper transitive dependency.

[liggitt]

The dependencies are resolving correctly, but are steadily increasing beyond acceptable levels because of the viper transitive dependency.

I'll be filing an issue in the cobra repo with a request to isolate the viper dependencies used only by the cobra command, with some suggestions/considerations based on our experience isolating dependencies for the cadvisor command (google/cadvisor#2437)

[liggitt]

I'll be filing an issue in the cobra repo with a request to isolate the viper dependencies used only by the cobra command, with some suggestions/considerations based on our experience isolating dependencies for the cadvisor command (google/cadvisor#2437)

opened spf13/cobra#1597

[marckhouzam]

Do you guys still plan to go ahead with upgrading to Cobra 1.3 or do you want to wait for the next release of Cobra which will remove the dependency on Viper?
Some Cobra 1.3 features could be used right away (completion description for Bash), and others would unblock other PRs (completion for plugins #105867).

[k8s-ci-robot]

@brianpursley: PR needs rebase.

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.

[dims]

@marckhouzam i'd rather wait until viper is out.

[liggitt]

FYI, the viper dependency has been dropped from cobra HEAD in spf13/cobra#1604, and cobra v1.4.0 will be tagged shortly. Once it is, this can be updated to cobra v1.4.0 🎉

I imagine it will be easier to reset against master and redo the update, since the bump will be dropping a bunch of viper-only dependencies instead of requiring updates to them (there may still be updates to other existing things, which could be ok, but I'd expect fewer new deps)

[marckhouzam]

FYI, the viper dependency has been dropped from cobra HEAD in spf13/cobra#1604, and cobra v1.4.0 will be tagged shortly. Once it is, this can be updated to cobra v1.4.0 🎉

Thanks @liggitt for working with Cobra to make this happen. It will benefit a whole bunch of projects 🎉

[jpmcb]

Hi all - Just cut v1.4.0

https://github.com/spf13/cobra/releases/tag/v1.4.0

Feel free to tag me if there's any needed cobra support 🐍 🚀 🌔

---

And thank you all for the support in making this happen! Big shoutout to @liggitt for the in depth proposal and help on this!

[dims]

thanks a ton @liggitt @jpmcb !!

[brianpursley]

Closing this PR.

It sounds like @liggitt may open a PR to upgrade to Cobra 1.4.0.