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.

Sign up for GitHub

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

Jump to bottom

pull-kubernetes-verify-govet-levee fails everything now #111452

Closed
MikeSpreitzer opened this issue Jul 27, 2022 · 24 comments
Closed

pull-kubernetes-verify-govet-levee fails everything now #111452

MikeSpreitzer opened this issue Jul 27, 2022 · 24 comments
Labels
kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. sig/testing Categorizes an issue or PR as relevant to SIG Testing. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@MikeSpreitzer
Copy link
Member

Which jobs are failing?

pull-kubernetes-verify-govet-levee

Which tests are failing?

verify: govet-levee

Since when has it been failing?

Failing everything from 7:35 PM EDT July 26.
Failing almost everything from 5:29 PM EDT July 26.

Testgrid link

https://k8s-testgrid.appspot.com/presubmits-kubernetes-blocking#pull-kubernetes-verify-govet-levee

Reason for failure (if possible)

No response

Anything else we need to know?

No response

Relevant SIG(s)

/sig testing
@MikeSpreitzer MikeSpreitzer added the kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. label Jul 27, 2022
@k8s-ci-robot k8s-ci-robot added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Jul 27, 2022
@k8s-ci-robot
Copy link
Contributor

@MikeSpreitzer: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jul 27, 2022
@MikeSpreitzer
Copy link
Member Author

There is a Slack message in #sig-testing associating this with #111449. Maybe from @heyste ?

@MikeSpreitzer
Copy link
Member Author

/priority critical-urgent

@k8s-ci-robot k8s-ci-robot added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Jul 27, 2022
@MikeSpreitzer MikeSpreitzer mentioned this issue Jul 27, 2022
Merged
@nikhita
Copy link
Member

nikhita commented Jul 27, 2022 •
edited

Looks like it's due to golang/go#48525. golang/x/tools doesn’t seem to support type parameters yet.

Edit: but this should have affected 1.18 too, not sure why it only fails on 1.19 yet.

@nikhita
Copy link
Member

nikhita commented Jul 27, 2022

Why the tests didn't fail in #111254:

@MadhavJivrajani
Copy link
Contributor

For future - @palnabarun, can we document this, where we need to wait for the image bump to propagate to the CI so that changes such as #111254 can test on that?

xref: kubernetes/sig-release#1972

@MadhavJivrajani
Copy link
Contributor

Also, looks like adding support for type parameters in golang/x/tools is bumped to go1.20 release: golang/go#48525 (comment)

@MadhavJivrajani
Copy link
Contributor

Not sure what happened in go1.19 that is causing the panic, the go issue linked by @nikhita effects go1.18 as well

@nikhita
Copy link
Member

nikhita commented Jul 27, 2022 •
edited

go 1.19 is the first release to use generics in the standard library though - https://pkg.go.dev/sync/atomic@go1.19rc2#Pointer 🤔

@nikhita
Copy link
Member

nikhita commented Jul 27, 2022

Let's open an issue in the go repo? If this is due to generics in the std lib, I'd expect the go team to prioritize fixing it.

@MadhavJivrajani
Copy link
Contributor

I can create one, worst case we'll understand a little more about Go!

@MadhavJivrajani
Copy link
Contributor

cc @dims @liggitt

@chendave chendave mentioned this issue Jul 27, 2022
Merged
@MadhavJivrajani MadhavJivrajani mentioned this issue Jul 27, 2022
Open
@MadhavJivrajani
Copy link
Contributor

Created: golang/go#54086

@MadhavJivrajani
Copy link
Contributor

MadhavJivrajani commented Jul 27, 2022 •
edited

Trying to think of how to unblock the CI for now:

So far:

I can think of 2 ways to unblock the CI for now:

  • disable the govet-levee check
    or
  • revert the go1.19rc2 switch over for the govet-levee job.

Also, as per golang/go#48525, further work on this is moved to the go1.20 release, unless we are able to fix it in upstream Go as part of 1.19, waiting for 1.19 GA also wouldn't help I guess.

@nikhita
Copy link
Member

nikhita commented Jul 27, 2022 •
edited

revert the go1.19rc2 switch over for the govet-levee job

This might be easier/better than disabling the govet-levee check completely and would help unblock master for now.

We could also keep the job on go1.18 until the Go team fixes it.

This was referenced Jul 27, 2022
Merged
Merged
@nikhita
Copy link
Member

nikhita commented Jul 27, 2022

Created kubernetes/test-infra#26931 and kubernetes/test-infra#26932 to move the test back to go1.18 to unblock master for now (in case we want to go with option 2 above).

@nikhita
Copy link
Member

nikhita commented Jul 27, 2022

Kicked off the job at #111449 (comment) after kubernetes/test-infra#26931 merged, the job is now green - https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/111449/pull-kubernetes-verify-govet-levee/1552238594398097408

@dims
Copy link
Member

dims commented Jul 27, 2022

NOTE: this is in a tool we use leevee. x/tools/go/ssa is NOT used by k8s itself. So yes +1 to use older image for this for now. and keep going forward.

@BenTheElder
Copy link
Member

using an older go version here means a skew in the go parser, it's probably fine in this instance, but as a general rule we probably want linters / analysis tools on the same go version.

How long do we expect to keep this out of sync? Do we have a path forward?

@BenTheElder BenTheElder added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 29, 2022
@dims
Copy link
Member

dims commented Jul 29, 2022

@BenTheElder the forward path is folks responsible for x/tools/go/ssa to fix things on their end - golang/go#48525

@BenTheElder
Copy link
Member

I mean on our end it is only reasonable to “fix” by rolling back go for one job if we expect to be able to do so short term.

it looks like we can hope for a fix soon golang/go#48525 (comment)

@timothy-king
Copy link

I left a description of a fix on golang/go#54086 (comment).

@MadhavJivrajani
Copy link
Contributor

Thank you, @timothy-king! ♥️

@dims dims mentioned this issue Aug 4, 2022
Merged
@MadhavJivrajani MadhavJivrajani mentioned this issue Aug 4, 2022
Merged
timothy-king added a commit to timothy-king/go-flow-levee that referenced this issue Aug 4, 2022
@timothy-king
Update to dependency on golang.org/x/tools to v0.1.12.
6e3ffe4 
This prevents crashes on Go code containing generics. The standard library now contains generics at head and levee crashes for folks using newer toolchains.

See kubernetes/kubernetes#111452.
This was referenced Aug 4, 2022
Closed
Merged
PurelyApplied pushed a commit to google/go-flow-levee that referenced this issue Aug 9, 2022
@timothy-king
Update to dependency on golang.org/x/tools to v0.1.12. (#327)
f056bc4 
* Update to dependency on golang.org/x/tools to v0.1.12.

This prevents crashes on Go code containing generics. The standard library now contains generics at head and levee crashes for folks using newer toolchains.

See kubernetes/kubernetes#111452.

* Update GH workflow to use go 1.18.
@dims
Copy link
Member

dims commented Sep 25, 2022

back on its feet now.

@dims dims closed this as completed Sep 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. sig/testing Categorizes an issue or PR as relevant to SIG Testing. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@dims @timothy-king @BenTheElder @MadhavJivrajani @MikeSpreitzer @nikhita @k8s-ci-robot