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

Add Docker image build #17

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
.dockerignore
.gitignore
.env
docker-compose.yml
Dockerfile
env/
venv/

cache/
/web/static/
/web/media/
*credentials*

.idea/
*.iml

__pycache__/
*.pyc

data/
52 changes: 52 additions & 0 deletions .github/workflows/build-and-publish.yaml

Choose a reason for hiding this comment

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

As the CI is configured right now, the image will not have (meaningful) OCI annotations, except the (deprecated) plain maintainer label in Dockerfile.

I suggest adding helpful OCI annotations like org.opencontainers.image.source (to find the underlying Git repo) or org.opencontainers.image.licenses (to quickly find out ParkAPI2's license(s)), either

Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
name: build & publish Docker image

on:
push:
branches:
# TODO: rename master to main
- master
# TODO: remove before merge
- docker

jobs:
build-publish:
runs-on: ubuntu-latest
steps:
- name: checkout
uses: actions/checkout@v4
with:
submodules: recursive

- name: set up Docker buildx
uses: docker/setup-buildx-action@v2

- name: log into the GitHub Container Registry
uses: docker/login-action@v2
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ github.token }}

- name: lowercase repo name
id: lower-repo
uses: ASzc/change-string-case-action@v6

Choose a reason for hiding this comment

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

It seems like relying on this unpinned "random" third-party dependency is not necessary because there are various built-in utilities to lower-case a string.

Suggested change
uses: ASzc/change-string-case-action@v6
run: |
echo "lowercase=$(echo -n '${{ github.repository }}' | tr '[:upper:]' '[:lower:]')" >> $GITHUB_OUTPUT

with:
string: ${{ github.repository }}

- name: set current date as env variable
id: version
run: |
echo "builddate=$(date +'%Y-%m-%d')" >> $GITHUB_OUTPUT

- name: build and push Docker image
uses: docker/build-push-action@v4
with:
file: Dockerfile
push: true
tags: |
ghcr.io/${{ steps.lower-repo.outputs.lowercase }}:latest,ghcr.io/${{ steps.lower-repo.outputs.lowercase }}:${{ steps.version.outputs.builddate }}

Choose a reason for hiding this comment

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

This will create an image named parkendd/parkapi2, where as ParkAPI v3 images are named parkendd/park-api-v3. Do we want to align the v2 naming scheme with the v3 one?

platforms: linux/amd64,linux/arm64
# https://docs.docker.com/build/ci/github-actions/cache/#cache-backend-api
cache-from: type=gha
cache-to: type=gha,mode=max

2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ cache/

__pycache__/
*.pyc

data/
34 changes: 34 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
FROM ghcr.io/osgeo/gdal:alpine-small-3.9.2 AS builder

Choose a reason for hiding this comment

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

This is not a "floating tag", but hard-codes as specific set of libraries, which means that we won't get security patches "automatically" with a rebuild. We should make sure that there is a tool like Mend Renovate (bot) that notifies about updates to this image.

LABEL maintainer="Holger Bruch <[email protected]>"

RUN apk add --no-cache build-base python3-dev py3-pip postgresql-client libpq-dev geos-dev


RUN python -m venv /opt/venv
ENV PATH="/opt/venv/bin:$PATH"

WORKDIR /app
COPY requirements.txt .
COPY web/scrapers/ParkAPI2_sources/requirements.txt ./ParkAPI2_sources_requirements.txt

RUN pip install -r requirements.txt -r ParkAPI2_sources_requirements.txt

# Operational stage
FROM ghcr.io/osgeo/gdal:alpine-small-3.9.2
LABEL maintainer="Holger Bruch <[email protected]>"

RUN apk add --no-cache bash git python3 py3-pip postgresql-client geos

COPY --from=builder /opt/venv /opt/venv
ENV RUN_MIGRATION=0 \
CREATE_SUPERUSER=0 \
ASSIGN_LOCATIONS=0 \
PYTHONBUFFERED=1 \
PATH="/opt/venv/bin:$PATH"

WORKDIR /app
COPY . /app/

EXPOSE 8000
ENTRYPOINT ["/bin/bash", "/app/entrypoint.sh"]
CMD runserver 0.0.0.0:8000
45 changes: 45 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
version: '3'
services:
db:
image: postgis/postgis:16-3.4
volumes:
- ./data:/var/lib/postgresql/data
env_file: .env
ports:
- 5432:5432
healthcheck:
test: ["CMD-SHELL", "pg_isready -U parkapi2"]
interval: 10s
timeout: 5s
retries: 5
start_period: 10s
api:
build:
context: .
dockerfile: ./Dockerfile
command: ["runserver", "0.0.0.0:8000"]
depends_on:
db:
condition: service_healthy
expose:
- 8000
ports:
- 8000:8000
env_file: .env
environment:
- RUN_MIGRATION=1
- ASSIGN_LOCATIONS=1

scraper:
build:
context: .
dockerfile: ./Dockerfile
command:
- pa_scrape
- scrape
- -j
- "100"
depends_on:
db:
condition: service_healthy
env_file: .env
24 changes: 24 additions & 0 deletions entrypoint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/bin/bash
set -euo pipefail

cd web

Choose a reason for hiding this comment

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

This script won't abort when commands fail and with unbound (a.k.a. undefined) variables (as with the typo below). However, given that some operating systems' sh shells seem to not support set -o pipefail yes, even though it should be supported everywhere, I suggest using bash instead.

Also, add a shebang, to avoid confusion about which shell to run this file with.

Suggested change
cd web
#!/bin/bash
set -eu -o pipefail
cd web

if [ "$RUN_MIGRATION" -eq "1" ]; then
echo "RUN MIGRATION is TRUE"

Choose a reason for hiding this comment

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

[nitpick]
You might want to log this to stderr.

Suggested change
echo "RUN MIGRATION is TRUE"
>&2 echo "RUN MIGRATION is TRUE"

fi

# Run migration only if explicitly set via ENV
if [ "$RUN_MIGRATION" -eq "1" ]; then
./manage.py migrate
fi

if [ "$ASSIGN_LOCATIONS" -eq "1" ]; then
./manage.py pa_find_locations
fi

# don't create an admin interface per default
if [ "$CREATE_SUPERUSER" -eq "1" ]; then
./manage.py createsuperuser
fi

# start the server
./manage.py $@

Choose a reason for hiding this comment

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

You might want to use exec here.

[…] If we call prog1 -> prog2 -> prog3 -> prog4 etc. and never go back, then make each call an exec. It saves resources (not much, admittedly, unless repeated) and makes shutdown simplier.

StackOverflow explainer about exec's purposes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

After entrypoint.sh is run, the container CMD should be executed. IMHO exec would prevent this(?)

Choose a reason for hiding this comment

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

I think we have a misunderstanding here. I referred to the exec Bash built-in. How would it prevent running the command specified in CMD?

Choose a reason for hiding this comment

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

Just using $@ verbatim has unintended consequences! ⚠️ Compare these two:

bash -c 'python3 -c "import sys;print(sys.argv[2:])" -- $@' -- foo 'bar baz' --hello --world
# ['foo', 'bar', 'baz', '--hello', '--world']
bash -c 'python3 -c "import sys;print(sys.argv[2:])" -- "$@"' -- foo 'bar baz' --hello --world
# ['foo', 'bar baz', '--hello', '--world']

Loading