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

chore: Workflow to build all Arch #1218

Merged
merged 17 commits into from
Jun 14, 2021
Merged

Conversation

mickkael
Copy link
Contributor

  • Changed the Dockerfile to be compatible with different architecture (default remains amd64 if no arg provided)
  • Changed "release" workflow for GH Actions
    • build the binaries for all architectures
    • publish the argo-events binaries as part of the release
    • push the docker image for all architectures
    • push a multi-manifest to have a multi-architecture tag
  • Changed the makefile to reflect all the previous changes

Checklist:

@mickkael mickkael force-pushed the release-gh-workflow branch from 67bc4e2 to 5170f39 Compare May 17, 2021 16:38
@mickkael mickkael force-pushed the release-gh-workflow branch 3 times, most recently from 2544e8d to 5170f39 Compare May 18, 2021 09:37
@mickkael mickkael force-pushed the release-gh-workflow branch from 3e6f7e9 to 550ecad Compare May 18, 2021 09:40
@mickkael
Copy link
Contributor Author

I've updated the makefile for the test workflow to pass following the modification.
We should be all good now

@whynowy whynowy changed the title Workflow to build all Arch chore: Workflow to build all Arch May 18, 2021
@mickkael
Copy link
Contributor Author

Hi @whynowy , any comment, anything to improve?
Thanks for the review!

.github/workflows/release.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@mickkael mickkael force-pushed the release-gh-workflow branch 3 times, most recently from a2e5285 to 17296e7 Compare May 26, 2021 14:03
Signed-off-by: mickkael <[email protected]>
@mickkael mickkael force-pushed the release-gh-workflow branch from 17296e7 to a7ca361 Compare May 26, 2021 14:21
Makefile Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
Copy link
Contributor Author

@mickkael mickkael left a comment

Choose a reason for hiding this comment

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

still WIP

.github/workflows/release.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@mickkael mickkael force-pushed the release-gh-workflow branch 2 times, most recently from a2c79d7 to 628d0e0 Compare May 27, 2021 01:54
@mickkael mickkael force-pushed the release-gh-workflow branch from 628d0e0 to 5a2d79b Compare May 27, 2021 02:16
@mickkael
Copy link
Contributor Author

mickkael commented May 29, 2021

@whynowy , ready for a "hopefully" final review.

  • the workflow builds using makefile and not github action
  • the build and push of the container arm64/amd64 multi image takes 42s, so the login timeout is very unlikely
  • the binary build takes 10 min and do amd64, arm64, arm, ppc64le, s390x . If needed some arch could be removed to speed it up.
  • the binaries are added to the release, gzipped, and with a checksum file for each
  • the code could be easily modified to have a multi registry push

I'm now fluent with github actions and makefile !

Makefile Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
Signed-off-by: mickkael <[email protected]>
Copy link
Contributor Author

@mickkael mickkael left a comment

Choose a reason for hiding this comment

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

clean up done, and comments replied

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@mickkael mickkael force-pushed the release-gh-workflow branch from 53fa99a to a107c22 Compare June 9, 2021 02:59
@mickkael
Copy link
Contributor Author

mickkael commented Jun 9, 2021

I have added a set-buildx instruction in the makefile. I guess it's useful to have buildx setup automatically rather than being a prereq.

Copy link
Member

@whynowy whynowy left a comment

Choose a reason for hiding this comment

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

Conditional approve, please remove the set-buildx part.

Makefile Outdated Show resolved Hide resolved
@whynowy whynowy merged commit 74dd3ad into argoproj:master Jun 14, 2021
juliev0 pushed a commit to juliev0/argo-events that referenced this pull request Mar 29, 2022
* Dockerfile compatible with all Architecture builds

Signed-off-by: mickkael <[email protected]>

* release workflow multi-arch

Signed-off-by: mickkael <[email protected]>

* fix image command in makefile

Signed-off-by: mickkael <[email protected]>

* Simplify code

Signed-off-by: mickkael <[email protected]>

* with DOCKER_PUSH to true
Signed-off-by: mickkael <[email protected]>

* fix release wf for non argoproj namespace build
Signed-off-by: mickkael <[email protected]>

* clean up
Signed-off-by: mickkael <[email protected]>

* rem DO_BUILD add set-buildx

Signed-off-by: mickkael <[email protected]>

* improve buildx to not reinstall and check exec

Signed-off-by: mickkael <[email protected]>

* fix bug if gzipped binaries exists already

Signed-off-by: mickkael <[email protected]>

* small bug

Signed-off-by: mickkael <[email protected]>

* improve reliability

Signed-off-by: mickkael <[email protected]>

* test compatible with sh instead of bash

Signed-off-by: mickkael <[email protected]>

* remove set-buildx

Signed-off-by: mickkael <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants