Skip to content

Conversation

kkourt
Copy link
Contributor

@kkourt kkourt commented Sep 15, 2021

verifier-test.sh allows users to pass what programs to test via
environment variables. However, these variables are only used if they
are not unset and not empty. This means that we, for example, cannot
pass XDP_PROGS="" to avoid verifiying any xdp progs.

This patch modifies the check to only ignore the environment variables
if they are unset.

So now we can use verifier-test.sh to test only a single program:

root# TC_PROGS="bpf_host" XDP_PROGS="" CG_PROGS="" ./test/bpf/verifier-test.sh
=> Loading bpf_host.c:from-netdev...
=> Loading bpf_host.c:from-host...
=> Loading bpf_host.c:to-netdev...
=> Loading bpf_host.c:to-host...

Signed-off-by: Kornilios Kourtis kornilios@isovalent.com

verifier-test.sh allows users to pass what programs to test via
environment variables. However, these variables are only used if they
are  not unset and not empty. This means that we, for example, cannot
pass XDP_PROGS="" to avoid verifiying any xdp progs.

This patch modifies the check to only ignore the environment variables
if they are unset.

So now we can use verifier-test.sh to test only a single program:

root# TC_PROGS="bpf_host" XDP_PROGS="" CG_PROGS="" ./test/bpf/verifier-test.sh
=> Loading bpf_host.c:from-netdev...
=> Loading bpf_host.c:from-host...
=> Loading bpf_host.c:to-netdev...
=> Loading bpf_host.c:to-host...

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
@kkourt kkourt requested a review from a team as a code owner September 15, 2021 17:30
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Sep 15, 2021
@kkourt kkourt requested a review from borkmann September 15, 2021 17:30
@kkourt kkourt added the release-note/misc This PR makes changes that have no direct user impact. label Sep 15, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Sep 15, 2021
@kkourt
Copy link
Contributor Author

kkourt commented Sep 15, 2021

test-me-please

Job 'Cilium-PR-K8s-1.16-net-next' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sBookInfoDemoTest Bookinfo Demo Tests bookinfo demo

Failure Output

FAIL: DNS entry is not ready after timeout

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.16-net-next so I can create a new GitHub issue to track it.

Job 'Cilium-PR-K8s-1.16-net-next' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sDemosTest Tests Star Wars Demo

Failure Output

FAIL: DNS entry is not ready after timeout

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.16-net-next so I can create a new GitHub issue to track it.

@pchaigno pchaigno added area/CI Continuous Integration testing issue or flake area/CI-improvement Topic or proposal to improve the Continuous Integration workflow area/loader Impacts the loading of BPF programs into the kernel. labels Sep 21, 2021
@pchaigno
Copy link
Member

pchaigno commented Sep 30, 2021

Jenkins page expired:
test-1.16-netnext

@pchaigno
Copy link
Member

k8s-1.16-kernel-netnext failed with:

Considering that and the fact that both tests are unaffected by the changes in this PR, I'm marking ready to merge.

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Sep 30, 2021
@glibsm glibsm merged commit 422458d into master Sep 30, 2021
@glibsm glibsm deleted the pr/kkourt/verifier-test-empty-vars branch September 30, 2021 16:13
@pchaigno
Copy link
Member

Marking this pull request for backport into v1.9 and v1.10 given we'll also backport the extension of K8sVerifier (cf. #17470) which relies on this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake area/CI-improvement Topic or proposal to improve the Continuous Integration workflow area/loader Impacts the loading of BPF programs into the kernel. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants