-
Notifications
You must be signed in to change notification settings - Fork 3.4k
ci: Fix CI-Fuzz Build failures #40728
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
Conversation
93bd463
to
74ff276
Compare
08a3c4e
to
446cee3
Compare
Commit 6994b8a does not match "(?m)^Signed-off-by:". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
Already being copied in cilium/cilium#40728 Signed-off-by: lomackie <me@louismackie.com>
bd4a095
to
9d46054
Compare
* Remove elf fuzzer Source was deleted by cilium/cilium@03b614f Signed-off-by: lomackie <me@louismackie.com> * Remove bgp config fuzzer Source was deleted by cilium@cilium/c2b935042f3efe44eecbe434a312256237c4960f Signed-off-by: lomackie <me@louismackie.com> * Remove monitor fuzzers These were removed by #467 Signed-off-by: lomackie <me@louismackie.com> * Run separate cilium/proxy fuzz build proxylib removed from cilium/cilium in cilium/cilium@51fc881 and moved to cilium/proxy CXXFLAGS updated to resolve linker errors in final clang++ step Signed-off-by: lomackie <me@louismackie.com> * Remove bpf_fuzzer.go BPFFSMigration was removed by cilium/cilium@8f5e88f Signed-off-by: lomackie <me@louismackie.com> * Remove logger setup Moved to slog as part of cilium/cilium@5a54846 Signed-off-by: lomackie <me@louismackie.com> * Update NewMonitorFormatter args Align with cilium/cilium@4d33968 Signed-off-by: lomackie <me@louismackie.com> * Remove l4_test Already being copied in cilium/cilium#40728 Signed-off-by: lomackie <me@louismackie.com> * Symlink cilium/proxy under GOPATH The oss-fuzz action handles this for cilium/cilium so we need to do it manually for cilium/proxy Signed-off-by: lomackie <me@louismackie.com> --------- Signed-off-by: lomackie <me@louismackie.com>
Yes I see the issue, thanks. I've updated all the fuzzers to use a The only test that seems to fail now is |
I think with these 3 PRs merged everything should build and run, I have all 14 fuzzers building and running successfully locally: |
@lomackie nice. Any order in which these should be merged? |
/test |
I think as long as they all go in before this one it doesn't particularly matter. |
Source file was deleted by 9d3c5e5 Fixes: cilium#37885 Signed-off-by: lomackie <me@louismackie.com>
l4_filter_test.go requires perSelectorPolicyToString Signed-off-by: lomackie <me@louismackie.com>
hivetest.Logger(t) isn't compatible with go-fuzz as the replacement *testing.T does not implement Chdir() hivetest.Logger(t) was been introduced by 5a54846, while CI Fuzz was not working. Replacing these with a slog.Logger that outputs nothing. Signed-off-by: lomackie <me@louismackie.com>
Annoyingly my oss-fuzz PR fails its CI because the project CIFuzz build doesn't work (which this PR is supposed to fix). google/oss-fuzz#13746 So this one will need to be merged first. The fuzzers should still be green without my other PRs merged in first, there will just be one fuzzer missing from the build and runs. I think some of the ginkgo tests are failing due to #40801 so hopefully a rebase might fix it, but I'm not sure if that first failing test is related to that issue. Are you able to rerun the test suite now that I've rebased? Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
/test |
Thanks for this @lomackie . Happy to see the CIFuzz workflow passing again 🎉 |
See cilium/cilium#40728 and cncf/cncf-fuzzing#557 for full context Signed-off-by: lomackie <me@louismackie.com>
Please ensure your pull request adheres to the following guidelines:
description and a
Fixes: #XXX
line if the commit addresses a particularGitHub issue.
Fixes: <commit-id>
tag, thenplease add the commit author[s] as reviewer[s] to this issue.
This PR causes all fuzzers to build successfully (tested locally using my fork of oss-fuzz here: lomackie/oss-fuzz@e59bb05). I'm not entirely sure if it will build perfectly in CI but this workflow was green a few commits ago (doesn't account for any of the cncf-fuzzing changes - but those can fail without the build failing): https://github.com/cilium/cilium/actions/runs/16532943538
Linked PRs:
set -e
- is that intended behaviour?Given there are now going to be more fuzzers running which haven't run for a while I'd expect to see a bunch of tests failing, I can't see the original failure in the linked issue as the build logs have expired, but I'm assuming this resolving just the build errors is okay, and the tests themselves are a separate issue.
Fixes: #37885