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

bpftrace-compiler: add run-via-edgeview #4230

Merged

Conversation

christoph-zededa
Copy link
Contributor

In more RL like scenarios, it is neither possible to reach EVE via ssh nor via direct network access. For these cases edgeview was developed.
This new feature leverages edgeview to run bpftrace scripts in more tough environments.

eve-tools/bpftrace-compiler/README.md Outdated Show resolved Hide resolved
eve-tools/bpftrace-compiler/main.go Show resolved Hide resolved
eve-tools/bpftrace-compiler/edgeview.go Outdated Show resolved Hide resolved
eve-tools/bpftrace-compiler/edgeview.go Show resolved Hide resolved
}

func (er edgeviewRun) runEdgeview(edgeviewParam string, matchOutput string) (string, error) {
cmd := exec.Command("bash", er.path, edgeviewParam)
Copy link
Member

Choose a reason for hiding this comment

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

As far as I can recall, 'sh' is the default interpreter for the edge-view scripts. We can either set it here, or just run the script directly as it already contains the shebang line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cannot run it directly because when I download it, the +x is not set.

Does $((9001 + $multi)) work in sh?

Copy link
Member

Choose a reason for hiding this comment

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

Nope, sh does not know it. A good catch for @naiming-zededa. Yep then setting bash is more than justified.

eve-tools/bpftrace-compiler/edgeview.go Show resolved Hide resolved
eve-tools/bpftrace-compiler/edgeview.go Show resolved Hide resolved
eve-tools/bpftrace-compiler/edgeview.go Outdated Show resolved Hide resolved
eve-tools/bpftrace-compiler/edgeview.go Show resolved Hide resolved
eve-tools/bpftrace-compiler/edgeview.go Show resolved Hide resolved
@christoph-zededa christoph-zededa force-pushed the http_debug_add_bpftrace_edgeview branch 5 times, most recently from 72c30c6 to 7dce673 Compare September 11, 2024 12:20
@OhmSpectator
Copy link
Member

@christoph-zededa, please ping me when you think all comments are addressed.

@christoph-zededa
Copy link
Contributor Author

@christoph-zededa, please ping me when you think all comments are addressed.

I think I addressed everything, @OhmSpectator ping.

Copy link
Member

@OhmSpectator OhmSpectator 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 enough to me.

In more RL like scenarios, it is neither possible to reach
EVE via ssh nor via direct network access. For these cases
edgeview was developed.
This new feature leverages edgeview to run bpftrace scripts
in more tough environments.

Signed-off-by: Christoph Ostarek <christoph@zededa.com>
@OhmSpectator
Copy link
Member

@christoph-zededa, do you think merging now is okay without waiting for all the test runs? I don't see this PR should affect any functionality checked by these tests...

@christoph-zededa
Copy link
Contributor Author

@christoph-zededa, do you think merging now is okay without waiting for all the test runs? I don't see this PR should affect any functionality checked by these tests...

I am okay with it, but we're also not in such a hurry not being able to wait for it ...

@OhmSpectator
Copy link
Member

I am okay with it, but we're also not in such a hurry not being able to wait for it ...

Just not to waste the runners' time.

@OhmSpectator OhmSpectator merged commit 351a451 into lf-edge:master Sep 12, 2024
45 of 48 checks passed
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