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

Modernize docker-compose.yml #464

Open
wants to merge 2 commits into
base: main
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 13 additions & 2 deletions .env.example
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
# This file contains Content Security Policy (CSP) directives to test Wagtail's compatibility with CSP.
# If the variables defined here are loaded into the environment, CSP will be enabled.
DJANGO_SETTINGS_MODULE=bakerydemo.settings.dev

REDIS_URL=redis://redis

# Database connection settings:
DATABASE_HOST=db
DATABASE_PORT=5432
Copy link
Contributor

Choose a reason for hiding this comment

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

.env is not just used for the Docker setup - if we make this change, it's going to break the virtualenv setup instructions, since we've not told people to set up a Postgres database there (and, of course, we'd rather not make that a requirement). Maybe there should be a separate .env.docker.example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good call, my brain is full of docker at the moment and lost site of the other install options. FYI, I'm wrapping up much more extensive docker compose "modernization" work over in wagtail/docker-wagtail-develop#71 which I'll backport here once complete.

DATABASE_NAME=app_db
DATABASE_USER=app_user
DATABASE_PASSWORD=changeme
DATABASE_URL=postgres://$DATABASE_USER:$DATABASE_PASSWORD@$DATABASE_HOST/$DATABASE_NAME

# Content Security Policy (CSP)
# To test Wagtail's compatibility with CSP, configure the variables below to enable CSP.
# These values are commented out by default because Wagtail is not (yet) compatible with
# the strict policy defined below.

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
steps:
- uses: actions/checkout@v3
- name: Build container
run: docker-compose build && docker-compose pull
run: cp .env.example .env && docker-compose build && docker-compose pull
- name: Run migrations
run: docker-compose run app /venv/bin/python manage.py migrate
- name: Load initial data
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: 3.9
python-version: 3.12
cache: 'pip'
cache-dependency-path: 'requirements/*.txt'
- name: Install dependencies
Expand Down
10 changes: 2 additions & 8 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM python:3.9-slim
FROM python:3.12-slim

# Install packages needed to run your application (not build deps):
# We need to recreate the /usr/share/man/man{1..8} directories first because
Expand Down Expand Up @@ -33,7 +33,7 @@ RUN set -ex \
zlib1g-dev \
" \
&& apt-get update && apt-get install -y --no-install-recommends $BUILD_DEPS \
&& python3.9 -m venv ${VIRTUAL_ENV} \
&& python3 -m venv ${VIRTUAL_ENV} \
&& pip install -U pip \
&& pip install --no-cache-dir -r /requirements/production.txt \
&& apt-get purge -y --auto-remove -o APT::AutoRemove::RecommendsImportant=false $BUILD_DEPS \
Expand All @@ -45,9 +45,6 @@ ADD . /code/
ENV PORT 8000
EXPOSE 8000

# Add custom environment variables needed by Django or your settings file here:
ENV DJANGO_SETTINGS_MODULE=bakerydemo.settings.production DJANGO_DEBUG=off

Copy link
Member

Choose a reason for hiding this comment

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

Question: Why are we removing the default settings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved DJANGO_SETTINGS_MODULE to .env/.env.example (although it points to the dev settings in this PR). DJANGO_DEBUG was not used anywhere.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should still keep the production default in the Dockerfile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there are undocumented use cases that require production-like settings and packages, could that be addressed by someone else who is aware of these scenarios in a separate PR? I can only work from the documented use cases in the readme, which has this to say about the docker compose setup:

**Important:** This `docker-compose.yml` is configured for local testing only, and is _not_ intended for production use.

The readme.md makes it sound like all paths for starting the demo are equivalent and only a matter of preference, but per my previous comment:

  • Gitpod installs requirements.txt which is essentially just an alias for requirements/production.txt
  • Vagrant installs requirements/base.txt
  • Docker Compose installs requirements/production.txt
  • Virtualenv installs requirements/development.txt

Either the readme needs some reworking or the configurations should be consistent with each other and with the readme. The simplest starting point for demo purposes is to install base.txt (and perhaps an extra file for docker to install the redis/psycopg3 libs).

For anyone who wishes to submit a PR to bakerydemo, they'll need to install development.txt (removing the -r base.txt from that file) in a virtualenv on their host so that they can run ruff etc. on their changes before submitting a PR (the contributing docs needs a note about installing pre-commit).

# Call collectstatic with dummy environment variables:
RUN DATABASE_URL=postgres://none REDIS_URL=none python manage.py collectstatic --noinput

Expand All @@ -57,8 +54,5 @@ RUN mkdir -p /code/bakerydemo/media/images && chown -R 1000:2000 /code/bakerydem
# mark the destination for images as a volume
VOLUME ["/code/bakerydemo/media/images/"]

# start uWSGI, using a wrapper script to allow us to easily add more commands to container startup:
ENTRYPOINT ["/code/docker-entrypoint.sh"]

# Start uWSGI
CMD ["uwsgi", "/code/etc/uwsgi.ini"]
56 changes: 29 additions & 27 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,38 +1,40 @@
version: '2'
volumes:
postgres-data:

services:
db:
environment:
POSTGRES_DB: app_db
POSTGRES_USER: app_user
POSTGRES_PASSWORD: changeme
restart: unless-stopped
image: postgres:14.1
expose:
- '5432'

redis:
restart: unless-stopped
image: redis:6.2
expose:
- '6379'

app:
environment:
DJANGO_SECRET_KEY: changeme
DATABASE_URL: postgres://app_user:changeme@db/app_db
REDIS_URL: redis://redis
DJANGO_SETTINGS_MODULE: bakerydemo.settings.dev
build:
context: .
dockerfile: ./Dockerfile
volumes:
- ./bakerydemo:/code/bakerydemo
links:
- db:db
- redis:redis
ports:
- '8000:8000'
depends_on:
- db
- redis
db:
condition: service_healthy
redis:
condition: service_healthy

db:
environment:
- "POSTGRES_DB=${DATABASE_NAME}"
- "POSTGRES_USER=${DATABASE_USER}"
- "POSTGRES_PASSWORD=${DATABASE_PASSWORD}"
restart: unless-stopped
image: postgres:16
volumes:
- postgres-data:/var/lib/postgresql/data
healthcheck:
test: "pg_isready --quiet --dbname=${DATABASE_URL}"
interval: 10s
timeout: 5s
retries: 5

redis:
restart: unless-stopped
image: redis:7
expose:
- '6379'
healthcheck:
test: ["CMD-SHELL", "redis-cli ping || exit 1"]
19 changes: 0 additions & 19 deletions docker-entrypoint.sh

This file was deleted.

18 changes: 8 additions & 10 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,19 +89,16 @@ Run the following commands:
```bash
git clone https://github.com/wagtail/bakerydemo.git
cd bakerydemo
cp .env.example .env
docker compose up --build -d
```

After this command completes and returns to the command prompt, wait 10 more seconds for the database setup to complete. Then run:
After this command completes and returns to the command prompt, run:

```bash
docker compose run app /venv/bin/python manage.py migrate
docker compose run app /venv/bin/python manage.py load_initial_data
```
If this fails with a database error, wait 10 more seconds and re-try. Finally, run:

```bash
docker compose up
docker compose exec app python manage.py migrate --noinput
docker compose exec app python manage.py load_initial_data
docker compose exec app python manage.py update_index
```

The demo site will now be accessible at [http://localhost:8000/](http://localhost:8000/) and the Wagtail admin
Expand All @@ -125,7 +122,7 @@ You can run the Wagtail demo locally without setting up Vagrant or Docker and si

#### Dependencies

- Python 3.7, 3.8, 3.9, 3.10 or 3.11
- Python 3.8, 3.9, 3.10, 3.11, or 3.12
- [Virtualenv](https://virtualenv.pypa.io/en/stable/installation/)
- [VirtualenvWrapper](https://virtualenvwrapper.readthedocs.io/en/latest/install.html) (optional)

Expand All @@ -141,7 +138,7 @@ Confirm that this is showing a compatible version of Python 3.x. If not, and you

deactivate
rmvirtualenv wagtailbakerydemo
mkvirtualenv wagtailbakerydemo --python=python3.9
mkvirtualenv wagtailbakerydemo --python=python3.12
python --version

Now we're ready to set up the bakery demo project itself:
Expand All @@ -161,6 +158,7 @@ To set up your database and load initial data, run the following commands:

./manage.py migrate
./manage.py load_initial_data
./manage.py update_index
./manage.py runserver

Log into the admin with the credentials `admin / changeme`.
Expand Down