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

Update dependencies to Kubernetes v1.25.3 #606

Merged
merged 3 commits into from Nov 3, 2022

Conversation

ash2k
Copy link
Member

@ash2k ash2k commented Sep 2, 2022

No description provided.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 2, 2022
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 2, 2022
@ash2k ash2k changed the title chore!: update dependencies to Kubernetes v1.25.0 Update dependencies to Kubernetes v1.25.0 Sep 3, 2022
@ash2k ash2k force-pushed the ash2k/bump-deps branch 2 times, most recently from 19cf2c2 to fbbf2bb Compare September 5, 2022 12:51
@ash2k ash2k force-pushed the ash2k/bump-deps branch 2 times, most recently from 4f027e8 to c7d5f48 Compare September 26, 2022 10:52
@ash2k ash2k changed the title Update dependencies to Kubernetes v1.25.0 Update dependencies to Kubernetes v1.25.2 Sep 26, 2022
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 26, 2022
@ash2k
Copy link
Member Author

ash2k commented Sep 26, 2022

This is ready for review.

@ash2k ash2k force-pushed the ash2k/bump-deps branch 2 times, most recently from 58ee485 to 68ecdbf Compare September 26, 2022 12:20
@ash2k
Copy link
Member Author

ash2k commented Sep 26, 2022

/retest

Copy link
Member

@mortent mortent left a comment

Choose a reason for hiding this comment

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

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ash2k, mortent

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 Sep 28, 2022
@ash2k
Copy link
Member Author

ash2k commented Sep 28, 2022

@seans3 @karlkfi PTAL

@karlkfi
Copy link
Contributor

karlkfi commented Sep 29, 2022

Upgrading k8s libs is always a bit risky...

For kpt & config-sync we're officially required to maintain compatibility with the last 3 Anthos/GKE minor versions, but the release channels still support the last 6 minor versions, and kubectl only officially supports the last 3 minor versions...

Practically speaking, the interfaces exposed by the k8s libraries aren't actually always reverse compatible tho, and they often require changes when you upgrade. So even if we didn't run into that here, a downstream consumer might not have an easy job of it. For example, we've had cases where interfaces were replaced/refactored and the new version no longer did what we used the old one for.

We've also been sort of unofficially putting off upgrading the version of Go, just to make sure no one adds usage of any reverse incompatible features (namely generics) that would require all downstream projects to also upgrade to the latest version of Go. But we haven't really discussed when it's safe to upgrade or when it's safe to start using generics.

That said, I hate to say no to upgrading...

Let me see if I can get someone to attempt to upgrade kpt and config-sync and see if those are going to run into any serious problems before we merge this.

@ash2k
Copy link
Member Author

ash2k commented Oct 11, 2022

@karlkfi friendly ping :)

@karlkfi
Copy link
Contributor

karlkfi commented Oct 11, 2022

Yeah, sorry. We’re all kind drowning in broken and flakey e2e tests in config-sync and kpt right now. I haven’t found anyone with spare bandwidth to validate the upgrade yet.

@karlkfi
Copy link
Contributor

karlkfi commented Nov 1, 2022

Alright. I asked @rquitales to validate K8s v1.25 in kpt and ConfigSync and it looks like we also need to update Go to at least 1.18 and probably also golinter, but we don't want to do it all at the same time.

So @rquitales is working on a Go 1.18 bump for cli-utils, and then a seperate golangci-lint update, either to the latest 1.49 or 1.50, depending on what changed upstream.

Once that's done, either you can rebase this PR or @rquitales can make one that has just the k8s 1.25 changes in it.

We decided to stay at Go 1.18 for a while, instead of going to 1.19, just so downstream users of cli-utils and kpt aren't forced to upgrade, and so we don't accidentally use new 1.19 features that would force using 1.19 to compile. We'll probably eventually upgrade to 1.19, just not right now. Maybe when 1.20 come out.

@ash2k ash2k mentioned this pull request Nov 1, 2022
@ash2k
Copy link
Member Author

ash2k commented Nov 1, 2022

@karlkfi Thanks for the update.

Go 1.18 bump for cli-utils

I personally don't mind using 1.18 but Kubernetes 1.25 uses 1.19 and that's why I bumped the build here to 1.19 too. So, I guess, if you want to go back, make sure to also downgrade the build. I don't know if 1.25 builds fine with 1.18, it may.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 2022
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 2, 2022
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 2, 2022
@ash2k ash2k changed the title Update dependencies to Kubernetes v1.25.2 Update dependencies to Kubernetes v1.25.3 Nov 3, 2022
Makefile Show resolved Hide resolved
@karlkfi
Copy link
Contributor

karlkfi commented Nov 3, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 3, 2022
@k8s-ci-robot k8s-ci-robot merged commit 135cc3f into kubernetes-sigs:master Nov 3, 2022
@ash2k ash2k deleted the ash2k/bump-deps branch November 3, 2022 21:36
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants