-
Notifications
You must be signed in to change notification settings - Fork 108
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
new: support driverkit multiarch driver builds #734
Conversation
Work is still in progress. Namely, backward compatibility must still be addressed, as new config files expect "/$arch/configs" paths, while the old had only x86_64 in the root. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
…rt to driverkit scrape_and_generate/generate utilities. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
…, utils/scrape_and_generate to be backward compatible. Moreover, fixed a couple of stupid bugs. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
Note: even when filtering for TARGET_ARCH, `validate_old` and `specific_target_old` will always run. This is a small issue while we support both the old and the new format, and will eventually go away once we drop support for old format. It is not impactful since we do never filter for TARGET_ARCH (given that it is a newly introduced filter). Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
…upport the new $driverversion/$arch/ format. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
# Recursive wildcard | ||
rwildcard=$(foreach d,$(wildcard $(1:=/*)),$(call rwildcard,$d,$2) $(filter $(subst *,%,$2),$d)) | ||
|
||
CONFIGS := $(call rwildcard,config/*,*.yaml) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Find all configs under config and its subfolders (eventually, for supported $arch).
S3_DRIVERS_BUCKET ?= "falco-distribution" | ||
S3_DRIVERS_KEY_PREFIX ?= "driver" | ||
SKIP_EXISTING ?= true | ||
|
||
validate: $(patsubst config_%,validate/%,$(subst /,_,$(wildcard config/${TARGET_VERSION}*/${TARGET_DISTRO}_${TARGET_KERNEL}-*))) | ||
validate: validate_old validate_new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split in 2 sub targets to correctly manage new and old driverkit config tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it, even not elegant. When needed to cleanup it will be simple.
|
||
all: $(patsubst config_%,%,$(subst /,_,$(CONFIGS))) | ||
|
||
specific_target: $(patsubst config_%,%,$(subst /,_,$(wildcard config/${TARGET_VERSION}*/${TARGET_DISTRO}_${TARGET_KERNEL}-*))) | ||
specific_target: specific_target_old specific_target_new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split in 2 sub targets to correctly manage new and old driverkit config tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
driverkit/Makefile
Outdated
|
||
prepare: $(addprefix prepare_,$(VERSIONS)) | ||
publish: $(addprefix publish_,$(VERSIONS)) | ||
publish_s3: $(addprefix publish_s3_,$(VERSIONS)) | ||
|
||
generate: | ||
$(foreach VERSION,$(VERSIONS),\ | ||
utils/generate -k ${TARGET_KERNEL} -d ${TARGET_DISTRO} -v ${VERSION}; \ | ||
utils/generate -a $(TARGET_ARCH) -k ${TARGET_KERNEL} -d ${TARGET_DISTRO} -h ${TARGET_HEADERS} -v ${VERSION}; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generate target will now generate configs with proper architecture
and kernelurls
set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See FedeDP#1 and review
@@ -40,36 +60,53 @@ cleanup_s3: | |||
S3_DRIVERS_BUCKET=${S3_DRIVERS_BUCKET} S3_DRIVERS_KEY_PREFIX=${S3_DRIVERS_KEY_PREFIX} utils/cleanup_s3 $(addprefix -v ,$(VERSIONS)) | |||
|
|||
# $(1): pseudo-target name | |||
# $(2): driver version | |||
# $(3): config file path | |||
# $(2): config file path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropped unused driver version param.
ifneq ("$(wildcard output/$(1)/*.ko)","") | ||
@mkdir -p output/$(1)/kernel-module | ||
@mv -f output/$(1)/*.ko output/$(1)/kernel-module | ||
endif | ||
ifneq ("$(wildcard output/$(1)/x86_64/*.ko)","") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, we now search in both root folder (old directory tree) and new $arch subfolders.
@@ -9,6 +9,15 @@ input="$1" | |||
IFS=/ read -a input_parts <<< ${input} | |||
driver_version="${input_parts[1]}" | |||
|
|||
# If there is an arch subdir -> use it! | |||
# TODO: rm once old format support is dropped; just use arch="${input_parts[2]}" | |||
if [[ ${#input_parts[@]} -eq 4 ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This workaround is needed to be able to check correct module path for old and new directory tree.
echo -n "${arch}" | ||
} | ||
|
||
arch="$(get_arch_from_path ${input})" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fetch arch from config file path (if any); it is later used to check that if config file is under a $arch subfolder, its output module and probe must be under same $arch subfolder.
configs=(config/${TARGET_VERSION}*/${TARGET_DISTRO}_${TARGET_KERNEL}-*.yaml) | ||
# Only load old configs if x86_64 or * arch is requested | ||
# TODO: rm this once old format support is dropped | ||
if [ "${TARGET_ARCH}" = "*" ] || [ "${TARGET_ARCH}" = "x86_64" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If users filter for TARGET_ARCH, only load old config if either "*" (any) or "x86_64" is requested.
driverkit/utils/generate
Outdated
# in the TARGET_VERSION folder. | ||
# TODO: rm this once old format support is dropped, and always use ${TARGET_VERSION}/${TARGET_ARCH} | ||
function path_from_version() { | ||
rx='^([0-9]+\.){0,2}(\*|[0-9]+)$' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We assume that new driverversions, that will use new directory tree, all have a semver.
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
Hey @FedeDP Could you describe the expected directory (and URL) layout in the PR description for a future reference? 🙏 |
@leogr i will edit the OP! |
Signed-off-by: Massimiliano Giovagnoli <me@maxgio.it>
00ebdaf
to
e87df75
Compare
/lgtm |
LGTM label has been added. Git tree hash: ee2bd9af134e184408ee12c06d8e2f0e15d134b3
|
I think we'll then need a refactoring of the DBG considering arbitrary architecture, maybe after the removal of deprecated (configuration and distribution) paths. |
Yep; i think the work is in place indeed, we just need some improvements here and there (that will be much easier once we drop support for old tree paths)! |
/hold |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's give it a try :)
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: FedeDP, leogr, maxgio92 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:
Approvers can indicate their approval by writing |
This PR adds support for driverkit configs $arch subfolders for newer driverversions, that we assume will be semversioned.
On top of that, it also adds support for driverkit 0.9.1 features like
kernelurls
and multiarch builds.Biggest challenge is keeping backward compatibility with current driverkit directory tree.
Old directory tree:
config/$driverversion/*.yml
and expected url:
https://download.falco.org/?prefix=driver/$driverversion/*.{o,ko}
New one:
config/$driverversion(semver)/$arch/*.yml
and expected url:
https://download.falco.org/?prefix=driver/$driverversion/$arch/*.{o,ko}