-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
parameterize jdk and nginx base images #11262
Conversation
5913dcd
to
307765c
Compare
@@ -34,6 +34,8 @@ export DOCKER_BUILD_PLATFORM=linux/arm64 | |||
export DOCKER_BUILD_ARCH=arm64 | |||
export ALPINE_IMAGE=arm64v8/alpine:3.14 | |||
export POSTGRES_IMAGE=arm64v8/postgres:13-alpine | |||
export NGINX_IMAGE=arm64v8/nginx:1.19-alpine |
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.
FYI, the way this is specified will change with #11450
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.
awesome! that change looks great
btw, @jrhizor have you ever actually run into the issue I mentioned in this PR's description?
Basically if you build for amd64, you end up with the amd64 versions of the openjdk
and nginx
images in your local docker cache, so if you try to build for arm64 next, you get build errors unless you untag those base images.
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 haven't actually run into that for some reason. I already have the arm64 version of nginx:1.19-alpine
cached locally, but I'm not sure why.
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.
Cool, thanks for the feedback. I actually haven't found anyone else that has run into this issue yet which is partially why I left this as a draft PR, but it is very possible that my own setup just has something quirky going on which is why I run into this problem.
307765c
to
fbd28b1
Compare
@git-phu can we wait for @a-honcharenko to merge in his changes before we merge this in? |
@davinchia yup, this is not urgent, just creating a PR to document what I did in case we think it's useful. |
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.
Lgtm
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.
CODEOWNER files looking good to me
@davinchia looks like this should be good to merge now |
@git-phu can we do this tmrw? do one test on the multi-arch stuff. |
yup, just lmk know when is fine |
@git-phu since I held this up, I'll merge and test for you :) |
Looks good locally with the multi-arch changes. |
currently on m1 macs, switching between building for arm vs amd64 architectures is a bit cumbersome because some of the base docker images have not been parameterized yet, so you will run into build errors unless you untag those base images every time you switch between architectures. This PR should allow you switch freely between the two without needing that manual step. This PR also adds a single env var BUILD_ARCH that can be used to switch between building for arm vs amd64. With this PR we can build and push images for individual platform components which is much faster than trying to redeploy everything when iterating on changes that are limited to only a few components. Ideally we'd have a github action that allowed us to deploy individual platform components, but until that exists this seems like a reasonable solution for faster iteration.
currently on m1 macs, switching between building for arm vs amd64
architectures is a bit cumbersome because some of the base docker images
have not been parameterized yet, so you will run into build errors unless
you untag those base images every time you switch between architectures.
This PR should allow you switch freely between the two without needing
that manual step.
This PR also adds a single env var
BUILD_ARCH
that can be used toswitch between building for arm vs amd64.
With this PR we can build and push images for individual platform components
which is much faster than trying to redeploy everything when iterating on
changes that are limited to only a few components.
Ideally we'd have a github action that allowed us to deploy individual platform
components, but until that exists this seems like a reasonable solution for faster
iteration.