Skip to content

Commit

Permalink
Fix docker configuration
Browse files Browse the repository at this point in the history
The job-runner container uses the host's docker service. This means that
when it bind mounts directories with `docker run`, the source path must
exist on the *hosts* file system.

We've fixed this already for tests, with PYTEST_HOST_TMP, but we had not
actually fixed it for actual using the docker image for real. Before the
previous change to use --mount rather than --volume, a job would
succeed, but the files would be in the wrong location. Now we have that
change, the job fails.

We use a simple if inelegant fix for this: we mount the STORAGE_BASE
directories inside the container at the same path as on the host. This
means that the same config value can be used for job-runner copying
files itself, or for passing the host's docker to mount.

As part of this, I changedthe way we supply these values to being in
one time generated docker/.env file. This is more discoverable, usable
outside of just, and more efficant than before. I chose to use
`docker/medium` and `docker/high` as the volume mount locations, for
simplicity, and also, so the the docker machinery is isolated from the
non-docker venv set up.

This include a basic `just docker/functional-test` command to spin up
a job-runner container and run an actual job on it.
  • Loading branch information
bloodearnest committed Oct 1, 2024
1 parent f3d1904 commit 7d73d6a
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 23 deletions.
2 changes: 2 additions & 0 deletions docker/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
medium/
high/
24 changes: 18 additions & 6 deletions docker/docker-compose.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# note: this docker-compose file is intended to define the *building* of
# job-runner images, and running them locally in dev, not for running them in
# production
name: job-runner

services:
prod:
# image name, both locally and public
Expand Down Expand Up @@ -29,24 +31,34 @@ services:
group_add: ["${DOCKER_HOST_GROUPID}"]
# paths are relative to docker-compose.yaml
volumes:
# use the default dev workdir
- ../workdir:/workdir
# docker control
- /var/run/docker.sock:/var/run/docker.sock
# use the default dev workdir
- ../workdir:/workdir
# used to avoid ugly uid/gid lookup failures, but not strictly required
- /etc/group:/etc/group:ro
- /etc/passwd:/etc/passwd:ro
# these paths must be absolute on the host, and must be mount to the
# identical absolute paths inside the container. This allows for
# job-runner to use the same value for both its own file access, and for
# what to pass to the host docker as a bindmount source directory, which
# needs to be on the host.
# We can potentially fix this by adding an explicit bind mount source configs, if we want to
- ${MEDIUM_PRIVACY_STORAGE_BASE}:${MEDIUM_PRIVACY_STORAGE_BASE}
- ${HIGH_PRIVACY_STORAGE_BASE}:${HIGH_PRIVACY_STORAGE_BASE}
# paths relative to docker-compose.yaml file
env_file:
# load jobrunner local dev config
- ../.env
- docker-compose.env
# explicitly load local .env after parent dir .env
- .env
# ensure WORKDIR environment points to fixed location
environment:
# default dev config
WORKDIR: /workdir
MEDIUM_PRIVACY_STORAGE_BASE: /workdir/workspaces
HIGH_PRIVACY_STORAGE_BASE: /workdir/workspaces
DOCKER_HOST: ${DOCKER_HOST:-unix:///var/run/docker.sock}
# force using mounted docker socket
DOCKER_HOST: unix:///var/run/docker.sock
# used to enable in CI
PRIVATE_REPO_TOKEN: ${PRIVATE_REPO_TOKEN:-}

# main development service
Expand Down
54 changes: 37 additions & 17 deletions docker/justfile
Original file line number Diff line number Diff line change
@@ -1,26 +1,13 @@
set dotenv-load := true

export DOCKER_USERID := `id -u`
export DOCKER_GROUPID := `id -g`
export PYTEST_HOST_TMP := "/tmp/jobrunner-docker"

# the dev build needs to know the hosts docker group ID, to match it in the container

export DOCKER_HOST_GROUPID := `getent group docker | awk -F: '{print $3}'`

# dev builds remap's appuser's uid to the running user, for easy file
# permissions when we mount things in.

export DEV_USERID := `id -u`
export DEV_GROUPID := `id -g`

build env="dev": tmpdir
build env="dev": tmpdir docker-compose-env
#!/usr/bin/env bash
set -eu
# ensure env files exist
# ensure parent env files exist
test -f ../.env || cp ../dotenv-sample ../.env
touch docker-compose.env

# enable modern docker build features
export DOCKER_BUILDKIT=1
Expand All @@ -41,17 +28,50 @@ tmpdir:
fi
mkdir $PYTEST_HOST_TMP

docker-compose-env:
#!/bin/bash
mkdir -p {medium,high}
test -f .env && exit 0
cat <<EOF > .env
DEV_USERID=$(id -u)
DEV_GROUPID=$(id -g)
DOCKER_HOST_GROUPID=$(getent group docker | awk -F: '{print $3}')
PYTEST_HOST_TMP=$PYTEST_HOST_TMP
MEDIUM_PRIVACY_STORAGE_BASE=$(realpath $PWD/medium)
HIGH_PRIVACY_STORAGE_BASE=$(realpath $PWD/high)
EOF

# run tests in docker container
test *args: build
#!/bin/bash
docker compose run --rm test {{ args }}

functional-test:
#!/bin/bash
container=job-runner-prod-1
trap 'docker compose kill prod' EXIT
docker compose up -d prod
docker compose exec prod python3 -m jobrunner.cli.add_job https://github.com/opensafely/research-template generate_dataset
attempts=""
# use docker logs reather than docker compose logs, as those will include previous run's logs
while ! docker logs -n 10 $container |& grep -q "Completed successfully";
do
if test "$attempts" = ".........."; then
docker compose logs prod
exit 1;
fi
attempts="${attempts}."
echo "waiting..."
sleep 1
done
docker logs -n 10 $container |& grep "Completed successfully"

# run dev server in docker container
service: build
docker compose up dev

# run command in dev container
run env="dev" *args="bash": build
run env="dev" *args="": build
docker compose run --rm {{ env }} {{ args }}

# exec command in existing dev container
Expand All @@ -60,5 +80,5 @@ exec *args="bash":

clean:
#!/bin/bash
rm -rf $PYTEST_HOST_TMP
rm -rf $PYTEST_HOST_TMP .env
docker image rm job-runner job-runner-dev || true

0 comments on commit 7d73d6a

Please sign in to comment.