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 cache, loading kernel modules, fixes #4266

Conversation

christoph-zededa
Copy link
Contributor

@christoph-zededa christoph-zededa commented Sep 17, 2024

  • 7e8649eaf config
  • don't use 127.1 forwarding
  • connect faster
  • ensure current kernel works
  • run tests in docker
  • factory method for qemu runner
  • generalize cobra cmds
  • cleanup ssh/sftp client
  • fix ssh auth
  • introduce caching
  • enable loading kernel modules
  • improve error handling
  • fix go.mod

@christoph-zededa christoph-zededa changed the title Http debug add bpftrace compiler cache bpftrace-compiler: add cache, loading kernel modules, fixes Sep 17, 2024
@christoph-zededa christoph-zededa force-pushed the http_debug_add_bpftrace_compiler-cache branch from 0508d74 to 68b8416 Compare September 17, 2024 21:48
no patchlevel expected as by `gopls`

Signed-off-by: Christoph Ostarek <[email protected]>
1 give some context to the error

2. if the user specified an unknown container type, it would not
   immediately fail, but instead give a confusing error message later

Signed-off-by: Christoph Ostarek <[email protected]>
this is needed in order to build bpftrace scripts that
use probes that come from modules

Signed-off-by: Christoph Ostarek <[email protected]>
@christoph-zededa christoph-zededa force-pushed the http_debug_add_bpftrace_compiler-cache branch from 68b8416 to a178d02 Compare September 18, 2024 08:25
unfortunately compiling the bpftrace script can take
a while, especially if userspace containers have to be
included into the image

Signed-off-by: Christoph Ostarek <[email protected]>
use one authenticator with several private ssh keys
instead of several authenticators with one private ssh
key
now ssh auth should work even if the second key is used

Signed-off-by: Christoph Ostarek <[email protected]>
remove unnecessary assignments - makes code more readable

Signed-off-by: Christoph Ostarek <[email protected]>
allow userspace containers and loading kernel modules for
all cobra commands
generalize the usage of flags

Signed-off-by: Christoph Ostarek <[email protected]>
instead of having the same code over and over, let's use a
factory method

Signed-off-by: Christoph Ostarek <[email protected]>
there it is possible to install qemu and run it

Signed-off-by: Christoph Ostarek <[email protected]>
a new test that checks kernel-commits.mk and does a basic
test to check that the kernel supports bpftrace

Signed-off-by: Christoph Ostarek <[email protected]>
by immediate breaking the reconnect loop if the connect try
was successful

Signed-off-by: Christoph Ostarek <[email protected]>
but use localhost, because this works even since before
commit 455bf4b

Signed-off-by: Christoph Ostarek <[email protected]>
@christoph-zededa christoph-zededa force-pushed the http_debug_add_bpftrace_compiler-cache branch from a178d02 to 50ba49e Compare September 18, 2024 10:41
@christoph-zededa christoph-zededa marked this pull request as ready for review September 18, 2024 11:00
@@ -241,6 +241,7 @@ func newEdgeviewCmd() *cobra.Command {
time.Sleep(time.Second)
continue
}
break
}

hr := newHTTPRun(fmt.Sprintf("localhost:%d", port))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be previous loop was not successful, should we fail here with an error if so?
Also 127.0.0.0 in the loop above and here is localhost. Not a big deal, but why not to make them identical?

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 added bd95726

otherwise it will leak mem and/or filedescriptor

Signed-off-by: Christoph Ostarek <[email protected]>
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM

@eriknordmark eriknordmark merged commit f6f03a6 into lf-edge:master Sep 25, 2024
42 of 43 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