Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Upgrading the cobra version to 1.6.1 causes kctrl tests to fail #4040

Closed
srm09 opened this issue Nov 29, 2022 · 3 comments · Fixed by #4065
Closed

Upgrading the cobra version to 1.6.1 causes kctrl tests to fail #4040

srm09 opened this issue Nov 29, 2022 · 3 comments · Fixed by #4065
Assignees
Labels
kind/bug PR/Issue related to a bug needs-triage Indicates an issue or PR needs to be triaged

Comments

@srm09
Copy link
Contributor

srm09 commented Nov 29, 2022

Bug description
The dependency github.com/spf13/cobra has a newer version available v1.6.1. This was supposed to be upgraded as part of the CAPA version bump in #3874. The bump was causing the kctrl tests to fail with the following error:

$ go test -timeout 30s -coverprofile=/var/folders/w9/fvjgk13x40s56gvtzqp3q7kh0000gq/T/vscode-golKDwiN/go-code-cover github.com/vmware-tanzu/tanzu-framework/cmd/cli/plugin/package/kctrl

--- FAIL: TestKctrlInvoke (0.00s)
    --- FAIL: TestKctrlInvoke/Invoke (0.00s)
        --- FAIL: TestKctrlInvoke/Invoke/add_package_commands_from_kctrl (0.00s)
panic: Expected to find available commands section in spf13/cobra default usage template [recovered]
	panic: Expected to find available commands section in spf13/cobra default usage template

goroutine 132 [running]:
testing.tRunner.func1.2({0x2391340, 0x2907540})
	/usr/local/Cellar/go@1.18/1.18.5/libexec/src/testing/testing.go:1389 +0x24e
testing.tRunner.func1()
	/usr/local/Cellar/go@1.18/1.18.5/libexec/src/testing/testing.go:1392 +0x39f
panic({0x2391340, 0x2907540})
	/usr/local/Cellar/go@1.18/1.18.5/libexec/src/runtime/panic.go:838 +0x207
github.com/cppforlife/cobrautil.HelpSectionsUsageTemplate({0xc000090d20, 0x2, 0x269e88d?})
	/Users/muchhals/go/pkg/mod/github.com/cppforlife/cobrautil@v0.0.0-20220907150944-da5ee3a6ab1f/help_sections.go:42 +0x25c
github.com/vmware-tanzu/carvel-kapp-controller/cli/pkg/kctrl/cmd.AddPackageCommands(0xc00071b400, 0x3b902b0?, {{0x29252f8?, 0xc0004f9290?}, {0x2924220?, 0xc0004f92c0?}}, {{0x26960f7, 0x5}, 0x1, 0x0, ...})
	/Users/muchhals/go/pkg/mod/github.com/vmware-tanzu/carvel-kapp-controller/cli@v0.0.0-20221026213745-c834810997d4/pkg/kctrl/cmd/kctrl.go:180 +0x267
github.com/vmware-tanzu/carvel-kapp-controller/cli/pkg/kctrl/cmd.AttachKctrlPackageCommandTree(0x235d540?, 0xc00003c240, {{0x26960f7?, 0x228ce0e?}, 0x40?, 0xd5?, 0x35?})
	/Users/muchhals/go/pkg/mod/github.com/vmware-tanzu/carvel-kapp-controller/cli@v0.0.0-20221026213745-c834810997d4/pkg/kctrl/cmd/kctrl.go:234 +0x254
github.com/vmware-tanzu/tanzu-framework/cmd/cli/plugin/package/kctrl.Invoke(0xc00019a988)
	/Users/muchhals/workspace/tkg/tanzu-framework/cmd/cli/plugin/package/kctrl/kctrl.go:21 +0x2ae
github.com/vmware-tanzu/tanzu-framework/cmd/cli/plugin/package/kctrl.TestKctrlInvoke.func1.1(0x0?)
	/Users/muchhals/workspace/tkg/tanzu-framework/cmd/cli/plugin/package/kctrl/kctrl_test.go:27 +0x13c
testing.tRunner(0xc000783d40, 0x2790cb8)
	/usr/local/Cellar/go@1.18/1.18.5/libexec/src/testing/testing.go:1439 +0x102
created by testing.(*T).Run
	/usr/local/Cellar/go@1.18/1.18.5/libexec/src/testing/testing.go:1486 +0x35f
FAIL	github.com/vmware-tanzu/tanzu-framework/cmd/cli/plugin/package/kctrl	0.444s
FAIL

Due to this, we pinned back the version used by cmd/cli/plugin/package to v1.5.0 for the time being. This issue talks about addressing the test failure and upgrading the cobra version to the latest available.

Affected product area (please put an X in all that apply)

  • ( ) APIs
  • ( ) Addons
  • (x) CLI
  • ( ) Docs
  • ( ) IAM
  • ( ) Installation
  • ( ) Plugin
  • ( ) Security
  • ( ) Test and Release
  • ( ) User Experience
  • ( ) Developer Experience

Expected behavior
We should be able to safely upgrade the cobra dependency version.

Steps to reproduce the bug

Version (include the SHA if the version is not obvious)

Environment where the bug was observed (cloud, OS, etc)

Relevant Debug Output (Logs, manifests, etc)

@srm09 srm09 added kind/bug PR/Issue related to a bug needs-triage Indicates an issue or PR needs to be triaged labels Nov 29, 2022
@marckhouzam
Copy link
Contributor

marckhouzam commented Nov 30, 2022

Thanks for this report @srm09 .
We have a blocked PR that was meant to upgrade cobra to v1.6.1 for all of Framework: #3695

The failure in the package plugin was concerning me however.
The PR also pinned the package plugin to cobra v1.5.0 but I wasn't sure that was sufficient. I was concerned that other plugins may have a similar problem, but that it was not triggered by CI in their case. And since the problem causes a panic I was hesitant to move forward.

The root cause of the problem is that the package plugin uses github.com/vmware-tanzu/carvel-kapp-controller/cli/pkg/kctrl/cmd which uses github.com/cppforlife/cobrautil which is not compatible with Cobra v1.6.1.

I have posted a fix to github.com/cppforlife/cobrautil but have got no response: cppforlife/cobrautil#8

Once github.com/cppforlife/cobrautil is fixed, we still need github.com/vmware-tanzu/carvel-kapp-controller to take that fix and then we'll be able to upgrade the package plugin.

Either that or we need to stop using these broken dependencies.

@praveenrewar
Copy link
Contributor

praveenrewar commented Nov 30, 2022

Thank you so much for the fix @marckhouzam. I have created a PR to bump cppforlife/cobrautil and once it's merged, I will upgrade package plugin to take that fix in.

@marckhouzam
Copy link
Contributor

Thanks @praveenrewar for getting this unblocked! Once you are done, I will rebase my PR to update the Cobra dependency for the rest of framework.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug PR/Issue related to a bug needs-triage Indicates an issue or PR needs to be triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants