Local deployment simulation #2774
Conversation
Add a just command (`just local-deploy`) to simulate deployment locally. Full explanation of usage in deploy/local/LOCAL_DEPLOYMENT_TESTING.md Updated the .dockerignore file so that when you build the image locally you don't get the database files bundled into the image. They're not meant to be built in to the image, and it can create enormous images if you have lots of local coding system databases.
StevenMaude
left a comment
StevenMaude
left a comment
There was a problem hiding this comment.
summary: This is close to done now.
I tried this out, and it worked fine for me. I've a couple of small comments, and a bigger question about the risks of the Docker setup.
| #!/bin/bash | ||
|
|
||
| export CONTAINER_NAME="opencodelists-local-deployment" | ||
| export CONTAINER_IMAGE="mcr.microsoft.com/devcontainers/universal:3" |
There was a problem hiding this comment.
thought: Maybe we don't need this particular image now, and there may be one more smaller and more suitable.
The reason for choosing a devcontainers image it was because this was originally to run in a dev container — but it's fine to leave it as is.
| ORIGINAL_DIR="$PWD" | ||
| TEMP_DIR="" | ||
|
|
||
| # Function to clean up when done |
There was a problem hiding this comment.
nit: There are a few comments in the scripts, like this one, that probably aren't necessary.
| rm -rf "$TEMP_DIR" | ||
| fi | ||
|
|
||
| # Return to original directory |
There was a problem hiding this comment.
nit: This is another slightly redundant comment that we could remove to keep things slightly shorter. We'd expect someone reading a shell script to understand what cd does.
| If you're feeling brave you can run this script to automatically view the logs, find the missing database, and copy it across. If you're more cautious, move to step 2. | ||
|
|
||
| ```bash | ||
| latest_db=$(docker exec -t opencodelists-local-deployment bash -c "docker logs opencodelists.web.1" | grep -oE 'coding_systems/[^ ]+\.sqlite3' | tail -n1) | ||
| if [[ -z "$latest_db" ]]; then | ||
| echo "No missing database found in logs." | ||
| else | ||
| echo "Latest missing database: $latest_db" | ||
| echo "Attempting to copy the database file..." | ||
| docker cp "$latest_db" opencodelists-local-deployment:/var/lib/dokku/data/storage/opencodelists/$latest_db | ||
| echo "Database file copied - try to load the codelist again." | ||
| fi | ||
| ``` |
There was a problem hiding this comment.
suggestion: I'd separate this from being interleaved with the instructions to do this manually; it could go before the manual steps, or after.
Maybe after is better? If you read the manual steps, you'll understand what the script is doing.
| # Check container logs | ||
| docker logs opencodelists-local-deployment |
There was a problem hiding this comment.
note: When I checked this, it was empty. I'm not sure if it's only if something gone's wrong that I should see anything there.
| ## TODO | ||
|
|
||
| - These scripts don't do any of the CI stuff from the main.yml and deploy.yml files. A lot of that is linting and testing which can be achieved with `just` commands. But there are some things like the smoke-test which may fail. Currently the smoke-test will fail as soon as you push your branch, so it will be caught before deployment. Therefore it's not essential to replicate that here, but it might be useful to add in the future. | ||
| - We might want a command to stop the container. Keeping it running means that redeploys are a lot faster so leaving for now - but it will consume system resources until stopped. |
There was a problem hiding this comment.
thought: If we don't have a full cleanup script yet, it might be helpful to have at least some guidance on how to clean this setup manually.
I was testing in Codespaces, and ran out of disk space after a couple of deploys — though the basic instance doesn't have lots of storage — so building this way is using a modest chunk of disk.
| - "3022:22" | ||
| - "8080:80" | ||
| - "8443:443" | ||
| - "7000:7000" |
There was a problem hiding this comment.
thought:
- I'm still not sure whether we need all of these ports; I partly cobbled this together from the Dokku documentation.
- This is how we typically configure ports in our applications with Docker Compose, but I think this does make them available over local networks because we don't use, for example:
127.0.0.1:7000:7000.
| -v "$(pwd):$WORKSPACE_DIR" \ | ||
| -w "$WORKSPACE_DIR" \ | ||
| -p "$HOST_PORT:7000" \ | ||
| --privileged \ |
There was a problem hiding this comment.
thought: --privileged isn't great from a security perspective. It is necessary in this case as a way of allowing the container to run Docker, which is what we want:
This flag exists to allow special use-cases, like running Docker within Docker.…
…Use the
--privilegedflag with caution. A container with--privilegedis not a securely sandboxed process. Containers in this mode can get a root shell on the host and take control over the system.
We don't control all of the code running inside the containers here either (we're running Dokku and OpenTelemetry, for example).
- It's possible to bind mount the Docker socket instead to allow the container to run Docker, but that also has risks.
- I think it's possible to run Podman rootless inside a container, but that's also more work, and not sure if anyone at Bennett has tried that.
- Maybe there's some other combination of command options that still allows this but mitigates the risk?
It's probably worth asking Simon to review this, and offer any suggestions.
There was a problem hiding this comment.
Good spot, in other places we've bind mounted the Docker socket for docker-in-docker, which I think is lower risk than using --privileged.
There was a problem hiding this comment.
I think the bind mounted socket is what dev containers do to make this work, but I couldn't easily discern what they did from a quick look at the docker-in-docker dev container feature. I couldn't find the use of --privileged there either though.
There was a problem hiding this comment.
I had a bit of a look at this and was able to remove --priveleged and mount the docker socket instead. It kind of worked but there were then other issues - largely because other bits of the scripts were working only because of that flag, and so needed changing. Probably no blockers, but it was beginning to spiral so I'm pulling out and putting back in your triage pile. I have some code in a local branch but pretty messy and the main bit is:
local DOCKER_GID
DOCKER_GID=$(stat -c '%g' /var/run/docker.sock)
log "Creating container '$CONTAINER_NAME' with Docker GID $DOCKER_GID..."
docker run -d \
--name "$CONTAINER_NAME" \
-v "$(pwd):$WORKSPACE_DIR" \
-v "/var/run/docker.sock:/var/run/docker.sock" \
--group-add "$DOCKER_GID" \
-w "$WORKSPACE_DIR" \
-p "$HOST_PORT:7000" \
"$CONTAINER_IMAGE" \
sleep infinityI also needed to change how and where the database file was created because that became inaccessible without the flag.
| log "No deployment needed." | ||
| echo -e " | ||
| ╔═════════════════════════════════════════════════════════════════════════════╗ | ||
| ║ 🚀 Which ever branch you were running last is now deployed locally. |
There was a problem hiding this comment.
nit:
| ║ 🚀 Which ever branch you were running last is now deployed locally. | |
| ║ 🚀 Whichever branch you were running last is now deployed locally. |
|
|
||
| ## TODO | ||
|
|
||
| - These scripts don't do any of the CI stuff from the main.yml and deploy.yml files. A lot of that is linting and testing which can be achieved with `just` commands. But there are some things like the smoke-test which may fail. Currently the smoke-test will fail as soon as you push your branch, so it will be caught before deployment. Therefore it's not essential to replicate that here, but it might be useful to add in the future. |
There was a problem hiding this comment.
thought: I think this is fine for now. The benefit is that we can least try out the deployment.
| └── opencodelists-local-deployment (Docker container) | ||
| ├── dokku container (deployment platform) | ||
| ├── otel-collector container (telemetry) | ||
| └── opencodelists container (the app) |
There was a problem hiding this comment.
Suggestion: update "opencodelists" to "opencodelists.web.1" to match the name of the app container running inside opencodelists-local-deployment.
| The system creates a container within a container setup: | ||
|
|
||
| - **Host container**: `opencodelists-local-deployment` - provides the Docker-in-Docker environment | ||
| - **Inner containers**: `dokku`, `otel-collector`, and your `opencodelists` application |
There was a problem hiding this comment.
Suggestion: update "opencodelists" to "opencodelists.web.1" to match the name of the app container running inside opencodelists-local-deployment.
|
There's a potentially simpler approach using Vagrant that was taken by Becky for a Job Server change that we could consider using instead of this. We're going to investigate it in order to do opensafely-core/job-server#4994 and can return to this PR then. |
UPDATE - I'm putting this back in triage as I tried to quickly remove the
--privelegedflag but now realise it might become a bit of a rabbit hole so I've stepped away.Add a just command (
just local-deploy) to simulate deployment locally.Full explanation of usage, architecture etc. in
./deploy/local/LOCAL_DEPLOYMENT_TESTING.md.This shouldn't make changes to your local database or environment so is safe to test repeatedly, and I have done locally. But I can't rule out that under different circumstances it might, so good for a few people to test locally and to backup anything in your dev environment (like the db) that you don't want to lose.