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

Add test for tracing bpf_check #281

Merged
merged 2 commits into from
Aug 8, 2022

Conversation

sarahfujimori
Copy link
Contributor

This is a unit test for tracing bpf verifier checks during program loads. We do this by first loading a CRD that traces the function bpf_check, then loading a second CRD and checking for events that record the correct program name and type. Calling the function GetDefaultObserverWithFile twice causes errors since we don’t want to create the fake k8s watcher, load the exporter, etc. again. Instead, for the second CRD, we only get and load the sensors. This approach requires importing a few extra packages, so it may be useful to find a better way to do this.

Signed-off-by: Sarah Fujimori [email protected]

This is a unit test for tracing bpf verifier checks during program loads. We do this by first loading a CRD that traces the function bpf_check, then loading a second CRD and checking for events that record the correct program name and type. Calling the function GetDefaultObserverWithFile twice causes errors since we don’t want to create the fake k8s watcher, load the exporter, etc. again. Instead, for the second CRD, we only get and load the sensors. This approach requires importing a few extra packages, so it may be useful to find a better way to do this.

Signed-off-by: Sarah Fujimori <[email protected]>
@sarahfujimori sarahfujimori requested a review from a team as a code owner August 2, 2022 18:20
@willfindlay
Copy link
Contributor

If you're concerned about bloat maybe we could just use cilium/ebpf to load a stub kprobe and attach it?

}

b := base.GetInitialSensor()
if err := b.Load(context.TODO(), option.Config.BpfDir, option.Config.MapDir, option.Config.CiliumDir); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good to me though should we also unload the program here in the flow?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would also be good to replace context.TODO()s here with a single context.Background() like we recently did for other tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the test logs show that the other program (sys_write kprobe) is already being unloaded at the end, so I think that part is fine..

Copy link
Contributor

@willfindlay willfindlay left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think it's OK to use sensor infra for running the test workload here, at least for now. Can always change our minds later if we want.

@jrfastab jrfastab merged commit 469231e into cilium:main Aug 8, 2022
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.

3 participants