On Slack, @Jefftree replied:

version.DefaultKubeBinaryVersion is probably a better source since it'll be present in tests, though we are sort of trying to collapse it with the gitVersion. #126686 has some history.

That change would go into code owned by SIG API-Machinery, so:

/sig api-machinery

/cc @siyuanfoundation

/triage accepted

/cc @aaron-prindle @jpbetz

This occurs because k8s.io/component-base/version depends on build flags to inject the git version. This injection does not happen when using go test manually (of course) but also not when using make test (might be an oversight that can be fixed).

Yeah, that seems like an oversight.

version.DefaultKubeBinaryVersion might be a better source of the major/minor version but we do have an issue to remove it in the future #126686

I really don't think we want something we have to bump manually every release. We should be talking to
/sig release
@kubernetes/release-engineering
and
/sig arch
[#code-organization] => @dims @liggitt

Depending on that we can update the makefile or repo management processes.

@BenTheElder: The label(s) sig/arch cannot be applied, because the repository doesn't have them.

/sig architecture

We definitely have other components using component-base outside of the repo and injecting the git version.

I don't think the integration tests (as opposed to unit tests) or e2e tests are considered to generally support go test directly and it's probably fine to assume that they're run via make, and we can fix making sure the git version is injected.

I don't think the integration tests (as opposed to unit tests) or e2e tests are considered to generally support go test directly

that's a little unfortunate, I use go test all the time with integration tests with an already running etcd and it's generally excellent

I don't think the integration tests (as opposed to unit tests) or e2e tests are considered to generally support go test directly and it's probably fine to assume that they're run via make, and we can fix making sure the git version is injected.

I disagree here too. make test-integration does not support running under a debugger, so to debug one has to invoke dlv test ./test/integration/<something> directly.

I disagree here too.

I mean certainly e2e tests do not. I don't think integration tests have advertised it and you have to do workarounds like making sure you have an appropriate etcd up manually.

make test-integration does not support running under a debugger, so to debug one has to invoke dlv test ./test/integration/ directly.

This is something we could specifically address.

I do not think we should be checking in hard-coded version metadata, and I do think it's reasonable for integration tests to use that metadata.

You could always go test with the xdefs set, or update the make targets to enable the lacking features that you use from go test directly (dlv, stress, ...) ?

I do not think we should be checking in hard-coded version metadata, and I do think it's reasonable for integration tests to use that metadata.

I'm sympathetic to preferring that go test, just work, but I think that's most reasonable for unit tests, for tests with external dependencies, without a significant rewrite they really do depend on the bash (or ginkgo or ...). You can manually replicate it though (e.g. standing up etcd, manually ensuring the correct go version, etc), but that may need to include injecting the version vars (at least for tests that use these)

For e2e and dlv we did this: f301e31

that's most reasonable for unit tests

Unit tests also might depend on knowing the current Kubernetes version. staging/src/k8s.io/api and it's staging/src/k8s.io/api/testdata come to mind.

You can manually replicate it though (e.g. standing up etcd,

This is done automatically by the test when run via go test. Both go test and make test-integration depend on the developer installing etcd beforehand.

manually ensuring the correct go version, etc)

This is covered by Go itself these days.

I do not think we should be checking in hard-coded version metadata,

So you want to move to a model where there is no versioning information committed at all to any file in the repo and the version gets injected only via make rules based on parameters, the environment or git tags?

For e2e and dlv we did this: f301e31

And I am not using it. I can't speak for others, but I prefer to work with pure Go when developing code.

Unit tests also might depend on knowing the current Kubernetes version. staging/src/k8s.io/api and it's staging/src/k8s.io/api/testdata come to mind.

possibly

This is covered by Go itself these days.

That's ... not quite true currently. We still use .go-version to wire this in. The go version in the module is the language level, not the toolchain. (it's possible to move this into the module definition, but that's a can of worms we haven't opened yet)

If you're not manually matching it or using the makefile, you're not using the supported / tested toolchain version.

So you want to move to a model where there is no versioning information committed at all to any file in the repo and the version gets injected only via make rules based on parameters, the environment or git tags?

This was the case until we added DefaultBinaryVersion. We have never checked-in the version previously.

And I am not using it. I can't speak for others, but I prefer to work with pure Go when developing code.

We've made lots of use of build tags, compiler options, and other features previously. We need them less today, but refusing to use them is counter-productive.

I mean, the e2e tests can't even be run with "pure go" anyhow, you have to build container images and bring up a cluster ...

It's worth noting that we failed to update DefaultBinaryVersion and it got out of sync with the actual major.minor version at the beginning of 1.32 for some time, and proved to be challenging to correct.

We really shouldn't have two sources of truth, and I don't see checked-in data replacing the git tags, so the arguments about "pure go" are a distraction. There should not be two sources of version information, and checked-in information is incapable of reasonably replacing the git build metadata.

We could have tests not rely on the current version, and they probably shouldn't where feasible.

If you're not manually matching it or using the makefile, you're not using the supported / tested toolchain version.

Let's face it: that's how most of the developers work. They edit the code in an IDE with language support and run tests without using make. It might not be supported and might not always work, but it seems "good enough" in practice and is more productive than doing everything the "official" way through make.

I don't know what that means for versioning. Perhaps the code which depends on the version needs to detect that it has no version and then react more gracefully?

I mean, the e2e tests can't even be run with "pure go" anyhow, you have to build container images and bring up a cluster

As with integration tests and installing etcd that is something that has do be done beforehand. Once it's done, E2E tests can run just fine with plain go test or dlv test.

Let's face it: that's how most of the developers work. They edit the code in an IDE with language support and run tests without using make.

Uh, I don't think we can assert how most Kubernetes contributors are running the integration tests. There's no data.

I think most contributors aren't running these at all, and letting CI do it for them.

It might not be supported and might not always work, [...]

... but that's the point. We shouldn't maintain two sources of truth just because some people don't want to run the makefiles for those tests, which sometimes happens to work with other test cases.

Getting reliable coverage in CI and not de-syncing the version info is more important than attempting to make all test cases work without using the supported make / shell targets.

It's not like we're prohibiting running the other tests cases, or running go test with the appropriate flags to inject the version info.

When you're running the integration tests with go test, you're not running all of them are you? Just the ones you're working on.

If you happen to need to work on the few that reference version info, that would be the only time that just using go test would be more difficult than it is currently, it can't be that bad to run those few tests with make IFF you're dealing with a test that reads the version metadata.

Most tests have no business reading the current version, which is why you've been able to ignore this so far, DefaultBinaryVersion is a very recent addition, other tests are clearly not reading it given we haven't been injecting any other version info to the integration tests.

Uh, I don't think we can assert how most Kubernetes contributors are running the integration tests. There's no data.

I'm talking about how contributors work with the source code, not how they run integration tests. I think it is safe to say that they are using some kind of IDE, which then probably also includes running tests through the IDE, at least for unit testing.

Whether that extends to integration tests depends on whether they have etcd available. What we do know is that both Jordan and I run those without make.

it can't be that bad to run those few tests with make IFF you're dealing with a test that reads the version metadata

Every integration test runs the apiserver and thus needs version metadata to correctly warn about obsolete APIs - that is what triggered this whole discussion. With "react more gracefully" I meant that if the apiserver gets started without version information, perhaps it shouldn't warn about those APIs because it simply cannot determine whether they are obsolete?

one wrinkle is that from code freeze to release, master and the release branch have identical code, but different git tags

master is 1.$next.0-alpha.0, release branch is 1.$current...

If we switch to pulling from git tags, this means all tests have to pass with both versions... I'm not sure that's always possible to do while still testing what the tests expect to test. Being able to specify the version for a test might be needed in that case ("test as if I'm 1.32", "test as if I'm 1.33", etc). Tests that don't specify, but are coded against the current default version are likely to immediately fail on master when we branch, creating an issue similar to #128616

I'm talking about how contributors work with the source code, not how they run integration tests. I think it is safe to say that they are using some kind of IDE, which then probably also includes running tests through the IDE, at least for unit testing.

Sure, some of them use IDEs, and then run unit tests that way, and above I suggested we probably shouldn't make unit tests depend on those. You can fairly reliably just click "run test" for unit tests in an IDE.

integration and e2e tests are not quite the same though, and I don't think it's reasonable that every integration/e2e test must work this way. We fully expect the e2e tests to be invoked with ginkgo, through the bash, and we are NOT testing that they work with "pure go", same for the integration tests. We shouldn't be requiring a mode of operation we don't test.

I'm not advocating that we go breaking all tests, but I think the tradeoff is more than reasonable for the handful of tests that want to be aware of the current version and feature gate enablement.

Every integration test runs the apiserver and thus needs version metadata to correctly warn about obsolete APIs - that is what triggered this whole discussion. With "react more gracefully" I meant that if the apiserver gets started without version information, perhaps it shouldn't warn about those APIs because it simply cannot determine whether they are obsolete?

I see, I guess I am advocating that we break all the integration tests without the proper version variables then :-)

I don't think this is a valid mode for starting api-server, it should refuse to start without knowing the current version, we can't correctly default.

We could allow setting the version via a single flag, we actually already have an api-server binary flag for telling it to override the version info), but you'd be responsible for correctly setting it.

You could use something like go test ... -- --version $(hack/print-workspace-status.sh | grep gitVersion) ... (I forget the flag name, @liggitt knows this, we should probably match it between integration api-server and real api-server).

If we switch to pulling from git tags, this means all tests have to pass with both versions... I'm not sure that's always possible to do while still testing what the tests expect to test.

This is already the case for the e2e tests around this behavior: #129060

The point is the correct version did change on those branches and the test should be designed to handle it or the test it's very useful. Very soon that release branch will be the release and master will be the next release "for real".

These tests are broken if they aren't prepared for the transition to the next release.

Being able to specify the version for a test might be needed in that case ("test as if I'm 1.32", "test as if I'm 1.33", etc). Tests that don't specify, but are coded against the current default version are likely to immediately fail on master when we branch, creating an issue similar to #128616

IMHO: They really need to not fail without hacking around it. This is causing real pain regardless of the source of version info. We can't have weeks of trying to get the version updated after cutting a release, every release.

You could use something like go test ... -- --version $(hack/print-workspace-status.sh | grep gitVersion) ... (I forget the flag name, @liggitt knows this, we should probably match it between integration api-server and real api-server).

the flag is --version, not to be confused with --emulation-version

https://kubernetes.io/docs/reference/command-line-tools-reference/kube-apiserver/

We could probably hack up some other fallback mode around https://pkg.go.dev/go/build#Package, but in CI and the make scripts we should prefer the single-source-of-truth instead of Go's own version codepaths (which won't quite match our binary versions, and we can't break that API) or hard-coding.

But that still leaves, regardless: we need to make N+1 version less brittle. We need to figure out how to test this feature in a way that tolerates the next release without manual fixups.

golang/go#50603

@aojea that's:

We could probably hack up some other fallback mode around https://pkg.go.dev/go/build#Package, but in CI and the make scripts we should prefer the single-source-of-truth instead of Go's own version codepaths (which won't quite match our binary versions, and we can't break that API) or hard-coding.

https://pkg.go.dev/go/build#Package doesn't have versioning information as of Go 1.23, or am I blind?
If I'm reading golang/go#50603 right, Go 1.24 will add that to https://pkg.go.dev/runtime/debug#Module