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: introduce run-vmtest.env #166

Merged
merged 4 commits into from
Jan 14, 2025
Merged

Conversation

theihor
Copy link
Contributor

@theihor theihor commented Jan 10, 2025

Update run-vmtest action to source $VMTEST_CONFIGS/run-vmtest.env when present. For detais read a comment in ci/vmtest/configs/run-vmtest.env

Add scripts local to run-vmtest that handle the merge of multiple allow/denylists for BPF selftests.

Passing @filename as allow/denylist to test runners, instead of inlining comma-separated test names.

Add test_progs-bpf_gcc as a possible test runner in run-vmtest/run-bpf-selftests.sh

theihor added a commit to theihor/vmtest that referenced this pull request Jan 10, 2025
@theihor theihor marked this pull request as ready for review January 10, 2025 20:17
@theihor theihor requested a review from chantra January 10, 2025 20:17
@theihor
Copy link
Contributor Author

theihor commented Jan 10, 2025

@chantra I got GCC BPF working at #165, and then decided to split it into a couple of PRs. Not all changes there are specific to the GCC support, like this one.

@theihor theihor mentioned this pull request Jan 10, 2025
@theihor
Copy link
Contributor Author

theihor commented Jan 10, 2025

vmtest CI passes: kernel-patches/vmtest#326

Copy link
Collaborator

@chantra chantra left a comment

Choose a reason for hiding this comment

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

LGTM overall aside from the fact that I don't think this works correctly as is.

Is there a way to update https://github.com/libbpf/ci/blob/main/run-vmtest/action.yml or a README to document those environment variables needed by run-vmtest.env?

Also, I suppose this needs to be released in a version manner right? e.g landing this into main would break CI as https://github.com/kernel-patches/vmtest/blob/78dc9a42e4c3cdbad127814f91219ee20e40dbf2/.github/workflows/kernel-test.yml#L67 fir instance, is unaware of this yet.

.github/workflows/kernel-build.yml Show resolved Hide resolved
.github/workflows/kernel-build.yml Show resolved Hide resolved
run-vmtest/merge_test_lists.py Outdated Show resolved Hide resolved
run-vmtest/merge_test_lists.py Outdated Show resolved Hide resolved
run-vmtest/prepare-bpf-selftests.sh Outdated Show resolved Hide resolved
@theihor
Copy link
Contributor Author

theihor commented Jan 14, 2025

@chantra I added "unit tests" for run-vmtest, and rewrote the "merge lists" script. Please take a look when you get a chance.

Copy link
Collaborator

@chantra chantra left a comment

Choose a reason for hiding this comment

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

Thanks, there is an issue in the rewritten script which was also surfaced by CI, but otherwise this looks go to go.

run-vmtest/prepare-bpf-selftests.sh Show resolved Hide resolved
run-vmtest/normalize_bpf_test_names.py Show resolved Hide resolved
@theihor theihor force-pushed the run-vmtest-update branch 2 times, most recently from d949c60 to 05b25cd Compare January 14, 2025 02:45
@theihor
Copy link
Contributor Author

theihor commented Jan 14, 2025

@chantra Didn't answer these in the first pass through your comments. Answering below.

Is there a way to update https://github.com/libbpf/ci/blob/main/run-vmtest/action.yml or a README to document those environment variables needed by run-vmtest.env?

It's tricky. run-vmtest action doesn't really care about the variables used by run-vmtest.env. It's simple "source", so whatever is in the file will be read.

Presumably, author of run-vmtest.env is also an author of a workflow calling libbpf/ci/run-vmtest action, and so they should know what variables to set.

On the other hand, there are SELFTESTS_BPF_{ALLOW,DENY}LIST_FILES, which is what run-vmtest.env must export (if it exists), because prepare-bpf-selftests.sh depends on this...

I think you're right, a note about this in the readme is warranted.

Also, I suppose this needs to be released in a version manner right? e.g landing this into main would break CI as https://github.com/kernel-patches/vmtest/blob/78dc9a42e4c3cdbad127814f91219ee20e40dbf2/.github/workflows/kernel-test.yml#L67 fir instance, is unaware of this yet.

Yes. This is not an issue for a while now. BPF CI is pinned to v2 currently, so a push to main won't break anything. I usually push to main, then check the update on vmtest, then update the current tag or push a new one (depending on wethere changes are significant).

Update run-vmtest action to source $VMTEST_CONFIGS/run-vmtest.env when
present. For detais read a comment in ci/vmtest/configs/run-vmtest.env

Add scripts local to run-vmtest that handle the merge of multiple
allow/denylists for BPF selftests.

Passing @filename as allow/denylist to test runners, instead of
inlining comma-separated test names.

Add test_progs-bpf_gcc as a possible test runner in
run-vmtest/run-bpf-selftests.sh

Signed-off-by: Ihor Solodrai <[email protected]>
Signed-off-by: Ihor Solodrai <[email protected]>
theihor added a commit to theihor/vmtest that referenced this pull request Jan 14, 2025
theihor added a commit to theihor/vmtest that referenced this pull request Jan 14, 2025
@theihor theihor self-assigned this Jan 14, 2025
@theihor
Copy link
Contributor Author

theihor commented Jan 14, 2025

vmtest pipeline has passed: https://github.com/kernel-patches/vmtest/actions/runs/12776803904

Merging now.

@theihor theihor merged commit cdaa7f0 into libbpf:main Jan 14, 2025
14 checks passed
theihor added a commit to kernel-patches/vmtest that referenced this pull request Jan 14, 2025
Add ci/vmtest/configs/run-vmtest.env to control allow/deny lists

Signed-off-by: Ihor Solodrai <[email protected]>
theihor added a commit to kernel-patches/vmtest that referenced this pull request Jan 14, 2025
* Add ci/vmtest/configs/run-vmtest.env to control allow/deny lists
* Use v3 of libbpf/ci actions in all workflows

Signed-off-by: Ihor Solodrai <[email protected]>
theihor added a commit to theihor/libbpf that referenced this pull request Jan 15, 2025
* vmtest.yml
  * use v3 of libbpf/ci actions
  * remove unnecessary selftests preparation steps
* ci/vmtest
  * remove unnecessary scripts and configs
  * add libbpf-specific run-vmtest.env [1]

[1] libbpf/ci#166
theihor added a commit to theihor/libbpf that referenced this pull request Jan 15, 2025
* vmtest.yml
  * use v3 of libbpf/ci actions
  * remove unnecessary selftests preparation steps
* ci/vmtest
  * remove unnecessary scripts and configs
  * add libbpf-specific run-vmtest.env [1]

[1] libbpf/ci#166

Signed-off-by: Ihor Solodrai <[email protected]>
anakryiko pushed a commit to libbpf/libbpf that referenced this pull request Jan 16, 2025
* vmtest.yml
  * use v3 of libbpf/ci actions
  * remove unnecessary selftests preparation steps
* ci/vmtest
  * remove unnecessary scripts and configs
  * add libbpf-specific run-vmtest.env [1]

[1] libbpf/ci#166

Signed-off-by: Ihor Solodrai <[email protected]>
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.

2 participants