Skip to content

Conversation

@mboersma
Copy link
Contributor

@mboersma mboersma commented Jul 9, 2024

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Updates CAPI to v1.6.7. Also bumps cert-manager to v1.15.1 to stay in sync with CAPI.

Which issue(s) this PR fixes:

N/A, but see #4916 for prior art.

Special notes for your reviewer:

  • cherry-pick candidate

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests

Release note:

Bump CAPI to v1.6.7

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 9, 2024
@k8s-ci-robot k8s-ci-robot requested review from Jont828 and marosset July 9, 2024 22:28
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 9, 2024
@mboersma
Copy link
Contributor Author

This is panicking with a nil pointer when running controller-gen, and I can reproduce it locally. It’s unclear to me what change triggered this bug.

Updating to controller-gen v0.14.0 seems to fix the problem, and that’s what we’re using in the newer release branch, so I’ll try that.

@nojnhuh
Copy link
Contributor

nojnhuh commented Jul 10, 2024

This is panicking with a nil pointer when running controller-gen, and I can reproduce it locally. It’s unclear to me what change triggered this bug.

Updating to controller-gen v0.14.0 seems to fix the problem, and that’s what we’re using in the newer release branch, so I’ll try that.

@sbueringer mentioned on Slack that the kubekins CI images got bumped to Go 1.22 which controller-gen 0.13 had panic issues with, so I'm assuming this is that same issue. Looks like what they did in CAPI was bump to (use but not require) Go 1.22: kubernetes-sigs/cluster-api#10802

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 10, 2024
@mboersma
Copy link
Contributor Author

Looks like what they did in CAPI was bump to (use but not require) Go 1.22

We could do that, although it would be strange to do so just in the release branch IMHO. I'd feel more comfortable making that change in main and then cherry-picking (so my workaround seems simpler for now). I'll try that approach soon.

@sbueringer
Copy link
Member

You can also just continue to compile controller-gen with Go 1.21, less churn: https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/pull/3077/files

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 10, 2024
@mboersma
Copy link
Contributor Author

/retest

@mboersma
Copy link
Contributor Author

info - 2024-07-10 21:33:17,198 -- ci service found: github-actions
Error: Codecov token not found. Please provide Codecov token with -t flag.

@nojnhuh I admit not to understanding how codecov works now. Any ideas how I can fix or work around it?

@nojnhuh
Copy link
Contributor

nojnhuh commented Jul 11, 2024

info - 2024-07-10 21:33:17,198 -- ci service found: github-actions
Error: Codecov token not found. Please provide Codecov token with -t flag.

@nojnhuh I admit not to understanding how codecov works now. Any ideas how I can fix or work around it?

That makes two of us. I think cherry-picking this change into this PR would fix it: #4952

I had originally closed that because of the controller-gen panic thing, but if that's fixed here too then I think it would work. Both changes probably have to land at the same time though.

@nojnhuh
Copy link
Contributor

nojnhuh commented Jul 11, 2024

I also wouldn't totally rule out skipping this entirely if we're going to stop supporting v1.14 next week. No changes have been merged since v1.14.6 and the couple of backports I've closed since then aren't user-facing.

@codecov
Copy link

codecov bot commented Jul 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.64%. Comparing base (1256461) to head (99d09d0).
Report is 1 commits behind head on release-1.14.

Additional details and impacted files
@@               Coverage Diff                @@
##           release-1.14    #4975      +/-   ##
================================================
- Coverage         62.65%   62.64%   -0.02%     
================================================
  Files               192      192              
  Lines             15694    15694              
================================================
- Hits               9833     9831       -2     
- Misses             5174     5176       +2     
  Partials            687      687              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mboersma
Copy link
Contributor Author

@nojnhuh this is green and ready for re-review. 😄

Copy link
Contributor

@nojnhuh nojnhuh left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 12, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: b744381cff771ba8189e985d266e42866df400b2

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nojnhuh

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

The pull request process is described 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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 12, 2024
@k8s-ci-robot k8s-ci-robot merged commit 0ba74d0 into kubernetes-sigs:release-1.14 Jul 12, 2024
@mboersma mboersma deleted the bump-capi-1.6.7 branch July 12, 2024 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants