Update Docker build artifacts to new installer pattern#1629
Conversation
Following up on #1433 which re-introduced target triplets and the installer to support it, this utilizes the installer in our Docker build steps. This consists of: - Switching our direct downloading of releases to go through our unified installer endpoint. - Durably uses the installer script to download the right architecture for a particular release using the logic in the installer for that release. - There is now a default `ROUTER_RELEASE` value of `latest` on the images, though the argument still works (and is used by our releasing pipeline) - Simplifies some logic (e.g., transforming version numbers to remove `v`). - Switching from `alpine` to `debian` since our installer doesn't support `alpine` and fails. This is _not_ a material change to the outcome because we were only using the alpine image to download a tarball, not for runtime. (Fwiw, it's worth, this change wouldn't have been necessary, but our installer runs `router --version` at the end which fails on Alpine.) Conceviably our Oribiter endpoint can just redirect to the tarballs with a small change which it already does for Rover, but this was quicker.
|
@abernix your pull request is missing a changelog! |
| - run: | ||
| name: Docker build | ||
| command: | | ||
| ROUTER_RELEASE=${VERSION:1} |
There was a problem hiding this comment.
This was removing the letter v from the Git release tag (which is inclusive of the v) so that it could pass it to ROUTER_RELEASE below. The Docker image build now defaults to latest (and accepts --build-arg ROUTER_RELEASE=latest), so we can pass this value in opaquely now.
This does mean that users do pass --build-arg ROUTER_RELEASE=... they need to pass ROUTER_RELEASE=v0.16.0 or ROUTER_RELEASE=latest, but that actually seems intuitive to me in the context of the notion of "release".
It is a breaking change for current Router image builders, but this seems like a good time to make a breaking change of that sort!
| ARG DEBUG_IMAGE | ||
| # Build is required to extract the release files | ||
| FROM --platform=linux/amd64 alpine:latest AS build | ||
| FROM --platform=linux/amd64 debian:bullseye-slim AS build |
There was a problem hiding this comment.
As noted in the PR body, this accounts for the fact that the installer necessitates an architecture that we can actually run the Router on and switches alpine:latest for a specific version of debian:*slim images.
There is no material change on the outcome of the image — we were using the Alpine image to download a tarball for the next build stage, not to create a small output image, nor on account of producing a Router image that runs on Alpine. Today, the Router does not support running on Alpine for reasons we will solve in the future.
| FROM --platform=linux/amd64 debian:bullseye-slim AS build | ||
|
|
||
| ARG ROUTER_RELEASE | ||
| ARG ROUTER_RELEASE=latest |
There was a problem hiding this comment.
I think this default makes a lot of sense and offers a nice user experience and pairs nicely with the capability of our router install endpoint.
| ADD https://github.com/apollographql/router/releases/download/v${ROUTER_RELEASE}/router-${ROUTER_RELEASE}-x86_64-unknown-linux-gnu.tar.gz /tmp/router.tar.gz | ||
| WORKDIR /dist | ||
|
|
||
| WORKDIR /tmp |
There was a problem hiding this comment.
We no longer need to work in a /tmp directory and then moving it to a /dist since the installer actually just takes care of the extraction itself.
| RUN mkdir /dist/config && mkdir /dist/schema | ||
| RUN mkdir config schema | ||
|
|
||
| # Copy configuration for docker image | ||
| COPY dockerfiles/router.yaml /dist/config | ||
| COPY dockerfiles/router.yaml config |
There was a problem hiding this comment.
Effectively using WORKDIR like a variable here to DRY up the repetition of /dist/
| else | ||
| # Let the user know what we are going to do | ||
| echo "Building image: ${ROUTER_VERSION}" from released tarballs"" | ||
| ROUTER_RELEASE="$(echo "${ROUTER_VERSION}" | cut -c2-)" |
There was a problem hiding this comment.
In the same spirit as the change in the .circleci/config.yml above, this transformation of the ROUTER_VERSION to remove the v character is no longer necessary and we can treat this like an opaque value.
| @@ -3,40 +3,44 @@ | |||
| # builds to work on platforms such as Mac OS X/M1 (with some help from | |||
There was a problem hiding this comment.
This file is almost fully identical to the Dockerfile.router above, now more than ever. The only notable difference is that the other file above supports the debug build argument. We should align these in the future?
garypen
left a comment
There was a problem hiding this comment.
Looks good. Just one question about ca-certificates.
| - run: | ||
| name: Docker build | ||
| command: | | ||
| ROUTER_RELEASE=${VERSION:1} |
There was a problem hiding this comment.
Nice to see all this dancing around to add/remove the "v" prefix is gone...
| # router.tar.gz extracts to "dist" | ||
| RUN tar xvzf router.tar.gz -C / | ||
| # Run the Router downloader which puts Router into current working directory | ||
| RUN curl -sSL https://router.apollo.dev/download/nix/${ROUTER_RELEASE}/ | sh |
There was a problem hiding this comment.
I'm assuming no certificate issues with debian:bullseye-slim. i.e.: no need to install ca-certificates.
There was a problem hiding this comment.
I think curl just actually brings in ca-certificates now.
Following up on #1433 which re-introduced target triplets and the installer to support it, this utilizes the installer in our Docker build steps.
This consists of:
ROUTER_RELEASEvalue oflateston the images, though the argument still works (and is used by our releasing pipeline)v).alpinetodebiansince our installer doesn't supportalpineand fails. This is not a material change to the outcome because we were only using the alpine image to download a tarball, not for runtime. (Fwiw, it's worth, this change wouldn't have been necessary, but our installer runsrouter --versionat the end which fails on Alpine.) Conceviably our Oribiter endpoint can just redirect to the tarballs with a small change which it already does for Rover, but this was quicker.