- Notifications
You must be signed in to change notification settings - Fork190
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
update the makefile to push multi-arch images#319
base:main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@## main #319 +/- ##==========================================+ Coverage 62.73% 62.96% +0.22%========================================== Files 58 58 Lines 4286 4760 +474 ==========================================+ Hits 2689 2997 +308- Misses 1382 1546 +164- Partials 215 217 +2
Continue to review full report at Codecov.
|
seems to be working
|
will test tomorrow to ensure they actually run on the platforms |
ok got it running in stage and it seems fine, both sso_proxy and sso_auth |
| RUN ln -s /usr/bin/dpkg-split /usr/sbin/dpkg-split | ||
| RUN ln -s /usr/bin/dpkg-deb /usr/sbin/dpkg-deb | ||
| RUN ln -s /bin/tar /usr/sbin/tar | ||
| RUN ln -s /bin/rm /usr/sbin/rm |
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 was needed for the builds to complete. seems like the file locations are in different spots on the ARM arch.
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.
👍 Would it be worth adding this in a quick inline comment?
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.
if this remains, yes. i want to test the resultant multi-arch images in a arm64 cluster before finalizing this. i only got to test it once during hackweek before tearing down the temp cluster.
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.
i'm going to see if we need this for arm64, which is the primary secondary target we want for M1's and Graviton's
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.
that didn't work, these are needed for arm64 also apparently.
Makefile Outdated
| docker buildx build --tag buzzfeed/sso:$(version) . --platform linux/amd64,linux/arm64,linux/arm/v7 --push | ||
| docker buildx build --tag buzzfeed/sso-dev:latest . --platform linux/amd64,linux/arm64,linux/arm/v7 --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.
I think we might also want to push abuzzfeed/sso:latest tag. I'm also not particularly sure we really need thebuzzfeed/sso-dev:latest tag to be honest. That tag should be created with each push to themain branch (throughthis logic), so I'm not sure we gain anything with it being here as well.
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.
thanks, this was a typo, should have beenbuzzfeed/sso:latest. fixing it.
53e6bb9 todb2fc03Compare171ce40 todb2fc03Compare
Problem
current sso images don't support arm64 architecture. that means we can't run them on AWS Graviton's or Apple M1's
Solution
build a multi-arch with
linux/amd64,linux/arm64platform supportNotes
based onhttps://www.docker.com/blog/multi-arch-images/ andhttps://www.docker.com/blog/multi-arch-build-what-about-circleci/
our upstream,
golang:1.14supports these architectures and more