new: dynamic builders #244

FedeDP · 2022-12-22T15:59:03Z

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area build
/area cmd
/area pkg

What this PR does / why we need it:

This PR implements the proposed improvements to builder images: #241 .

Basically, we do now allow to pass a list of docker repos in descending priority order.
falcosecurity/driverkit is always emplaced back, so that is has lowest available priority.

List of images is then dynamically and transparently loaded at runtime, given that images in the passed repositories follow this scheme for their name: driverkit-builder-(?P<target>[a-z0-9]+)-(?P<arch>x86_64|aarch64)(?P<gccVers>(_gcc[0-9]+.[0-9]+.[0-9]+)+)$, basically, it means something like this: myrepo/driverkit-builder-ubuntu-x86_64_gcc5.8.0_gcc7.0.0.

NOTE: provided images should expose full gcc semver fields (ie: even 0 ones), like the above example.
Internally, they should soft link gcc to the full semversioned name, like: ln -s /usr/bin/gcc-5 /usr/bin/gcc-5.0.0.
IMHO it should not be a big issue. Let me know wyt! We can try to find another solution if you think it is too time consuming doing that (but again, i don't really think so).

This PR should also improve the situation for @Lowaiz and its #238 patch.

"any" target builder images, are fallback images to be used when a specific-target one is missing. The algorithm goes as follow:

get required target GCC from either:
- command line argument (--gccversion) again, as a full semver string
- GCCVersionRequestor interface by the builder
- not-so-smart algorithm given a kernelrelease
then, load all available images from al specified docker repos
for each discovered image, create an entry for any of the provided gcc in a map whose key is $target_$gccversion
finally, try to find an image that provides target GCC, by first looking to $target_$gccversion, then, if it could not be found, looking at any_$gccversion
if any image is found, we are done
else, loop on all images that are either for target $target, or "any", to find the one that provides the nearest gcc version
we are over ;)
if you ask for a specific --builderimage, you must provide the requested GCC (or force a --gccversion); this was already enforced

Which issue(s) this PR fixes:

Fixes #241

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

new: added support for dynamic builder images

FedeDP · 2022-12-22T15:59:49Z

Note: CI will fail badly of course, because images cannot be found.

Moreover, i still need to update multiple docs files.

I just wanted to share this asap to get some comments ;)

FedeDP · 2022-12-22T16:00:42Z

Example test (i pushed an image to my own dockerhub):

./_output/bin/driverkit docker --kernelrelease 6.0.12.arch1-1 --target arch --output-module /tmp/arch.ko  --output-probe /tmp/arch.o -l debug --dockerrepo docker.io/fededp/driverkit
DEBU running without a configuration file
DEBU running with options                          arch=amd64 driverversion=master kernelrelease=6.0.12.arch1-1 kernelversion=1 output-module=/tmp/arch.ko output-probe=/tmp/arch.o repo-name=libs repo-org=falcosecurity target=arch
INFO driver building, it will take a few seconds   processor=docker
DEBU doing a new docker build
DEBU kernel header url found                       url="https://archive.archlinux.org/packages/l/linux-headers/linux-headers-6.0.12.arch1-1-x86_64.pkg.tar.zst"
DEBU Malformed image name: fededp/driverkit-builder
DEBU Malformed image name: fededp/driverkit
DEBU Malformed image name: fededp/driverkit-builder_buster
DEBU Malformed image name: fededp/driverkit-builder_bullseye
DEBU Malformed image name: fededp/driverkit-builder_bookworm
DEBU Malformed image name: falcosecurity/driverkit-builder
DEBU Malformed image name: falcosecurity/driverkit
DEBU Malformed image name: falcosecurity/driverkit-builder_bookworm
DEBU Malformed image name: falcosecurity/driverkit-builder_buster
DEBU Malformed image name: falcosecurity/driverkit-builder_bullseye
DEBU proposedGCC=10.0.0                            image=fededp/driverkit-builder-any-x86_64_gcc10.0.0_gcc9.0.0 targetGCC=12.0.0
DEBU proposedGCC=9.0.0                             image=fededp/driverkit-builder-any-x86_64_gcc10.0.0_gcc9.0.0 targetGCC=12.0.0
DEBU foundGCC=10.0.0                               targetGCC=12.0.0
DEBU starting container                            image="fededp/driverkit-builder-any-x86_64_gcc10.0.0_gcc9.0.0:latest"

FedeDP · 2022-12-22T16:03:42Z

Possible way to fix tests:

push all default images to eg: fededp dockerhub
update tests makefile to add --dockerrepo docker.io/fededp/driverkit
merge this
open a new PR to drop the makefile trick
we are done

FedeDP · 2022-12-22T16:25:04Z

As expected, integration tests are failing with:

_output/bin/driverkit docker -c test/x86_64/configs/al.yaml --builderimage auto:master -l debug --timeout 600
INFO using config file                             file=test/x86_64/configs/al.yaml
DEBU running with options                          arch=amd64 driverversion=master kernelrelease=4.14.128-87.105.amzn1.x86_64 kernelurls="[https://download.falco.org/fixtures/driverkit/x86_64_devel/kernel-devel-4.14.128-87.105.amzn1.x86_64.rpm]" kernelversion=1 output-module=/tmp/amazonlinux_x86_64.ko repo-name=libs repo-org=falcosecurity target=amazonlinux
INFO driver building, it will take a few seconds   processor=docker
DEBU doing a new docker build                     
DEBU kernel header url found                       url="https://download.falco.org/fixtures/driverkit/x86_64_devel/kernel-devel-4.14.128-87.105.amzn1.x86_64.rpm"
DEBU Malformed image name: falcosecurity/driverkit-builder 
DEBU Malformed image name: falcosecurity/driverkit 
DEBU Malformed image name: falcosecurity/driverkit-builder_bookworm 
DEBU Malformed image name: falcosecurity/driverkit-builder_buster 
DEBU Malformed image name: falcosecurity/driverkit-builder_bullseye 
2022/12/22 16:23:07 Could not load any builder image. Leaving.

FedeDP · 2023-01-09T08:59:38Z

Instead of naming the builder images using the aforementioned regex, we could build a solution that uses http docker registry API to fetch image labels for each image without actually pulling the image; this is done eg by this project: https://github.com/felicianotech/sonar/blob/7d98bae716997a3016c74ec5b1518601e5ca59ae/sonar/docker/label.go#L11
and explained on a medium article: https://medium.com/hackernoon/inspecting-docker-images-without-pulling-them-4de53d34a604

Let's assume this is in place, we could have multiple labels:

DriverkitArch:...
DriverkitGCC:4.8.0,5.0.0,7.0.0
DriverkitTarget: ...

In the end, this method allows for cleaner builder images dockerfiles; moreover has the big plus that LABELS and installed GCCs are declared in the very same context (ie: the dockerfile), therefore it is harder to have it wrong (while is much easier to get image name wrong instead).

At the same time, i am afraid that the http api could change (eg: being bumped to v3) and therefore we would need more work to keep our code up to date.

WDYT?

EDIT: i think i could give a try to this impl, perhaps in a single commit, and then we can decide which one we prefer ;)
I still want to collect some feedback on this as a whole!

FedeDP · 2023-01-10T09:07:34Z

On a second thought, the problem with the http registry API version is performance: imagine we need to scrape up to 100 builder images for each repo; this means we are doing 100 http requests for each repo listed; assuming a 10ms ping, we are losing 1s for the build for each repo listed.
Driverkit must be as quick as possible IMHO.

FedeDP · 2023-01-13T07:56:38Z

The new images command output example:

./_output/bin/driverkit images --kernelrelease 6.0.12.arch1-1 --target arch --output-module /tmp/arch.ko  --output-probe /tmp/arch.o -l debug --dockerrepo docker.io/fededp/driverkit
DEBU running without a configuration file
DEBU running with options                          arch=amd64 driverversion=master kernelrelease=6.0.12.arch1-1 kernelversion=1 output-module=/tmp/arch.ko output-probe=/tmp/arch.o repo-name=libs repo-org=falcosecurity target=arch
INFO listing images                                processor=images
|                         IMAGE                          | TARGET | ARCH  |  GCC   |
|--------------------------------------------------------|--------|-------|--------|
| fededp/driverkit-builder-any-x86_64_gcc10.0.0_gcc9.0.0 | any    | amd64 | 10.0.0 |
| fededp/driverkit-builder-any-x86_64_gcc10.0.0_gcc9.0.0 | any    | amd64 | 9.0.0  |

FedeDP · 2023-01-24T09:59:20Z

TODO:

update doc
provide an image that fixes Cannot Build CentOS 7: 3.10.0-1160.80.1.el7.x86_64 #236
add support for docker login for private repos? (this can also be post-poned when needed)
Test ...??

dwindsor

NOTE: provided images should expose full gcc semver fields (ie: even 0 ones), like the above example.
Internally, they should soft link gcc to the full semversioned name, like: ln -s /usr/bin/gcc-5 /usr/bin/gcc-5.0.0.
IMHO it should naot be a big issue. Let me know wyt! We can try to find another solution if you think it is too time consuming doing that (but again, i don't really think so).

Agreed, adopting this convention of linking to a semver gcc is not a big deal. Thanks for this feature! In terms of maintenance, we don't have to maintain the static map of gcc versions anymore correct?

FedeDP · 2023-01-31T07:49:26Z

Agreed, adopting this convention of linking to a semver gcc is not a big deal. Thanks for this feature! In terms of maintenance, we don't have to maintain the static map of gcc versions anymore correct?

Exactly!
I wish to at least provide a new image for centos to fix #236 ; can you help me? I tried with a centos one, but it is not working :(
Then, i'd need to fix tests before eventually merging this!

…d indexing given a docker repo. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

Ported makefile too. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

….0.Dockerfile. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

…epo to integration tests. This should allow us to test them. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

…by user, if any. In this way, we are able to support `--gccversion` properly. Moreover, `images` cmd is now `--gccversion` aware. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

FedeDP · 2023-02-03T14:28:49Z

After latest commits, i fixed --gccversion support in both images cmd and build.

Example:

./_output/bin/driverkit images --kernelrelease 6.0.12.arch1-1 --target arch --output-module /tmp/arch.ko  --output-probe /tmp/arch.o --dockerrepo docker.io/fededp/driverkit
INFO listing images                                processor=images
|                                      IMAGE                                       | TARGET | ARCH  |  GCC   |
|----------------------------------------------------------------------------------|--------|-------|--------|
| fededp/driverkit-builder-any-x86_64_gcc8.0.0_gcc6.0.0_gcc5.0.0_gcc4.9.0_gcc4.8.0 | any    | amd64 | 5.0.0  |
| fededp/driverkit-builder-any-x86_64_gcc12.0.0_gcc11.0.0                          | any    | amd64 | 12.0.0 |
| fededp/driverkit-builder-any-x86_64_gcc8.0.0_gcc6.0.0_gcc5.0.0_gcc4.9.0_gcc4.8.0 | any    | amd64 | 8.0.0  |
| fededp/driverkit-builder-any-x86_64_gcc8.0.0_gcc6.0.0_gcc5.0.0_gcc4.9.0_gcc4.8.0 | any    | amd64 | 6.0.0  |
| fededp/driverkit-builder-any-x86_64_gcc8.0.0_gcc6.0.0_gcc5.0.0_gcc4.9.0_gcc4.8.0 | any    | amd64 | 4.9.0  |
| fededp/driverkit-builder-any-x86_64_gcc8.0.0_gcc6.0.0_gcc5.0.0_gcc4.9.0_gcc4.8.0 | any    | amd64 | 4.8.0  |
| fededp/driverkit-builder-any-x86_64_gcc10.0.0_gcc9.0.0                           | any    | amd64 | 10.0.0 |
| fededp/driverkit-builder-any-x86_64_gcc10.0.0_gcc9.0.0                           | any    | amd64 | 9.0.0  |
| fededp/driverkit-builder-any-x86_64_gcc12.0.0_gcc11.0.0                          | any    | amd64 | 11.0.0 |

And using --gccversion:

./_output/bin/driverkit images --kernelrelease 6.0.12.arch1-1 --target arch --output-module /tmp/arch.ko  --output-probe /tmp/arch.o --dockerrepo docker.io/fededp/driverkit --gccversion 12.0.0
INFO listing images                                processor=images
|                          IMAGE                          | TARGET | ARCH  |  GCC   |
|---------------------------------------------------------|--------|-------|--------|
| fededp/driverkit-builder-any-x86_64_gcc12.0.0_gcc11.0.0 | any    | amd64 | 12.0.0 |

FedeDP · 2023-02-06T08:37:58Z

IMHO the PR is ready; removed the wip; adding the hold label like for a week or so, to wait to gather some more feedback :)
/hold

dwindsor · 2023-02-06T14:37:39Z

Thank you for the incredible amount of work that's gone into this. 🙏

How do you feel about changing the name of the new driverkit command-line arg from --dockerrepo to something like --builderrepo, so as to indicate to the user that said repo is for hosting the image in --builderimage?

/lgtm

FedeDP · 2023-02-06T14:39:15Z

Uh it is a neat idea! Thank you!

FedeDP · 2023-02-06T15:01:14Z

For a future CLI api break...i feel like builder.repo builder.image would be a much better naming (ie: having a new builder. config group)?

The same could apply to repo-org and repo-name too!

Like:

A command line tool to build Falco kernel modules and eBPF probes.

Usage:
driverkit
driverkit [command]

Available Commands:
completion            Generates completion scripts.
docker                Build Falco kernel modules and eBPF probes against a docker daemon.
help                  Help about any command
images                List builder images
kubernetes            Build Falco kernel modules and eBPF probes against a Kubernetes cluster.
kubernetes-in-cluster Build Falco kernel modules and eBPF probes against a Kubernetes cluster inside a Kubernetes cluster.

Flags:
--architecture string       target architecture for the built driver, one of [amd64,arm64] (default "amd64")
-c, --config string             config file path (default $HOME/.driverkit.yaml if exists)
--driverversion string      driver version as a git commit hash or as a git tag (default "master")
--dryrun                    do not actually perform the action
--gccversion string         enforce a specific gcc version for the build
-h, --help                      help for driverkit
--kernelconfigdata string   base64 encoded kernel config data: in some systems it can be found under the /boot directory, in other it is gzip compressed under /proc
--kernelrelease string      kernel release to build the module for, it can be found by executing 'uname -v'
--kernelurls strings        list of kernel header urls (e.g. --kernelurls <URL1> --kernelurls <URL2> --kernelurls "<URL3>,<URL4>")
--kernelversion string      kernel version to build the module for, it's the numeric value after the hash when you execute 'uname -v' (default "1")
-l, --loglevel string           log level (default "info")
--moduledevicename string   kernel module device name (the default is falco, so the device will be under /dev/falco*) (default "falco")
--moduledrivername string   kernel module driver name, i.e. the name you see when you check installed modules via lsmod (default "falco")
--name string               repository github name (default "libs")
--org string                repository github organization (default "falcosecurity")
--output-module string      filepath where to save the resulting kernel module
--output-probe string       filepath where to save the resulting eBPF probe
--proxy string              the proxy to use to download data
-t, --target string             the system to target the build for, one of [almalinux,amazonlinux,amazonlinux2,amazonlinux2022,arch,bottlerocket,centos,debian,fedora,flatcar,minikube,opensuse,photon,redhat,rocky,ubuntu,vanilla]
--timeout int               timeout in seconds (default 120)
-v, --version                   version for driverkit

Builder Flags:
--image string       docker image to be used to build the kernel module and eBPF probe. If not provided, an automatically selected image will be used.
--repo strings        list of docker repositories in descending priority order, used to search for builder images. Default falcosecurity/driverkit will always be enforced as lowest priority repo. eg: --dockerrepo myorg/driverkit --dockerrepo falcosecurity/driverkit

Repo Flags:
--name string          repository github name (default "libs")
--org string           repository github organization (default "falcosecurity")

And so on, even kernel{urls,version,release,configdata} could take advantage of this.
It would help the user better understand various driverkit option.

Cobra does not support this right now but a PR might get merged soonish: spf13/cobra#1778

FedeDP · 2023-02-06T15:14:10Z

Done @dwindsor :)

Signed-off-by: Federico Di Pierro <nierro92@gmail.com> Co-authored-by: David Windsor <dwindsor@secureworks.com>

FedeDP · 2023-02-06T15:16:32Z

Run make docs too! Forgot about it!

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

FedeDP · 2023-02-08T13:31:59Z

Improved documentation! 📄

FedeDP · 2023-02-16T13:34:13Z

/unhold

FedeDP · 2023-02-16T13:34:26Z

@dwindsor can you reapprove since i pushed some more docs? :)

dwindsor

Goodbye static list of builders =)

poiana · 2023-02-16T17:23:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dwindsor, FedeDP

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [FedeDP,dwindsor]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

poiana added kind/feature New feature or request do-not-merge/work-in-progress dco-signoff: yes area/build Further information is requested area/cmd area/pkg labels Dec 22, 2022

poiana requested review from dwindsor and leodido December 22, 2022 15:59

poiana added approved size/XL labels Dec 22, 2022

poiana added size/XXL and removed size/XL labels Jan 19, 2023

FedeDP force-pushed the new/dynamic_builders branch from d7463eb to 6cc4437 Compare January 19, 2023 16:16

FedeDP mentioned this pull request Jan 20, 2023

Cannot Build CentOS 7: 3.10.0-1160.80.1.el7.x86_64 #236

Closed

dwindsor previously approved these changes Jan 31, 2023

View reviewed changes

poiana assigned dwindsor Jan 31, 2023

poiana added the lgtm label Jan 31, 2023

FedeDP added 4 commits February 2, 2023 10:09

new(pkg): initial implementation of dynamic builder images loading an…

e34ae87

…d indexing given a docker repo. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

new(builder,makefile,pkg): ported images to new name template.

a2db8f4

Ported makefile too. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

new(pkg,cmd): allow to pass list of descending priority docker repos.

df83c3e

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

cleanup(pkg): small refactor.

fcab049

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

FedeDP added 5 commits February 3, 2023 10:51

fix(docker): fixed small typo in builder-any-x86_64_gcc12.0.0_gcc11.0…

7af5e61

….0.Dockerfile. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

fix(docker): correctly named gcc{8,6,5,4.8} image.

1c95a19

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

update(docs): add a note about image tags.

ca791b2

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

chore(makefile): add docker.io/fededp/driverkit as builder images r…

d185404

…epo to integration tests. This should allow us to test them. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

fix(pkg): properly made LoadImages() aware of fixed gccversion set …

67cca5d

…by user, if any. In this way, we are able to support `--gccversion` properly. Moreover, `images` cmd is now `--gccversion` aware. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

FedeDP force-pushed the new/dynamic_builders branch from bad00ce to 67cca5d Compare February 3, 2023 10:31

FedeDP changed the title ~~wip: new: dynamic builders~~ new: dynamic builders Feb 6, 2023

poiana removed the do-not-merge/work-in-progress label Feb 6, 2023

poiana added the do-not-merge/hold label Feb 6, 2023

chore: renamed dockerrepo to builderrepo.

4e0f11c

Signed-off-by: Federico Di Pierro <nierro92@gmail.com> Co-authored-by: David Windsor <dwindsor@secureworks.com>

FedeDP force-pushed the new/dynamic_builders branch from c7d76f2 to 4e0f11c Compare February 6, 2023 15:16

dwindsor previously approved these changes Feb 6, 2023

View reviewed changes

update(docs): multiple improvements to docs.

cb578b6

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

FedeDP dismissed dwindsor’s stale review via cb578b6 February 8, 2023 13:31

poiana removed the do-not-merge/hold label Feb 16, 2023

dwindsor approved these changes Feb 16, 2023

View reviewed changes

poiana merged commit a3b171a into master Feb 16, 2023

poiana deleted the new/dynamic_builders branch February 16, 2023 17:24

FedeDP mentioned this pull request Feb 17, 2023

cleanup: removed builderrepo option pointing to my dockerhub. #252

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new: dynamic builders #244

new: dynamic builders #244

FedeDP commented Dec 22, 2022 •

edited

FedeDP commented Dec 22, 2022 •

edited

FedeDP commented Dec 22, 2022 •

edited

FedeDP commented Dec 22, 2022

FedeDP commented Dec 22, 2022

FedeDP commented Jan 9, 2023 •

edited

FedeDP commented Jan 10, 2023

FedeDP commented Jan 13, 2023

FedeDP commented Jan 24, 2023 •

edited

dwindsor left a comment

FedeDP commented Jan 31, 2023

FedeDP commented Feb 3, 2023 •

edited

FedeDP commented Feb 6, 2023

dwindsor commented Feb 6, 2023 •

edited

FedeDP commented Feb 6, 2023

FedeDP commented Feb 6, 2023 •

edited

FedeDP commented Feb 6, 2023

FedeDP commented Feb 6, 2023

FedeDP commented Feb 8, 2023

FedeDP commented Feb 16, 2023

FedeDP commented Feb 16, 2023

dwindsor left a comment

poiana commented Feb 16, 2023

new: dynamic builders #244

new: dynamic builders #244

Conversation

FedeDP commented Dec 22, 2022 • edited

FedeDP commented Dec 22, 2022 • edited

FedeDP commented Dec 22, 2022 • edited

FedeDP commented Dec 22, 2022

FedeDP commented Dec 22, 2022

FedeDP commented Jan 9, 2023 • edited

FedeDP commented Jan 10, 2023

FedeDP commented Jan 13, 2023

FedeDP commented Jan 24, 2023 • edited

dwindsor left a comment

Choose a reason for hiding this comment

FedeDP commented Jan 31, 2023

FedeDP commented Feb 3, 2023 • edited

FedeDP commented Feb 6, 2023

dwindsor commented Feb 6, 2023 • edited

FedeDP commented Feb 6, 2023

FedeDP commented Feb 6, 2023 • edited

FedeDP commented Feb 6, 2023

FedeDP commented Feb 6, 2023

FedeDP commented Feb 8, 2023

FedeDP commented Feb 16, 2023

FedeDP commented Feb 16, 2023

dwindsor left a comment

Choose a reason for hiding this comment

poiana commented Feb 16, 2023

FedeDP commented Dec 22, 2022 •

edited

FedeDP commented Dec 22, 2022 •

edited

FedeDP commented Dec 22, 2022 •

edited

FedeDP commented Jan 9, 2023 •

edited

FedeDP commented Jan 24, 2023 •

edited

FedeDP commented Feb 3, 2023 •

edited

dwindsor commented Feb 6, 2023 •

edited

FedeDP commented Feb 6, 2023 •

edited