Bump Cobra from 1.5.0 to 1.6.1 #3695

marckhouzam · 2022-10-18T16:27:46Z

What this PR does / why we need it

This PR bumps to Cobra 1.6.1 which was recently released: https://github.com/spf13/cobra/releases/tag/v1.6.1
There is nothing really major in this new release, but it does have bug fixes and a couple of new features. We should keep up to date.

Describe testing done for PR

Let CI run

vuil

lgtm, assuming CI checks out

github-actions · 2022-10-18T16:37:57Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3695/20221018163755/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

marckhouzam · 2022-10-18T21:39:07Z

This just needs the ok-to-merge label and it's good to go.

randomvariable · 2022-10-20T17:32:15Z

Let's get #3699 in first if we can

marckhouzam · 2022-10-20T18:04:09Z

Let's get #3699 in first if we can

Sure, let's wait. No particular hurry for the Cobra upgrade.

marckhouzam · 2022-10-25T15:39:09Z

I've updated the PR to move to Cobra 1.6.1 instead of 1.6.0 which was released yesterday

github-actions · 2022-10-25T15:47:08Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3695/20221025154706/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

codecov · 2022-10-25T16:38:17Z

Codecov Report

Merging #3695 (53aee7e) into main (2f9044a) will decrease coverage by 0.90%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3695      +/-   ##
==========================================
- Coverage   46.93%   46.03%   -0.91%     
==========================================
  Files         402      427      +25     
  Lines       40203    41754    +1551     
==========================================
+ Hits        18869    19221     +352     
- Misses      19582    20765    +1183     
- Partials     1752     1768      +16

Impacted Files	Coverage Δ
...ons/controllers/packageinstallstatus_controller.go	`79.15% <0.00%> (ø)`
cmd/cli/plugin/cluster/osimage.go	`100.00% <0.00%> (ø)`
cmd/cli/plugin/cluster/set_machinehealthcheck.go	`23.33% <0.00%> (ø)`
cmd/cli/plugin/cluster/machinehealthcheck.go	`100.00% <0.00%> (ø)`
cmd/cli/plugin/cluster/get_machinehealthcheck.go	`11.42% <0.00%> (ø)`
cmd/cli/plugin/cluster/create.go	`42.22% <0.00%> (ø)`
.../cli/plugin/cluster/get_machinehealthcheck_node.go	`9.30% <0.00%> (ø)`
cmd/cli/plugin/cluster/osimage_oracle.go	`3.20% <0.00%> (ø)`
cmd/cli/plugin/cluster/scale.go	`17.85% <0.00%> (ø)`
cmd/cli/plugin/cluster/delete_node_pool.go	`16.66% <0.00%> (ø)`
... and 17 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

anujc25

LGTM. Thanks for updating this.

marckhouzam · 2022-10-27T14:12:58Z

This PR is waiting for #3752 (which is replacing #3699) so we require yet another rebase from that PR.

marckhouzam · 2022-11-04T19:58:34Z

#3752 was replaced by #3772 which is now merged, so we can go ahead with this PR here.

github-actions · 2022-11-04T20:09:45Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3695/20221104200942/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2022-11-07T13:38:58Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3695/20221107133856/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

anujc25

LGTM. But need to make sure all CI pipelines are successful before we merge this.

anujc25 · 2022-11-08T16:33:08Z

Looks like some unit tests are failing as part of Main/Build CI

--- 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

marckhouzam · 2022-11-08T17:39:01Z

Looks like some unit tests are failing as part of Main/Build CI

--- 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

Wow, thanks for finding this. I will look into it.

marckhouzam · 2022-11-08T17:59:49Z

Looks like some unit tests are failing as part of Main/Build CI

--- 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

So Framework's kctrl imports carvel-kapp-controller which in turns uses cppforlife which needs to be modified to accept a recent Cobra change. I will have to chase this down.

We don't upgrade the 'package' plugin however, because it uses github.com/vmware-tanzu/carvel-kapp-controller/cli/pkg/kctrl/cmd which in turn uses github.com/cppforlife/cobrautil which panics when using Cobra 1.6. See cppforlife/cobrautil#7 Signed-off-by: Marc Khouzam <kmarc@vmware.com>

github-actions · 2022-11-10T02:18:03Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3695/20221110021801/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

marckhouzam · 2022-11-14T15:20:27Z

I've made the CI pass by not bumping cobra for the package plugin because it first needs github.com/vmware-tanzu/carvel-kapp-controller/cli/pkg/kctrl/cmd to be fixed once github.com/cppforlife/cobrautil is fixed.

However, I am concerned that another plugin may have a similar problem, but that it is not triggered by CI.

I have posted a PR to fix github.com/cppforlife/cobrautil: cppforlife/cobrautil#8

marckhouzam requested review from a team as code owners October 18, 2022 16:27

vmwclabot added the cla-not-required label Oct 18, 2022

marckhouzam requested a review from randomvariable October 18, 2022 16:28

vuil approved these changes Oct 18, 2022

View reviewed changes

randomvariable approved these changes Oct 18, 2022

View reviewed changes

randomvariable added ok-to-merge PRs should be labelled with this before merging and removed ok-to-merge PRs should be labelled with this before merging labels Oct 20, 2022

marckhouzam added the do-not-merge/hold Some fixes necessary, hold for merging label Oct 20, 2022

marckhouzam force-pushed the feat/cobra1_6_0 branch from ea702a6 to bb5093f Compare October 25, 2022 15:36

marckhouzam changed the title ~~Bump Cobra from 1.5.0 to 1.6.0~~ Bump Cobra from 1.5.0 to 1.6.1 Oct 25, 2022

anujc25 approved these changes Oct 25, 2022

View reviewed changes

marckhouzam force-pushed the feat/cobra1_6_0 branch from bb5093f to 9d18403 Compare November 4, 2022 19:57

marckhouzam removed the do-not-merge/hold Some fixes necessary, hold for merging label Nov 4, 2022

marckhouzam force-pushed the feat/cobra1_6_0 branch from 9d18403 to 573d6bc Compare November 7, 2022 13:24

anujc25 approved these changes Nov 8, 2022

View reviewed changes

marckhouzam added the do-not-merge/hold Some fixes necessary, hold for merging label Nov 8, 2022

marckhouzam force-pushed the feat/cobra1_6_0 branch from 573d6bc to 53aee7e Compare November 10, 2022 02:08

marckhouzam mentioned this pull request Nov 30, 2022

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

Closed

marckhouzam closed this Aug 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump Cobra from 1.5.0 to 1.6.1 #3695

Bump Cobra from 1.5.0 to 1.6.1 #3695

marckhouzam commented Oct 18, 2022 •

edited

Loading

vuil left a comment

github-actions bot commented Oct 18, 2022

marckhouzam commented Oct 18, 2022

randomvariable commented Oct 20, 2022

marckhouzam commented Oct 20, 2022

marckhouzam commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

codecov bot commented Oct 25, 2022 •

edited

Loading

anujc25 left a comment

marckhouzam commented Oct 27, 2022

marckhouzam commented Nov 4, 2022

github-actions bot commented Nov 4, 2022

github-actions bot commented Nov 7, 2022

anujc25 left a comment

anujc25 commented Nov 8, 2022

marckhouzam commented Nov 8, 2022

marckhouzam commented Nov 8, 2022

github-actions bot commented Nov 10, 2022

marckhouzam commented Nov 14, 2022 •

edited

Loading

Bump Cobra from 1.5.0 to 1.6.1 #3695

Bump Cobra from 1.5.0 to 1.6.1 #3695

Conversation

marckhouzam commented Oct 18, 2022 • edited Loading

What this PR does / why we need it

Describe testing done for PR

vuil left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 18, 2022

marckhouzam commented Oct 18, 2022

randomvariable commented Oct 20, 2022

marckhouzam commented Oct 20, 2022

marckhouzam commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

codecov bot commented Oct 25, 2022 • edited Loading

Codecov Report

anujc25 left a comment

Choose a reason for hiding this comment

marckhouzam commented Oct 27, 2022

marckhouzam commented Nov 4, 2022

github-actions bot commented Nov 4, 2022

github-actions bot commented Nov 7, 2022

anujc25 left a comment

Choose a reason for hiding this comment

anujc25 commented Nov 8, 2022

marckhouzam commented Nov 8, 2022

marckhouzam commented Nov 8, 2022

github-actions bot commented Nov 10, 2022

marckhouzam commented Nov 14, 2022 • edited Loading

marckhouzam commented Oct 18, 2022 •

edited

Loading

codecov bot commented Oct 25, 2022 •

edited

Loading

marckhouzam commented Nov 14, 2022 •

edited

Loading