-
Notifications
You must be signed in to change notification settings - Fork 13
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(ci): run update-kernels daily. #179
Conversation
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
/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.
This would be an amazing improvement! Thanks for getting this PR up @FedeDP!
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.
Ok for me
Also, it would be great if we removed the This can be done in a follow up PR of course. |
Ohhh I now realize how this is done. Hmm I understand we can't change it over night, but perhaps we can just invoke the kernel crawler cron job from test-infra and the kernelrelease build yamls will only exists within the CI job, no PRs or approvals at all. Any other solution that accomplishes full automation will work as well. |
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.
/approve
(let's coordinate on this for unlocking it :) )
Please remember that our dbg is not the only consumer of the kernel crawler output jsons. The only real workaround would be to let |
@FedeDP thanks for the additional context! Would it be possible to have 2 streams? The existing workflow for all remaining use cases. A fast runner so to say that constantly crawls and builds kernel drivers. Hard requirement would be full automation, no PRs. Something like this fast runner is what I have for custom kernels and it has never failed me. |
Full automation would be a bit hard to achieve, we would need to completely trust the process; ie: it would mean that test-infra update-dbg job should directly push to master, i don't really like this.
I think if we move the current kernel-crawler site onto download.falco.org just like we do for drivers (and it makes sense because it gets nearer to them) and the big jsons too, we can run it daily directly from test-infra that will:
WDYT? |
I'd propose an alternative option for the publishing of the kernel list. In order to:
we could generate the list on the fly in the GH actions pipeline, and publish it directly to the GH page. This way, we'd achieve the following:
The drawback I see is that:
But we could overcome this con by running it as a Prow cronjob. It would be less efficient, but dbg-go is smart enough to skip Driverkit configurations that have not changed (thus skipping unneeded PRs). WDYT? |
I like this idea @maxgio92 ! Much cleaner, even if we would need to move |
+1 @maxgio92 thanks for the more concrete outline and yes I also don't see a problem with the planned cron jobs in this regard in test-infra. |
@@ -3,35 +3,8 @@ name: Update Kernels | |||
|
|||
on: | |||
workflow_dispatch: | |||
inputs: |
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 don't support single input anymore:
- since dbg-go is super quick to generate dbg configs (a matter of seconds)
- since we will run the crawler daily
we have no more needs for it.
contents: write | ||
pull-requests: write | ||
contents: read | ||
pages: write |
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 will directly deploy pages without pushing anything to any branch.
pull-requests: write | ||
contents: read | ||
pages: write | ||
id-token: write | ||
steps: | ||
- name: Checkout crawler | ||
uses: actions/checkout@v3 |
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 checkout default branch that now has the index.html file.
- name: Update json lists | ||
kernel-crawler crawl --distro="*" --arch=aarch64 > $RUNNER_TEMP/aarch64/list.json | ||
|
||
- name: Move everything under site folder |
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 use site
folder as root for the pages deployment.
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 file was completely copy/pasted by the kernels branch.
document.getElementById('archs').appendChild(element); | ||
}); | ||
|
||
$.getJSON(arch+'/list.json', function(data) { |
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 now points to a local (relative) file.
// "data" : data, | ||
"order": [[ 1, "desc" ]], | ||
ajax: { | ||
url: arch+'/list.json', |
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.
Instead, directly deploy pages using `update-kernels` github action. Signed-off-by: Federico Di Pierro <nierro92@gmail.com> Co-authored-by: Massimiliano Giovagnoli <me@maxgio.it>
f6f4258
to
aa02b15
Compare
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
/cc |
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.
/approve
🚀
/unhold |
Let's test this! |
What type of PR is this?
/kind feature
Any specific area of the project related to this PR?
/area ci
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Refs falcosecurity/test-infra#1255
Special notes for your reviewer: