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

[PSA] Potential pitfalls with dependency bumps #123744

Open
MadhavJivrajani opened this issue Mar 6, 2024 · 5 comments
Open

[PSA] Potential pitfalls with dependency bumps #123744

MadhavJivrajani opened this issue Mar 6, 2024 · 5 comments
Labels
area/code-organization Issues or PRs related to kubernetes code organization sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@MadhavJivrajani
Copy link
Contributor

MadhavJivrajani commented Mar 6, 2024

_Note: this issue is only meant to document the situation that currently exists because discussions in slack are ephemeral and hard to discover.

TL;DR

golang/go#65573 is a proposal accepted and targeting Go 1.23 that will address the issue being described here!

In the meantime, please exercise extra caution in the following scenarios:

  • Backporting dependency changes - please check what the go directives are in these dependencies before we backport them.
  • Upgrading go directives in community owned, single branch dependencies such as k/utils, k/gengo, k/kube-openapi etc.

Context

go1.21 introduced 2 immensely helpful features:

  1. Default GODEBUG settings are now taken based on the go directive in go.mods (ref: https://go.dev/doc/godebug). This is particularly helpful to maintain compatibility behaviour when Go makes breaking(-but-compatible) changes as part of its minor releases. This is also especially important since Kubernetes now bumps minor versions of Go on its release branches: https://github.com/kubernetes/enhancements/tree/master/keps/sig-release/3744-stay-on-supported-go-versions
  2. The go directive in go.mod now specifies the minimum go version required to build your own code. We couldn't do this before, and that meant older go toolchains were allowed to compile code that was meant for newer go versions, even though it would lead to breaking changes: https://go.dev/blog/toolchain

The Problem

While (1) is highly desirable, (2) interferes with its efficacy of being able to default GODEBUG settings, illustrated below.

The currently supported release branches of Kubernetes have their versions of Go bumped to go1.21.x (same as master). What this means is code is tested, built and released using go1.21.x:

However, the go directive in go.mod is as follows for the currently supported Kubernetes versions:

Essentially, we leave the go directive in the go.mods to indicate the Go version with which a Kubernetes minor was first shipped. Because of go1.21's GODEBUG defaulting, we get to preserve that behaviour for our users even though we now continue to build with higher versions of Go (to stay well within the support window of Go).

However, many of the fixes needed to bump minor versions of Go on release branches involves backporting dependency bumps, for example:

If dependency D has a go directive in its go.mod of go 1.21, was bumped at master to fix a bug and we need to backport this to our release branches that has a go directive of go 1.20, the bump will essentially force the go directive in our release branches to go 1.21 because of (2) - the go directive required is going to be the max(all dep. go directives). This is not desirable since we now also loose the compatibility behaviour of go1.20 that our users would expect.

What Do We Do?

golang/go#65573 is a proposal that is being worked on in the Go community to address this dichotomy between (1) and (2) and will hopefully land in go1.23.

In the meantime, we need to exercise extra caution in the following scenarios:

  • Backporting dependency changes - please check what the go directives are in these dependencies before we backport them.
  • Upgrading go directives in community owned, single branch dependencies such as k/utils, k/gengo, k/kube-openapi etc. If we bump the go directive here and later need to absorb a fix in one of these dependencies into k/k via backporting, we will run into the same issue.

There is also work happening to safeguard accidental bumps:

/sig architecture
/area code-organization
/triage accepted
/cc @kubernetes/dep-approvers

@k8s-ci-robot k8s-ci-robot added sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. area/code-organization Issues or PRs related to kubernetes code organization triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Mar 6, 2024
@liggitt
Copy link
Member

liggitt commented Mar 6, 2024

Fantastic write-up, thanks for laying everything out so clearly

@nathanperkins
Copy link

Thank you for the write up. This will be really helpful when we engage with OSS projects asking to bump go versions back down where possible.

@troy0820
Copy link
Member

Definitely appreciate the explanation and path to resolution.

@sbueringer
Copy link
Member

💯 Thank you very much!

@liggitt
Copy link
Member

liggitt commented Mar 19, 2024

updated description, the Go proposal was accepted and is targeting Go 1.23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/code-organization Issues or PRs related to kubernetes code organization sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

6 participants