-
Notifications
You must be signed in to change notification settings - Fork 109
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
Support testing different kernel versions #1100
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1100 +/- ##
==========================================
- Coverage 79.44% 79.38% -0.07%
==========================================
Files 140 140
Lines 11514 11514
==========================================
- Hits 9147 9140 -7
- Misses 1825 1829 +4
- Partials 542 545 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
b52b012
to
64081af
Compare
bdb3148
to
8db65a7
Compare
8db65a7
to
1fe6d4f
Compare
6f1c0e2
to
8121d5c
Compare
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.
LGTM! Excellent work!
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.
Amazing work @rafaelroquetto, very much needed!
My question here is, do we want to run these tests on every PR? The pipeline is already slow, some times flaky, and quite complex to debug if something fails. Adding more tests here might be a burden for some pull requests.
In this PR you added two tests, but what would happen if we want to test more kernel and different architectures? My suggestion would be to move these tests to be built only in main. This has the cost that we have to monitor the health of builds/test in main as some thing may broke, but hopefully what can use some tooling like this: https://grafana.com/blog/2023/11/20/ci-cd-observability-via-opentelemetry-at-grafana-labs/
I think limiting when these tests run makes a lot of sense, I wonder if the best approach is to run them only if we have changes in the BPF generated files? PRs that don't touch that part shouldn't need to run the VM tests. This is similar how testing of the documentation works, unless we touch docs, doc testing isn't running. WDYT? |
I personally would like to run them on PRs, because they catch failures which I can't catch myself with my main development kernel version. |
Good idea - I can look into running these workflows only when the ebpf files have changed. This should address @marctc's concerns whilst not compromising coverage. |
8121d5c
to
3f8cada
Compare
Tests now passing:
They aren't appearing on the latest update as they have been disabled when there are no changes to the ebpf files. |
4c78ac3
to
a4264c5
Compare
1db7459
to
bf00bf7
Compare
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.
Looks great!
Virtual Machine Tests enables Beyla to be tested under different kernel versions and architectures. This works by exposing the path to the Beyla source code (i.e. this git repository) to the guest by means of mounting it to the '/build' directory. This allows Makefile targets to be invoked from within the guest, in particular those tied to integration tests. Analogous to a Docker container, changes to the '/build' directory are volatile and are neither reflected on the host nor preserved once the VM terminates.
The supported pre-compiled kernels can be found in the
kernels/$arch/
subdirectory, alongside their respective kernel config files.USAGE
Simply run 'make' to get started. This will build a new rootfs image and launch the VM using the default kernel specified in the Makefile. To select a different kernel version. The following environment variables can be used to specify the kernel version or architecture to run:
ARCH
: the target architecture (currently only x86_64 is supported)KERNEL_VER
: the kernel version to use OR;KERNEL
: path the kernel image to useTARGET_TEST
: the target to pass to Beyla's toplevel Makefile (defaults to run-integration-test-vm). When set to empty, the VM will drop onto an interactive shellFor debugging purposes, the VM also exposes an SSH endpoint at port
2222
. Theaccess credentials are
root:root
.See
test/vm/README
for further information, including on how to deploy new kernels.Resolves #878
Tip: reviewing individual commits might be simpler.