Release TerriaMap using create-docker-context #681
Conversation
deploy/docker/Dockerfile
Outdated
| @@ -1,5 +1,5 @@ | |||
| # Docker image for the primary terria map application server | |||
| FROM node:16 | |||
| FROM node:16-slim | |||
|
|
|||
| RUN mkdir -p /usr/src/app && mkdir -p /etc/config/client | |||
| WORKDIR /usr/src/app/component | |||
There was a problem hiding this comment.
I'm not against the change per se, but there are a lot of things I find confusing.
Is it the terriajs specified in package.json that gets installed, or does this pull from the TerriaJS repository? What creates the node_modules that are copied into /usr/src/app? Is there documentation for how to rebuild the release image locally without using Github actions?
Why does this Dockerfile use /usr/src/app and the Dockerfile in the root of this repository use /app?
There was a problem hiding this comment.
I've added an ADR (architecture decision record) explaining why I'm proposing this. Please comment on that, or here if you are still confused or disagree.
This dockerfile copies from a "context" that is created by create-docker-context.js - which is a script that copies only production dependencies and build artifacts to make a docker image. It's non-standard, but gets around running dependency installation and compilation on emulated architectures. The terriajs that is used is whichever is installed at the time that yarn docker-build-prod is run. We usually run releases from GitHub Actions, or if we need to build docker images locally we use a clean clone of TerriaMap. Both methods will always be a clean install pulling the terriajs version specified in package.json.
I think it's more normal to use /usr/src/app than /app
There was a problem hiding this comment.
Based on the description, this seems like a great idea.
Are the steps for how to build the docker image locally written down somewhere?
There was a problem hiding this comment.
Ok, ADR actually in this PR now. Sorry if you tried looking before, I forgot to push it.
As for building the docker image locally, this would be a separate command yarn docker-build-local, which doesn't do multi-arch builds. I haven't tested that one yet.
There was a problem hiding this comment.
I rebuild the release image to get security fixes and a few other local modifications, so I prefer a local build process that is as close to the release-image as possible so I don't have to re-do all the general quality assurance you have already done for the release.
I don't use multi-arch builds personally so not sure how important that is in the grand scheme of things; if I suddenly get ARM machines I can do separate builds on ARM/X86 and then stitch together the two images with crane or some other tool.
| @@ -1,5 +1,5 @@ | |||
| # Docker image for the primary terria map application server | |||
| FROM node:16 | |||
| FROM node:16-slim | |||
There was a problem hiding this comment.
This Dockerfile runs the node process as root unlike the Dockerfile in the root of this repository that runs it as node.
deploy/docker/Dockerfile
Outdated
| @@ -1,5 +1,5 @@ | |||
| # Docker image for the primary terria map application server | |||
| FROM node:16 | |||
| FROM node:16-slim | |||
|
|
|||
| RUN mkdir -p /usr/src/app && mkdir -p /etc/config/client | |||
| WORKDIR /usr/src/app/component | |||
There was a problem hiding this comment.
The files inside this directory are owned by uid 1001 and gid 127. Is there a reason to not use node:node as owner?
There was a problem hiding this comment.
No reason. I'll apply the changes you made to our other dockerfile here too.
…s and faster buildx builds
…Hub container registry
tephenavies
force-pushed
the
docker-context
branch
from
8dffe50
to
3641a36
Compare
| `create-docker-context.js`. | ||
| - Our GitHub Actions TerriaMap release time will reduce from 2.5 hours to less | ||
| than 10 minutes. | ||
| - If in future TerriaMap uses a binary installed by side effect during JS |
There was a problem hiding this comment.
I don't think this is a big deal, Github provides ARM runners now so if this becomes a problem in the future it's "easy" to rectify.
|
Does the |
|
It looks like my comments about users and permissions have been resolved, is this ready to be merged? |
Current TerriaMap ghcr.io/terriajs/terriamap releases take more than 2 hours to run various typescript and webpack builds on virtualised ARM. This PR switches those releases to use create-docker-context.js to install dependencies and build JS once on the current machine, then create the multi-arch docker image using those build artifacts and installed dependencies. Development dependencies also will no longer be shipped in our docker images as create-docker-context.js does not copy these.