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

feat: add docker build for amd64 / arm64, e2e tests, publishing ghcr #503

Merged
merged 23 commits into from
Dec 19, 2024

Conversation

dutterbutter
Copy link
Collaborator

@dutterbutter dutterbutter commented Dec 19, 2024

What πŸ’»

  • Adds docker build / publish for amd64 / arm64. Example of successful build using fork
  • Adds new workflow to run e2e tests against docker image
  • Publishes to ghcr.io

Why βœ‹

  • Wider support
  • Ensures docker image works as intended

Closes #479, Closes #480, Closes #481

Evidence πŸ“·

Include screenshots, screen recordings, or console output here demonstrating that your changes work as intended

Comment on lines 67 to 68
us-docker.pkg.dev/matterlabs-infra/matterlabs-docker/era-test-node:${{ steps.docker_tag.outputs.tag }}
us-docker.pkg.dev/matterlabs-infra/matterlabs-docker/era-test-node:latest
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@popzxc how can we update this to be anvil-zksync instead of era-test-node?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would just try changing the name first and see if that will work. The way I am parsing this string is matterlabs-infra is the name of project, matterlabs-docker is the name of the container registry and era-test-node is the image in the registry. AFAIR you should be able to add new images to an existing registry by default unless we have disabled that on purpose.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can temporarily enable on.push.branches.<your-branch-name> to make this trigger before we merge the PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dutterbutter dutterbutter marked this pull request as ready for review December 19, 2024 01:55
@dutterbutter dutterbutter requested a review from a team as a code owner December 19, 2024 01:55
@dutterbutter dutterbutter added the needs review πŸ‘“ PR requires a review label Dec 19, 2024
@dutterbutter dutterbutter added the do not merge πŸ›‘ Do not merge this PR until label is removed label Dec 19, 2024
@dutterbutter dutterbutter removed the do not merge πŸ›‘ Do not merge this PR until label is removed label Dec 19, 2024
itegulov
itegulov previously approved these changes Dec 19, 2024
Copy link
Contributor

@itegulov itegulov left a comment

Choose a reason for hiding this comment

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

LGTM!

@dutterbutter
Copy link
Collaborator Author

@itegulov I want to test this again on a fork before merging.

@antonbaliasnikov has pointed out that with using the same tag it might just overwrite the last job that completes.

@dutterbutter dutterbutter added do not merge πŸ›‘ Do not merge this PR until label is removed and removed needs review πŸ‘“ PR requires a review labels Dec 19, 2024
@dutterbutter dutterbutter removed the do not merge πŸ›‘ Do not merge this PR until label is removed label Dec 19, 2024
@dutterbutter dutterbutter merged commit 133121d into main Dec 19, 2024
14 checks passed
@dutterbutter dutterbutter deleted the db/multi-platform-docker branch December 19, 2024 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants