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

Improve to use multi-arch images when building services #250

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Expand Up @@ -37,13 +37,13 @@ An alternative version of the app uses Windows containers based on Nano Server.
You can build from source using:

```
docker-compose -f docker-compose-windows.yml build
docker-compose -f compose-windows.yml build
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit on the fence on renaming docker-compose*.yml to compose.yml (also still curious what led to the decision to make it the recommended name); I think docker-compose.yml is more familiar to users (and more recognisable) than compose.yml. (every time I see compose.yml, I confuse it for composer.json

```

Then run the app using:

```
docker-compose -f docker-compose-windows.yml up -d
docker-compose -f compose-windows.yml up -d
```

> Or in a Windows swarm, run `docker stack deploy -c docker-stack-windows.yml vote`
Expand Down
2 changes: 0 additions & 2 deletions docker-compose-javaworker.yml → compose-javaworker.yml
@@ -1,5 +1,3 @@
version: "3"

services:
vote:
build: ./vote
Expand Down
2 changes: 0 additions & 2 deletions docker-compose-k8s.yml → compose-k8s.yml
@@ -1,5 +1,3 @@
version: '3'

services:
redis:
image: redis:alpine
Expand Down
2 changes: 0 additions & 2 deletions docker-compose-simple.yml → compose-simple.yml
@@ -1,5 +1,3 @@
version: "3"

services:
vote:
build: ./vote
Expand Down
2 changes: 0 additions & 2 deletions docker-compose-windows-1809.yml → compose-windows-1809.yml
@@ -1,5 +1,3 @@
version: "3.2"

services:
vote:
image: dockersamples/examplevotingapp_vote:dotnet-nanoserver-1809
Expand Down
2 changes: 0 additions & 2 deletions docker-compose-windows.yml → compose-windows.yml
@@ -1,5 +1,3 @@
version: "3.2"

services:
vote:
image: dockersamples/examplevotingapp_vote:dotnet-nanoserver-sac2016
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion docker-compose.yml → compose.yml
Expand Up @@ -57,7 +57,7 @@ services:
- back-tier

db:
image: postgres:9.4
image: postgres:12.12-bullseye
Copy link
Member

Choose a reason for hiding this comment

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

I see this compose file bind-mounts various things, including health-checks; would it be "better practice" to have a build: for these, so that the healtchchecks are part of the image? That way;

  • the compose file can also run on a remote Docker Engine, not just on a local machine
  • the versions are defined in the Dockerfile (not spread over Dockerfile and compose-file)

environment:
POSTGRES_USER: "postgres"
POSTGRES_PASSWORD: "postgres"
Expand Down
2 changes: 1 addition & 1 deletion result/Dockerfile
@@ -1,4 +1,4 @@
FROM node:10-slim
FROM --platform=$BUILDPLATFORM node:18.0-bullseye-slim
Copy link
Member

Choose a reason for hiding this comment

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

While we're updating these files, we should probably add a # syntax line, to follow best practice (and to make sure we're always building with the current syntax); (Also see my other comment w.r.t. --platform)

Suggested change
FROM node:10-slim
FROM --platform=$BUILDPLATFORM node:18.0-bullseye-slim
# syntax=docker/dockerfile:1
FROM node:18.0-bullseye-slim

Some other suggestions:

  • As we're not always keeping a close eye on this repository to keep it up-to-date, perhaps we should add build-args (ARG) to allow overriding versions
  • That said; I'm a bit on the fence on adding the -bullseye. The "pro" is that the image wouldn't unexpectedly switch to bookworm (debian 12), but I think for these examples, things would likely "just work" with newer versions. Pinning to bullseye may also mean that (if we do add build-args) switching to (say) Node 27.0 may mean that there's no longer a bullseye variant (but only a bookworm variant).
Suggested change
FROM node:10-slim
FROM --platform=$BUILDPLATFORM node:18.0-bullseye-slim
# syntax=docker/dockerfile:1
ARG NODE_VERSION=18.0
FROM node:${NODE_VERSION}-slim

I wonder if we should switch back to -alpine (looks to be still ~30MB smaller); I see it was changed from -alpine to -slim in #140, because there was a bug in npm (npm/uid-number#7), but that one looks to be fixed now.


# add curl for healthcheck
RUN apt-get update \
Copy link
Member

Choose a reason for hiding this comment

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

(can't comment on the lines further down)

Perhaps we should make this a multi-stage Dockerfile, and move the tini download to a separate stage that we --copy from 🤔

# Add Tini for proper init of signals
ENV TINI_VERSION v0.19.0
ADD https://github.com/krallin/tini/releases/download/${TINI_VERSION}/tini /tini
RUN chmod +x /tini

Now that ADD also supports --chmod, we can skip the RUN, so it can be a FROM scratch;

FROM scratch AS tini
ARG TINI_VERSION=v0.19.0
ADD --chmod=0755 https://github.com/krallin/tini/releases/download/${TINI_VERSION}/tini /tini

Copy link
Member

Choose a reason for hiding this comment

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

Erm... looks like there's more to fix; this doesn't take multi-arch into account, and always downloads x86 (also looks like it's not statically linked?); https://github.com/krallin/tini/releases/tag/v0.19.0

docker build -t foo -<<'EOF'
FROM scratch AS tini
ARG TINI_VERSION=v0.19.0
ADD --chmod=0755 https://github.com/krallin/tini/releases/download/${TINI_VERSION}/tini /tini
EOF

docker run --rm foo /tini --version
qemu-x86_64: Could not open '/lib64/ld-linux-x86-64.so.2': No such file or directory

And doesn't work on alpine 😞

ldd /tini
	/lib/ld-linux-aarch64.so.1 (0xffffb203e000)
	libc.so.6 => /lib/ld-linux-aarch64.so.1 (0xffffb203e000)
Error relocating /tini: __fprintf_chk: symbol not found

This looks to work though;

FROM scratch AS tini
ARG TINI_VERSION=v0.19.0
ARG TARGETARCH
ADD --chmod=0755 https://github.com/krallin/tini/releases/download/${TINI_VERSION}/tini-${TARGETARCH} /tini

FROM debian:bullseye-slim
COPY --from=tini /tini /tini
docker run --rm foo /tini --version
tini version 0.19.0 - git.de40ad0
docker run --rm foo /tini --version
WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested
tini version 0.19.0 - git.de40ad0

Expand Down
2 changes: 0 additions & 2 deletions result/docker-compose.test.yml
@@ -1,5 +1,3 @@
version: '2'

services:

sut:
Expand Down
2 changes: 1 addition & 1 deletion seed-data/Dockerfile
@@ -1,4 +1,4 @@
FROM python:3.9-slim
FROM --platform=$BUILDPLATFORM python:3.10-bullseye
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't the --platform=$BUILDPLATFORM now prevent me from using QEMU to build for other architectures?

--platform=$BUILDPLATFORM works if the stage does a cross-compile (so toolset is "native" but the artefacts produced are for another architecture), but in this case, no cross-compile is happening, which means that if we would build with docker build --platform=linux/amd64 on an arm64 machine, we would produce an image with linux/amd64, but the content actually being linux/arm64


# add apache bench (ab) tool
RUN apt-get update \
Expand Down
2 changes: 1 addition & 1 deletion vote/Dockerfile
@@ -1,5 +1,5 @@
# Using official python runtime base image
FROM python:3.9-slim
FROM --platform=$BUILDPLATFORM python:3.10-bullseye

# add curl for healthcheck
RUN apt-get update \
Expand Down
4 changes: 2 additions & 2 deletions worker/Dockerfile
@@ -1,4 +1,4 @@
FROM mcr.microsoft.com/dotnet/core/sdk:3.1 as builder
FROM --platform=$BUILDPLATFORM mcr.microsoft.com/dotnet/sdk:6.0-bullseye-slim as builder

WORKDIR /Worker
COPY src/Worker/Worker.csproj .
Expand All @@ -8,7 +8,7 @@ COPY src/Worker/ .
RUN dotnet publish -c Release -o /out Worker.csproj

# app image
FROM mcr.microsoft.com/dotnet/core/runtime:3.1
FROM mcr.microsoft.com/dotnet/runtime:6.0-bullseye-slim

WORKDIR /app
ENTRYPOINT ["dotnet", "Worker.dll"]
Expand Down
4 changes: 2 additions & 2 deletions worker/Dockerfile.j
@@ -1,4 +1,4 @@
FROM maven:3.5-jdk-8-alpine AS build
FROM --platform=$BUILDPLATFORM maven:3.8.6-eclipse-temurin-19-focal AS build

WORKDIR /code

Expand All @@ -10,7 +10,7 @@ RUN ["mvn", "verify"]
COPY ["src/main", "/code/src/main"]
RUN ["mvn", "package"]

FROM openjdk:8-jre-alpine
FROM eclipse-temurin:19_36-jre-jammy

COPY --from=build /code/target/worker-jar-with-dependencies.jar /

Expand Down
4 changes: 2 additions & 2 deletions worker/dotnet/Dockerfile
@@ -1,4 +1,4 @@
FROM microsoft/dotnet:2.1-sdk-nanoserver-sac2016 as builder
FROM --platform=$BUILDPLATFORM mcr.microsoft.com/dotnet/sdk:6.0-bullseye-slim as builder

WORKDIR /Worker
COPY Worker/Worker.csproj .
Expand All @@ -8,7 +8,7 @@ COPY /Worker .
RUN dotnet publish -c Release -o /out Worker.csproj

# app image
FROM microsoft/dotnet:2.1-runtime-nanoserver-sac2016
FROM mcr.microsoft.com/dotnet/runtime:6.0-bullseye-slim

WORKDIR /app
ENTRYPOINT ["dotnet", "Worker.dll"]
Expand Down
2 changes: 1 addition & 1 deletion worker/dotnet/Dockerfile.1809
@@ -1,4 +1,4 @@
FROM mcr.microsoft.com/dotnet/core/sdk:3.1 as builder
FROM --platform=$BUILDPLATFORM mcr.microsoft.com/dotnet/core/sdk:3.1 as builder

WORKDIR /Worker
COPY Worker/Worker.csproj .
Expand Down
18 changes: 9 additions & 9 deletions worker/dotnet/Worker/Worker.csproj
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp3.1</TargetFramework>
<TargetFramework>netcoreapp6.0</TargetFramework>
</PropertyGroup>

<ItemGroup>
Expand All @@ -17,14 +17,14 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.EntityFrameworkCore" Version="2.0.3" />
<PackageReference Include="Microsoft.Extensions.Configuration.EnvironmentVariables" Version="2.1.1" />
<PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="2.1.1" />
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="2.1.1" />
<PackageReference Include="Microsoft.Extensions.Logging" Version="2.1.1" />
<PackageReference Include="Microsoft.Extensions.Logging.Console" Version="2.1.1" />
<PackageReference Include="MySql.Data.EntityFrameworkCore" Version="8.0.12" />
<PackageReference Include="NATS.Client" Version="0.8.1" />
<PackageReference Include="Microsoft.EntityFrameworkCore" Version="6.0.9" />
<PackageReference Include="Microsoft.Extensions.Configuration.EnvironmentVariables" Version="6.0.9" />
<PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="6.0.9" />
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="6.0.9" />
<PackageReference Include="Microsoft.Extensions.Logging" Version="6.0.9" />
<PackageReference Include="Microsoft.Extensions.Logging.Console" Version="6.0.9" />
<PackageReference Include="MySql.Data.EntityFrameworkCore" Version="8.0.22" />
<PackageReference Include="NATS.Client" Version="1.0.1" />
</ItemGroup>

</Project>
8 changes: 4 additions & 4 deletions worker/src/Worker/Worker.csproj
Expand Up @@ -2,13 +2,13 @@

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp3.1</TargetFramework>
<TargetFramework>netcoreapp6.0</TargetFramework>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="StackExchange.Redis" Version="2.0.601" />
<PackageReference Include="Npgsql" Version="4.0.9" />
<PackageReference Include="Newtonsoft.Json" Version="12.0.2" />
<PackageReference Include="StackExchange.Redis" Version="2.6.66" />
<PackageReference Include="Npgsql" Version="6.0.7" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
</ItemGroup>

</Project>