kubernetes-sigs / kubetest2

Kubetest2 is the framework for launching and running end-to-end tests on Kubernetes.
Apache License 2.0
326 stars 105 forks source link

Update sigs.k8s.io/boskos #207

Closed palnabarun closed 1 year ago

palnabarun commented 1 year ago

This can potentially fix https://github.com/kubernetes/kubernetes/issues/111584

Ran the following:

$ go get -u sigs.k8s.io/boskos@37bd9bb41b86a849da3e2fa379e5c60d666cb4de
$ make fix

Using https://github.com/kubernetes-sigs/boskos/commit/37bd9bb41b86a849da3e2fa379e5c60d666cb4de because that's the last commit of boskos which doesn't pull in k/test-infra code which uses Generics.

Signed-off-by: Nabarun Pal pal.nabarun95@gmail.com

palnabarun commented 1 year ago

Interesting! https://github.com/kubernetes/test-infra/commit/0e771b68d04fcb5421e36ff68d98a85d60f0e508 introduces generics. We can't use that here since kubetest2 still uses Go 1.17. Updating Go is a different concern right now.

# k8s.io/test-infra/prow/config/secret
../../../../pkg/mod/k8s.io/test-infra@v0.0.0-20220726093710-8f4d4ce3e334/prow/config/secret/agent.go:73:6: missing function body
../../../../pkg/mod/k8s.io/test-infra@v0.0.0-20220726093710-8f4d4ce3e334/prow/config/secret/agent.go:73:19: syntax error: unexpected [, expecting (
../../../../pkg/mod/k8s.io/test-infra@v0.0.0-20220726093710-8f4d4ce3e334/prow/config/secret/agent.go:111:51: syntax error: unexpected {, expecting comma or )
../../../../pkg/mod/k8s.io/test-infra@v0.0.0-20220726093710-8f4d4ce3e334/prow/config/secret/agent.go:113:22: syntax error: unexpected ] at end of statement
note: module requires Go 1.18 

Trying to find a Boskos version which uses a k/test-infra version before generics were added.

ameukam commented 1 year ago

/assign @BenTheElder @MushuEE

dims commented 1 year ago

/approve /lgtm

BenTheElder commented 1 year ago

Using https://github.com/kubernetes-sigs/boskos/commit/37bd9bb41b86a849da3e2fa379e5c60d666cb4de because that's the last commit of boskos which doesn't pull in k/test-infra code which uses Generics.

ugh, we really should consider breaking the dependency on test-infra here, that is not a fun repo to have as a dependency :/

thanks for the PR

BenTheElder commented 1 year ago

/lgtm /approve

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, dims, palnabarun

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: - ~~[OWNERS](https://github.com/kubernetes-sigs/kubetest2/blob/master/OWNERS)~~ [BenTheElder] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
palnabarun commented 1 year ago

ugh, we really should consider breaking the dependency on test-infra here, that is not a fun repo to have as a dependency :/

I agree. We need to see how we can change that dependency.

BenTheElder commented 1 year ago

The best answer is probably copying in our own boskos client like @munnerz did in test-infra, boskos client is pretty simple and stable, and the API is not really changing.

I'm going to file a test-infra bug about generics use though, this is problematic given we have kubernetes versions without it yet.

palnabarun commented 1 year ago

I'm going to file a test-infra bug about generics use though, this is problematic given we have kubernetes versions without it yet.

Yes please. Thank you! I was wondering how we got the generics change in. I like the policy you wrote in https://github.com/kubernetes/community/pull/6693 and should ideally want to get it in soon.

BenTheElder commented 1 year ago

https://github.com/kubernetes/test-infra/issues/26989

palnabarun commented 1 year ago

The best answer is probably copying in our own boskos client like @munnerz did in test-infra, boskos client is pretty simple and stable, and the API is not really changing.

I will try to make this change.