test-infra
test-infra copied to clipboard
don't use generics yet
Using generics in this repo forces all dependent projects to switch to go 1.18 + generics.
Notably, boskos depends on prow utility packages, and then projects like kubetest2 use boskos's client library ... https://github.com/kubernetes-sigs/kubetest2/pull/207
This is not particularly tenable when we have to run in environments with Kubernetes release branches that use older go and won't be upgrading or using genericgs, see also: https://github.com/kubernetes/community/pull/6693
The best alternative I see is ensuring nothing else depends on test-infra, I'm not sure if that only means boskos / boskos client users at the moment.
xref: https://github.com/kubernetes/test-infra/pull/25917 cc @alvaroaleman @chaodaiG
/sig testing
So far the assumption has been that this repo is fairly self-contained and that if we want to upgrade e.G. the go version, we can just do that. I wasn't aware that this changed.
This might be visible right now with generics, but the same could happen for any new feature or stdlib addition we use from a newer go version. If there is any expectation on how fast we can or can not update go or any other dependency, we should IMHO be very explicit about that.
Maybe we can make the boskos client lib a submodule of boskos? Ideally, that thing should only depend on the boskos api and the go stdlib for the http client.
This might be visible right now with generics, but the same could happen for any new feature or stdlib addition we use from a newer go version. If there is any expectation on how fast we can or can not update go or any other dependency, we should IMHO be very explicit about that.
yeah, unfortunately this is also hard to manage in go, while the go.mod go statement gates language features like generics, there's nothing like this for the stdlib.
Maybe we can make the boskos client lib a submodule of boskos? Ideally, that thing should only depend on the boskos api and the go stdlib for the http client.
you would think, but actually that's the issue here, having generic code in test-infra is one thing, but boskos client depends on the prow secret agent, so that breaks compilation https://github.com/kubernetes-sigs/boskos/blob/35595e83c0806a9f0f86498d28eb5a25ef30897d/client/client.go#L41
So far the assumption has been that this repo is fairly self-contained and that if we want to upgrade e.G. the go version, we can just do that. I wasn't aware that this changed.
It hasn't been since boskos was broken out, in particular. Actually until last week we had a dependency cycle between the repos 🔥 https://github.com/kubernetes/test-infra/pull/26956
So also, even if we broke the boskos client out into it's own package, it's actually the boskos client causing issues already (AFAIK go only tries to compile packages that are actually imported into a binary ...), due to use of the secret agent
Otherwise i'd just fork the boskos client into kubetest2 for now like we merged with kubetest in test-infra and punt on the rest.
If this is just about kubetest, what keeps us from just upgrading its go version?
It’s “go install”ed by developers and CI, and when on older currently supported releases of kubernetes the toolchains required lack it.
switching everyone to prebuilt binaries or something is impractical and this hasn’t been an issue for O(years), probably because of the previous poor state of toolchains in test-infra …
We really should be deciding: is test-infra supported as a library? If so, we should still consider back pedaling forcing generics yet IMO, the with parser api could be reworked to be Unmarshal style. If not we should undo boskos heavily importing test-infra packages, which we previously rejected.
I believe our statement to is test-infra supported as a library is no. Openshift has a repo that heavily depends on test-infra so we informally agreed on asking before removing functionality. Other than that though I think we always assumed that it is self-contained and has no external consumers. We at some point had an issue open to clarify the boundaries of that but it never got traction: https://github.com/kubernetes/test-infra/issues/21268
For this specific occurence I think it doesn't actually need generics, I can look into that maybe later this week. We should still be clear about what our policy for the general case is though and if we act like we are a lib. cc @chaodaiG @cjwagner
We should still be clear about what our policy for the general case is though and if we act like we are a lib. cc @chaodaiG @cjwagner
In my opinion, we should not act like a library in our current state. We don't identify or control what is exposed so it isn't practical for contributors to anticipate arbitrary external consumption. Continually avoiding changing any previously created public function would make refactoring difficult and grow our tech debt.
ACK, I think we should probably note this somewhere, and boskos probably needs to move off of a number of prow util packages.
https://github.com/kubernetes-sigs/boskos/search?q=k8s.io%2Ftest-infra shows at least:
prow/config
prow/config/secret
prow/flagutil
prow/interrupts
prow/logrusutil
prow/metrics
prow/pjutil
prow/pjutil/pprof
prow/simplifypath
What's an acceptable path forward for that? Fork them into boskos? Create an actual library repo?
In the short term, I think we can unblock upgrading boskos client users just by eliminating the dependency on prow/config/secret.Agent in boskos/client
I think creating an actual library repo is a better solution in the long run. For now, we can start with forking secret.Agent which doesn't use Generics to boskos. This will let consumers of boskos to use the latest boskos version.
With time, all the util like packages can move to a library repo like prow-utils or test-infra-utils.
I think creating an actual library repo is a better solution in the long run
probably? but it requires buy-in from the folks actually developing this code.
For now, we can start with forking secret.Agent which doesn't use Generics to boskos. This will let consumers of boskos to use the latest boskos version.
Yeah, I think this is the most reasonable step, boskos users should mostly be either using binaries or the client library, and the client library only imports prow/config/secret so we can cut the test-infra dependency for the client library at least and mitigate most of the issue.
However, if test-infra is not intended to be used as a library, we still have a problem with boskos using it heavily elsewhere.
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue or PR as fresh with
/remove-lifecycle stale - Mark this issue or PR as rotten with
/lifecycle rotten - Close this issue or PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
/lifecycle frozen
been a while :) i think we are ok now to use generics
/close
@dims: Closing this issue.
In response to this:
been a while :) i think we are ok now to use generics
/close
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.