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

new(ci): gha master and release workflows #2501

Merged
merged 22 commits into from Apr 27, 2023
Merged

Conversation

FedeDP
Copy link
Contributor

@FedeDP FedeDP commented Apr 18, 2023

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:

  • fix arm64 build
  • fix arm64 job build Falco version (it sees 0.0.0 somehow...)
  • add GPG_KEY as gha variable (needs repo admin)
  • fix master and release yaml issues
  • understand how to docker in docker (reusable_build_docker workflow) -> just go on host. Tested that docker and awscli can be used in those jobs
  • Fix packages publishing (see new(config): added iam s3 access roles for Falco (dev and release). test-infra#1102)
  • Make reusable_publish_docker the only docker publishing job (ie: build_docker today is publishing images too, while publish_docker is using the docker manifest API to merge architectures specific image into a multi arch tag)
  • What about ecr docker publishing?
  • test master :)

Other notes:

Also, as you can see, reusable_build_packages is not using the falco modern builder image; instead it is directly building in a centos: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.

ERROR: invalid tag "falcosecurity/falco-no-driver:aarch64-2501/merge": invalid reference format

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?:

new(ci): ported master and release artifacts publishing CI to gha

…es + docker images publish).

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
Renamed `dev.yaml` to `master.yaml`.

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>
requires:
- "build-docker-dev"
- "build-docker-dev-arm64"
# - "rpm-sign":
Copy link
Contributor Author

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:
Copy link
Contributor Author

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.

concurrency:
group: ${{ github.head_ref || github.run_id }}
cancel-in-progress: true

jobs:
test-arm:
Copy link
Contributor Author

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.

Copy link
Contributor Author

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:
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

.github/workflows/master.yaml Outdated Show resolved Hide resolved
@FedeDP
Copy link
Contributor Author

FedeDP commented Apr 18, 2023

Remaining steps have been added to PR body.

@FedeDP
Copy link
Contributor Author

FedeDP commented Apr 18, 2023

/milestone 0.35.0

@poiana poiana added this to the 0.35.0 milestone Apr 18, 2023
@FedeDP FedeDP force-pushed the new/gha_dev_release_workflows branch from 2f4110e to b882b9d Compare April 18, 2023 08:48
version: ${{ needs.build-dev-packages.outputs.version }}
secrets: inherit

build-dev-docker:
Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

@FedeDP
Copy link
Contributor Author

FedeDP commented Apr 18, 2023

Also, we definitely need more powerful gha self-hosted runners for arm (and more replicas).

@FedeDP FedeDP force-pushed the new/gha_dev_release_workflows branch from 2cab6b7 to 3cdc306 Compare April 18, 2023 10:21
@FedeDP
Copy link
Contributor Author

FedeDP commented Apr 18, 2023

Uh this time build-arm64 is much faster. Don't know what happened :/

@FedeDP FedeDP force-pushed the new/gha_dev_release_workflows branch 2 times, most recently from 83bb5f0 to ae7256c Compare April 18, 2023 10:55
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

Co-authored-by: Luca Guerra <luca@guerra.sh>
@LucaGuerra
Copy link
Contributor

LGTM, I think we can test it on master :)

@FedeDP
Copy link
Contributor Author

FedeDP commented Apr 27, 2023

/hold

Copy link
Member

@leogr leogr left a 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).

Copy link
Member

@Andreagit97 Andreagit97 left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link

poiana commented Apr 27, 2023

[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:
  • OWNERS [Andreagit97,FedeDP,leogr]

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

@FedeDP
Copy link
Contributor Author

FedeDP commented Apr 27, 2023

/unhold

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

CI issue: data race in publishing jobs for -dev packages
5 participants