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

run-vmtest: action to test bpf using danobi/vmtest #117

Merged
merged 3 commits into from
Jan 25, 2024

Conversation

chantra
Copy link
Collaborator

@chantra chantra commented Jan 12, 2024

The first commit message explains the rationale. Copied below.

Note that this PR only adds a new action and is essentially a no-op until kernel-patches/vmtest is modified.

Test done:

Pointed kernel-patches/vmtest to use chantra/libbpf-ci/run-vmtest@vmtest (kernel-patches/vmtest#243):
https://github.com/kernel-patches/vmtest/actions/runs/7506022729/job/20436527154?pr=243

To validate Meta's veristat, hacked up a PR on kernel-patches/bpf#6245:

https://github.com/kernel-patches/bpf/actions/runs/7496945955/job/20409897434?pr=6245

First commit message:

Add a new action to mimick what used to be done by run-qemu.

This action is roughly a cp run-qemu run-vmtest in term of functionalities.
See end of this commit message for a rationale of this change.

Just like run-qemu assumes the presence of a rootfs which is provisioned with
a copy of the script from ci/vmtest/run_selftests.sh, run-vmtest assumes the
presence of ci/vmtest/vmtest_selftests.sh and will run it.

vmtest_selftests.sh functionally does the same as run_selftests.sh with a
few adjustments to make it work in the vmtest world` and leave in the "callee"
repository, not libbpf-ci.

print_test_summary.py was copied over unchanged. Later diff will remove the
version from run-qemu and point to this one instead.

action.yml needs to install a few tools that were historically baked in the rootfs.
A bunch of parameters are now historical.... this diff does not attempt to remove them
yet. This will be address later too and will probably change as libbpf/libbpf use case
is merged in.

run.sh gets rid of the logic to create the qemu one-liner as well as the
downloading of files from within the rootfs, and adjust it to using files on
disk that were left over by the test run.

Full end-to-end testing will be done through a PR in https://github.com/kernel-patches/vmtest

== Main functional difference between run-qemu and run-vmtest.

run-qemu runs the kernel inside a rootfs which is isolated from the host FS,
meaning that we potentially have a host in which we build the kernel/selftests
which is different than the host we run in. Causing depedency issues, see #83 and
to some extends #103 .
We could work around this with statically built binaries, but because of the different
rootfs, we end up having to maintain rootfs and doing a dance of copying files
in and out of the rootfs (through the prepare-rootfs action), which is slow, pulls
a fair amount of bytes through the network...

run-vmtest on the other hand does mount the host rootfs (read-only) and shares the
current directory (read-write) under /mnt/vmtest.
This resolves 2 things for us.

  1. We don't have libraries incompatibilitie issues anymore as the building OS
    and the OS running the tests will be the same.
  2. We don't need to copy files in and out of the root fs. The files are already
    on the FS seen by the VM, and files dumped by the tests are directly accessible
    outside the VM.

On top of this, vmtest also handles the peculiarities of crafting the right
qemu one-liner.

Add a new action to mimick what used to be done by `run-qemu`.

This action is roughly a `cp run-qemu run-vmtest` in term of functionalities.
See end of this commit message for a rationale of this change.

Just like `run-qemu` assumes the presence of a rootfs which is provisioned with
a copy of the script from `ci/vmtest/run_selftests.sh`, `run-vmtest` assumes the
presence of `ci/vmtest/vmtest_selftests.sh` and will run it.

`vmtest_selftests.sh` functionally does the same as `run_selftests.sh` with a
few adjustments to make it work in the `vmtest` world` and leave in the "callee"
repository, not libbpf-ci.

`print_test_summary.py` was copied over unchanged. Later diff will remove the
version from `run-qemu` and point to this one instead.

`action.yml` needs to install a few tools that were historically baked in the rootfs.
A bunch of parameters are now historical.... this diff does not attempt to remove them
yet. This will be address later too and will probably change as libbpf/libbpf use case
is merged in.

`run.sh` gets rid of the logic to create the `qemu` one-liner as well as the
downloading of files from within the rootfs, and adjust it to using files on
disk that were left over by the test run.

Full end-to-end testing will be done through a PR in https://github.com/kernel-patches/vmtest

== Main functional difference between `run-qemu` and `run-vmtest`.

`run-qemu` runs the kernel inside a rootfs which is isolated from the host FS,
meaning that we potentially have a host in which we build the kernel/selftests
which is different than the host we run in. Causing depedency issues, see libbpf#83 and
to some extends libbpf#103 .
We could work around this with statically built binaries, but because of the different
rootfs, we end up having to maintain rootfs *and* doing a dance of copying files
in and out of the rootfs (through the prepare-rootfs action), which is slow, pulls
a fair amount of bytes through the network...

`run-vmtest` on the other hand does mount the host rootfs (read-only) and shares the
current directory (read-write) under /mnt/vmtest.
This resolves 2 things for us.
1. We don't have libraries incompatibilitie issues anymore as the building OS
and the OS running the tests will be the same.
1. We don't need to copy files in and out of the root fs. The files are already
on the FS seen by the VM, and files dumped by the tests are directly accessible
outside the VM.

On top of this, `vmtest` also handles the peculiarities of crafting the right
qemu one-liner.

Signed-off-by: Manu Bretelle <[email protected]>
Avoid keeping duplicated version of the script by re-using the one now located
under run-vmtest.

Signed-off-by: Manu Bretelle <[email protected]>
vmtest_selftests.sh expect them to be space-separated.

Signed-off-by: Manu Bretelle <[email protected]>
@chantra
Copy link
Collaborator Author

chantra commented Jan 25, 2024

I have tested this again and it works as expected from my manual tests.

Currently using a fork of danobi/vmtest due to danobi/vmtest#40 which is affecting s390x.

From the look of it, using vmtest instead of our old mechanism saves us 170s (p90) when testing s390x as we do not need to prepare the chroot anymore.

@chantra chantra merged commit 1bb84d4 into libbpf:main Jan 25, 2024
2 checks passed
@chantra chantra deleted the vmtest branch January 25, 2024 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant