-
Notifications
You must be signed in to change notification settings - Fork 912
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): gha master and release workflows #2501
Conversation
…es + docker images publish). Signed-off-by: Federico Di Pierro <[email protected]>
Renamed `dev.yaml` to `master.yaml`. Signed-off-by: Federico Di Pierro <[email protected]>
Signed-off-by: Federico Di Pierro <[email protected]>
Signed-off-by: Federico Di Pierro <[email protected]>
requires: | ||
- "build-docker-dev" | ||
- "build-docker-dev-arm64" | ||
# - "rpm-sign": |
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.
Avoid conflicts; disable circleCI jobs.
@@ -2,17 +2,71 @@ name: CI Build | |||
on: | |||
pull_request: | |||
branches: [master] | |||
push: |
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 now have a dedicated master.yaml
for that.
.github/workflows/ci.yml
Outdated
concurrency: | ||
group: ${{ github.head_ref || github.run_id }} | ||
cancel-in-progress: true | ||
|
||
jobs: | ||
test-arm: |
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 is just to test.
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.
Will be removed before removing wip
.
branches: [master] | ||
|
||
# Checks if any concurrent jobs under the same pull request or branch are being executed | ||
concurrency: |
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 fixes #1876
~/sign $RUNNER_TEMP/falco-*.rpm | ||
rpm --qf %{SIGPGP:pgpsig} -qp $RUNNER_TEMP/falco-*.rpm | grep SHA256 | ||
|
||
- name: Publish rpm |
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.
To publish, we also need S3 credentials and access.
Remaining steps have been added to PR body. |
/milestone 0.35.0 |
2f4110e
to
b882b9d
Compare
version: ${{ needs.build-dev-packages.outputs.version }} | ||
secrets: inherit | ||
|
||
build-dev-docker: |
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.
Both build-dev-docker
and its arm64 counterpart require build-dev-packages
because they use its output.
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 goes for the release CI.
Also, we definitely need more powerful gha self-hosted runners for arm (and more replicas). |
2cab6b7
to
3cdc306
Compare
Uh this time build-arm64 is much faster. Don't know what happened :/ |
83bb5f0
to
ae7256c
Compare
Signed-off-by: Federico Di Pierro <[email protected]> Co-authored-by: Luca Guerra <[email protected]>
LGTM, I think we can test it on master :) |
/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.
Couldn't try it in detail, but this is 🤩
I think we should give it a try, then fix any issues that will arise (as usual with the CI).
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Andreagit97, FedeDP, leogr 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 |
/unhold |
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:
This PR tries to enable master and release build + packages + docker images CI on gha instead of circleci.
It surely needs some tests hence the
wip
.To do so, it uses
reusable
workflows that are shared between master and release; therefore testing CI on master should be enough to test also the release one, since they are exactly the same.Moreover, people could also reuse our provided workflows in their own CI.
Which issue(s) this PR fixes:
Fixes #1876
Special notes for your reviewer:
Steps:
docker
andawscli
can be used in those jobsreusable_publish_docker
the only docker publishing job (ie:build_docker
today is publishing images too, whilepublish_docker
is using the docker manifest API to merge architectures specific image into a multi arch tag)Other notes:
reusable_build_packages
job was tested with a fake job in the PR ci on the arm64 node: https://github.com/falcosecurity/falco/actions/runs/4734095566The operation was canceled, but it worked just fine.
See the commit with the code: 75759c8
Also, as you can see,
reusable_build_packages
is not using the falco modern builder image; instead it is directly building in acentos:7
container (that is the base image for the falco modern builder). This is needed because we cannot run docker in docker on our prow nodes (ie: the arm64 ones).Instead, the skeleton building phase is made on a fedora:latest image, because it already ships
bpftool
in its repo with a recent version.reusable_build_docker
workflow has been tested through a fake job in the PR ci on both arm and x86: https://github.com/falcosecurity/falco/actions/runs/4743816658It was failing because
{{ github.ref_name }}
variable was notmaster
nor a tag name, instead it was2501/merge
:But you see it was failing with the same error at the same stage on both x86 and arm; therefore i expect it to work just fine on master.
See the commit with the code: dc37254
Does this PR introduce a user-facing change?: