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

parameterize jdk and nginx base images #11262

Merged
merged 3 commits into from
May 12, 2022

Conversation

git-phu
Copy link
Contributor

@git-phu git-phu commented Mar 18, 2022

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.

@git-phu git-phu force-pushed the peter/m1-building-docker-images branch from 5913dcd to 307765c Compare March 18, 2022 18:33
@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Mar 18, 2022
@git-phu git-phu temporarily deployed to more-secrets March 18, 2022 18:34 Inactive
@git-phu git-phu temporarily deployed to more-secrets March 18, 2022 18:35 Inactive
@@ -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
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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.

@CLAassistant
Copy link

CLAassistant commented May 5, 2022

CLA assistant check
All committers have signed the CLA.

@git-phu git-phu force-pushed the peter/m1-building-docker-images branch from 307765c to fbd28b1 Compare May 6, 2022 02:32
@github-actions github-actions bot removed the area/documentation Improvements or additions to documentation label May 6, 2022
@git-phu git-phu temporarily deployed to more-secrets May 6, 2022 02:35 Inactive
@git-phu git-phu temporarily deployed to more-secrets May 6, 2022 02:35 Inactive
@git-phu git-phu marked this pull request as ready for review May 6, 2022 15:32
@git-phu git-phu requested a review from a team as a code owner May 6, 2022 15:32
@git-phu git-phu requested review from a team and terencecho and removed request for a team May 6, 2022 15:32
@davinchia
Copy link
Contributor

@git-phu can we wait for @a-honcharenko to merge in his changes before we merge this in?

@git-phu
Copy link
Contributor Author

git-phu commented May 6, 2022

@davinchia yup, this is not urgent, just creating a PR to document what I did in case we think it's useful.

Copy link
Contributor

@terencecho terencecho left a comment

Choose a reason for hiding this comment

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

Lgtm

Copy link
Collaborator

@timroes timroes left a 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

@git-phu
Copy link
Contributor Author

git-phu commented May 11, 2022

@davinchia looks like this should be good to merge now

@davinchia
Copy link
Contributor

@git-phu can we do this tmrw? do one test on the multi-arch stuff.

@git-phu
Copy link
Contributor Author

git-phu commented May 11, 2022

yup, just lmk know when is fine

@davinchia davinchia merged commit c69423b into master May 12, 2022
@davinchia davinchia deleted the peter/m1-building-docker-images branch May 12, 2022 09:36
@davinchia
Copy link
Contributor

@git-phu since I held this up, I'll merge and test for you :)

@davinchia
Copy link
Contributor

Looks good locally with the multi-arch changes.

suhomud pushed a commit that referenced this pull request May 23, 2022
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform area/scheduler area/server area/worker Related to worker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants