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

Flaky results for logcheck #23

Closed
marquiz opened this issue Aug 8, 2023 · 5 comments · Fixed by #24
Closed

Flaky results for logcheck #23

marquiz opened this issue Aug 8, 2023 · 5 comments · Fixed by #24
Labels
needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@marquiz
Copy link

marquiz commented Aug 8, 2023

Running logcheck seemingly spuriously fails. Happens both locally and in CI. An example of running locally:

$ logcheck -config scripts/test-infra/logcheck.conf ./cmd/... ./pkg/...  ./source/...
$ logcheck -config scripts/test-infra/logcheck.conf ./cmd/... ./pkg/...  ./source/...
$ logcheck -config scripts/test-infra/logcheck.conf ./cmd/... ./pkg/...  ./source/...
$ logcheck -config scripts/test-infra/logcheck.conf ./cmd/... ./pkg/...  ./source/...
/home/marquiz/go/git/node-feature-discovery/pkg/nfd-master/nfd-master.go:349:13: lis.Close undefined (type "net".Listener has no field or method Close)
logcheck: analysis skipped due to errors in package
logcheck: analysis skipped due to errors in package
logcheck: analysis skipped due to errors in package
logcheck: analysis skipped due to errors in package

This happens at least in the node-feature-discovery project.

@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Aug 8, 2023
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If logtools contributors determine 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.

@pohly
Copy link
Contributor

pohly commented Aug 8, 2023

Logcheck just uses golang.org/x/tools/go/analysis. It doesn't get involved until after parsing is done. This error must be somewhere in that area, not in logcheck itself.

Which Go version is used to build logcheck? Updating to the latest might help.

@pohly
Copy link
Contributor

pohly commented Aug 8, 2023

I was able to reproduce the failure locally with Go 1.21rc. But golang.org/x/tools/go/analysis isn't part of the Go release and logcheck was using a pretty old version of it. Updating dependencies seems to have fixed this for me.

Before:

$ $GOPATH/bin/stress -p 1 /tmp/logcheck -config scripts/test-infra/logcheck.conf ./cmd/... ./pkg/...  ./source/...
...
15s: 12 runs so far, 6 failures (50.00%)
^C

After:

1m20s: 72 runs so far, 0 failures

@marquiz
Copy link
Author

marquiz commented Aug 8, 2023

Thanks for the really quick response on this one. What is the process of requesting a new release to be tagged? 😊

@pohly
Copy link
Contributor

pohly commented Aug 8, 2023

Reminding me 😄... done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants