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: build docker images for arm arch #3309

Closed
wants to merge 2 commits into from

Conversation

DaleWebb
Copy link

@DaleWebb DaleWebb commented May 1, 2023

What change does this PR introduce?

Builds the docker images for arm64 and amd64 architectures using buildkit emulation

Why was this change needed?

Closes #1316

Other information (Screenshots)

  • Had to remove the base image: nikolaik/python-nodejs:python3.10-nodejs16-alpine as this couldn't be compiled for arm.
  • Build times might increase significantly...

Copy link
Contributor

@p-fernandez p-fernandez left a comment

Choose a reason for hiding this comment

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

LGTM.
@Cliftonz @scopsy thoughts?

@scopsy scopsy requested a review from Cliftonz May 7, 2023 15:01
@scopsy
Copy link
Contributor

scopsy commented May 7, 2023

@Cliftonz should we utilize depot in someway for this? Also, how this should affect the way we push to GHCR? Or no further action is needed from our side there

@underfisk
Copy link
Contributor

@scopsy Any thoughts on this one? I've been loving the integration of Novu but for m1 mac's its being a blocker as sometimes the API crashes and needs manual reloading

@scopsy
Copy link
Contributor

scopsy commented May 24, 2023

Would love to have @Cliftonz take a look at this to make sure we are not making the docker files larger and it's backwards compatible 🙏

@Cliftonz
Copy link
Contributor

Taking a look at this now

@Cliftonz
Copy link
Contributor

Cliftonz commented May 26, 2023

@DaleWebb Sorry this took so long, I need to look a little deeper at some of our internal tooling to see if this can be accomplished with the process we already have.

@scopsy Should be able to give me access sunday and I will be able to dig deeper monday.

@Cliftonz
Copy link
Contributor

@DaleWebb @scopsy Following up on this Depot is not currently building ARM containers otherwise it will block all of our builds. I have reached out to support to get this solved hopefully as soon as possible.

Otherwise, the local docker build does not seem like any issue other then needing to run docker buildx create --use before it would actually build multiple architectures.

@Cliftonz
Copy link
Contributor

The issue was with depot waiting for context to be passed in on std in and not timing out.
This is getting with with depot right now.

@jainpawan21
Copy link
Member

@Cliftonz Can we close/merge this PR?

@Cliftonz
Copy link
Contributor

@Cliftonz Can we close/merge this PR?

@jainpawan21 Not yet, I will look over this monday

@Cliftonz
Copy link
Contributor

@DaleWebb @underfisk I am going to have to delay working on this for the moment.
I have resiliency issues I need to attend to at this time.
My apologies

@underfisk
Copy link
Contributor

Ah no worries, lately I've developed a tool with Playwright that automates the setup which was a thing that wasn't possible with Novu because the SDK always demands an API key, there are no "root/master" credentials which I hope there were.
My workaround now is a health check that does wget http:/.../v1/health-check to the novu-api because that often gets the following error:

assertion failed [result.value != EEXIST]: VmTracker attempted to allocate existing mapping

Let me know once there are some news regarding ARM support as it's starting to become somewhat important for our codebase

@Cliftonz
Copy link
Contributor

Thanks for letting us know. I will see what I can do shortly.

@github-actions github-actions bot added the stale Pull Request that needs to be reviewed label Jul 12, 2023
@paulwer
Copy link
Contributor

paulwer commented Aug 1, 2023

are there any updates regarding arm64 support? ❤️

@github-actions github-actions bot removed the stale Pull Request that needs to be reviewed label Aug 1, 2023
@github-actions github-actions bot added the stale Pull Request that needs to be reviewed label Aug 15, 2023
@jainpawan21
Copy link
Member

Thanks for letting us know. I will see what I can do shortly.

Hi @Cliftonz
What do you think? can we prioritize this somehow?

@github-actions github-actions bot removed the stale Pull Request that needs to be reviewed label Sep 7, 2023
@Cliftonz
Copy link
Contributor

Cliftonz commented Sep 7, 2023

Not at this current time, I really hate to do this but we have much higher priorities from the business and reliability side of things. This is definitely a great to have though we do not have the time currently to implement this in our pipeline.

@github-actions github-actions bot added the stale Pull Request that needs to be reviewed label Sep 21, 2023
@AliaksandrRyzhou
Copy link
Contributor

Closed by PR 5139

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

Successfully merging this pull request may close these issues.

🚀 Feature: Arm64 Support
8 participants