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

.github/workflows: re-enable coverage in BPF tests #23291

Merged
merged 1 commit into from Mar 2, 2023
Merged

.github/workflows: re-enable coverage in BPF tests #23291

merged 1 commit into from Mar 2, 2023

Conversation

tklauser
Copy link
Member

With the recent CoverBee version bump in commit a38fa32 ("vendor: Bumped CoverBee to v0.3.0 and cilium/ebpf to v0.10.0") we can enable coverage reporting for BPF tests again.

This reverts commit edd2abf (".github/workflows: disable coverage in BPF tests").

Fixes #22088

@tklauser tklauser added the release-note/ci This PR makes changes to the CI. label Jan 24, 2023
@tklauser tklauser changed the title .github/workflows: re-enable coverage in BPF tests" .github/workflows: re-enable coverage in BPF tests Jan 24, 2023
@tklauser tklauser force-pushed the pr/tklauser/bpf-lint-coverage branch from c013ee7 to e989f2b Compare January 24, 2023 13:32
@tklauser
Copy link
Member Author

tklauser commented Jan 24, 2023 •
edited

/cc @dylandreimerink

Looks like CoverBee is panic'ing in a different place now: https://github.com/cilium/cilium/actions/runs/3996735051/jobs/6857147501

panic: runtime error: index out of range [2667] with length 2667 [recovered]
	panic: runtime error: index out of range [2667] with length 2667

goroutine 7 [running]:
testing.tRunner.func1.2({0x1b4e1c0, 0xc00b73ab58})
	/opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:1396 +0x24e
testing.tRunner.func1()
	/opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:1399 +0x39f
panic({0x1b4e1c0, 0xc00b73ab58})
	/opt/hostedtoolcache/go/1.19.5/x64/src/runtime/panic.go:884 +0x212
github.com/cilium/coverbee.InstrumentCollection(0xc000c14750, {0x0, 0x0})
	/home/runner/work/cilium/cilium/vendor/github.com/cilium/coverbee/instrumentation.go:277 +0x25b4
github.com/cilium/coverbee.InstrumentAndLoadCollection(0x7efed9023fff?, {{{0x0, 0x0}, {0x0, 0x0, 0x0}}, {0x0, 0x4000000, 0x0, 0x0}, ...}, ...)
	/home/runner/work/cilium/cilium/vendor/github.com/cilium/coverbee/instrumentation.go:27 +0x46
github.com/cilium/cilium/test/bpf_tests.loadAndRunSpec(0xc0003eb040, {0x1f7d040?, 0xc0000bad00?}, {0x0, 0x0})
	/home/runner/work/cilium/cilium/test/bpf_tests/bpf_test.go:143 +0x225
github.com/cilium/cilium/test/bpf_tests.TestBPF(0xc0003eb040)
	/home/runner/work/cilium/cilium/test/bpf_tests/bpf_test.go:91 +0x73a
testing.tRunner(0xc0003eb040, 0x1d9ce58)
	/opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:1446 +0x10b
created by testing.(*T).Run
	/opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:1493 +0x35f
FAIL	github.com/cilium/cilium/test/bpf_tests	22.973s
FAIL

@github-actions
Copy link

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Feb 24, 2023
@tklauser tklauser added dont-merge/blocked Another PR must be merged before this one. and removed stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. labels Mar 1, 2023
@tklauser
Copy link
Member Author

tklauser commented Mar 1, 2023

blocked on #24094

@tklauser tklauser force-pushed the pr/tklauser/bpf-lint-coverage branch 2 times, most recently from c084a2f to 570db59 Compare March 1, 2023 09:27
@tklauser
Copy link
Member Author

tklauser commented Mar 1, 2023 •
edited

BPF tests passed with test commit and #24094 pulled in: https://github.com/cilium/cilium/actions/runs/4302018934

Removing test commit and marking as ready for review.

@tklauser tklauser marked this pull request as ready for review March 1, 2023 09:37
@tklauser tklauser requested review from a team as code owners March 1, 2023 09:37
@tklauser tklauser force-pushed the pr/tklauser/bpf-lint-coverage branch from 570db59 to d37f1d7 Compare March 1, 2023 09:37
@tklauser tklauser mentioned this pull request Mar 1, 2023
Merged
@tklauser tklauser removed the dont-merge/blocked Another PR must be merged before this one. label Mar 1, 2023
@tklauser
.github/workflows: re-enable coverage in BPF tests
8cfa043 
With the recent CoverBee version bump to v0.3.1 we can enable coverage
reporting for BPF tests again.

This reverts commit edd2abf (".github/workflows: disable coverage
in BPF tests").

Fixes #22088

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser tklauser force-pushed the pr/tklauser/bpf-lint-coverage branch from d37f1d7 to 8cfa043 Compare March 1, 2023 14:49
sayboras
sayboras approved these changes Mar 2, 2023
View reviewed changes
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@sayboras
Copy link
Member

sayboras commented Mar 2, 2023

The related job is skipped in this PR (as there is no bpf change), however, I have verified locally, so it's safe to merge.

https://github.com/cilium/cilium/actions/runs/4304772254/jobs/7506267906

@sayboras sayboras added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 2, 2023
@sayboras sayboras merged commit 2b5b005 into master Mar 2, 2023
@sayboras sayboras deleted the pr/tklauser/bpf-lint-coverage branch March 2, 2023 01:27
@tklauser
Copy link
Member Author

tklauser commented Mar 2, 2023

The related job is skipped in this PR (as there is no bpf change), however, I have verified locally, so it's safe to merge.

https://github.com/cilium/cilium/actions/runs/4304772254/jobs/7506267906

@sayboras FWIW this was also successfully run with a test commit that triggered the respective test, see #23291 (comment)

@sayboras
Copy link
Member

sayboras commented Mar 2, 2023

oops my bad 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brlbil brlbil brlbil approved these changes

@sayboras sayboras sayboras approved these changes

@rgo3 rgo3 rgo3 approved these changes

Assignees
No one assigned
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CI: re-enable coverage reporting in .github/workflows/lint-bpf-checks.yaml
4 participants
@tklauser @sayboras @brlbil @rgo3