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

Faster post build startup: #22938

Merged
merged 1 commit into from
Dec 13, 2024
Merged
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
10 changes: 9 additions & 1 deletion .github/workflows/_test_check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ jobs:
version: ${{ matrix.version }}
compose_file: ${{ matrix.compose_file }}
EOF
- uses: ./.github/actions/run-docker
- name: ${{ matrix.version == 'local' && 'Uncached Build' || 'Pull' }} Check
Copy link
Member

Choose a reason for hiding this comment

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

Does the syntax support brackets? It's not immediately obvious to me (or future me) if it's doing (A and B) or C or A and (B or C)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is pretty standard JS syntax afaik (GHA operators), it's the first one. The statements are evaluated left to right.

image

Copy link
Member

Choose a reason for hiding this comment

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

Ah, it's 'Uncached build' if matrix.version == 'local' else 'Pull' in python syntax

uses: ./.github/actions/run-docker
# Set environment variables that are expected to be ignored
env:
DOCKER_COMMIT: 'not-expected'
Expand All @@ -74,6 +75,13 @@ jobs:
version: ${{ matrix.version }}
compose_file: ${{ matrix.compose_file }}
run: make check
- name: Cached Build Check
uses: ./.github/actions/run-docker
if: ${{ matrix.version == 'local' }}
with:
version: ${{ matrix.version }}
compose_file: ${{ matrix.compose_file }}
run: echo true

test_make_docker_configuration:
runs-on: ubuntu-latest
Expand Down
12 changes: 4 additions & 8 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ chown -R olympia:olympia /deps
ln -s /deps/bin/uwsgi /usr/bin/uwsgi
ln -s /usr/bin/uwsgi /usr/sbin/uwsgi

# link to the package*.json at ${HOME} so npm can install in /deps
ln -s ${HOME}/package.json /deps/package.json
ln -s ${HOME}/package-lock.json /deps/package-lock.json

# Create the storage directory and the test file to verify nginx routing
mkdir -p ${HOME}/storage
chown -R olympia:olympia ${HOME}/storage
Expand Down Expand Up @@ -109,8 +105,8 @@ RUN \
# Files required to install pip dependencies
--mount=type=bind,source=./requirements/prod.txt,target=${HOME}/requirements/prod.txt \
# Files required to install npm dependencies
--mount=type=bind,source=package.json,target=${HOME}/package.json \
--mount=type=bind,source=package-lock.json,target=${HOME}/package-lock.json \
--mount=type=bind,source=package.json,target=/deps/package.json \
--mount=type=bind,source=package-lock.json,target=/deps/package-lock.json \
# Mounts for caching dependencies
--mount=type=cache,target=${PIP_CACHE_DIR},uid=${OLYMPIA_UID},gid=${OLYMPIA_UID} \
--mount=type=cache,target=${NPM_CACHE_DIR},uid=${OLYMPIA_UID},gid=${OLYMPIA_UID} \
Expand All @@ -126,8 +122,8 @@ RUN \
--mount=type=bind,source=./requirements/prod.txt,target=${HOME}/requirements/prod.txt \
--mount=type=bind,source=./requirements/dev.txt,target=${HOME}/requirements/dev.txt \
# Files required to install npm dependencies
--mount=type=bind,source=package.json,target=${HOME}/package.json \
--mount=type=bind,source=package-lock.json,target=${HOME}/package-lock.json \
--mount=type=bind,source=package.json,target=/deps/package.json \
--mount=type=bind,source=package-lock.json,target=/deps/package-lock.json \
# Mounts for caching dependencies
--mount=type=cache,target=${PIP_CACHE_DIR},uid=${OLYMPIA_UID},gid=${OLYMPIA_UID} \
--mount=type=cache,target=${NPM_CACHE_DIR},uid=${OLYMPIA_UID},gid=${OLYMPIA_UID} \
Expand Down
1 change: 1 addition & 0 deletions Makefile-docker
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ initialize: ## ensure database exists
@echo "Initializing data..."
@echo "args: $(ARGS)"
$(PYTHON_COMMAND) ./manage.py initialize $(ARGS)
./scripts/healthcheck.py

PYTEST_SRC := src/olympia/

Expand Down
23 changes: 13 additions & 10 deletions Makefile-os
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ ifneq ($(INIT_LOAD),)
INITIALIZE_ARGS += --load $(INIT_LOAD)
endif



DOCKER_BAKE_ARGS := \
--file docker-bake.hcl \
--file .env \
Expand All @@ -43,7 +41,6 @@ DOCKER_COMPOSE_ARGS := \
--remove-orphans \
--no-build \
--quiet-pull \
--renew-anon-volumes

# Paths should be cleaned before mounting .:/data/olympia
# These are files which should be sourced from the container
Expand Down Expand Up @@ -150,17 +147,23 @@ docker_clean_build_cache: ## Remove buildx build cache
.PHONY: clean_docker
clean_docker: docker_compose_down docker_mysqld_volume_remove docker_clean_images docker_clean_volumes docker_clean_build_cache ## Remove all docker resources taking space on the host machine

.PHONY: docker_compose_up
docker_compose_up: docker_mysqld_volume_create ## Start the docker containers
.PHONY: up_pre
up_pre: setup docker_pull_or_build ## Pre-up the environment, setup files, volumes and host state

.PHONY: up_start
up_start: docker_mysqld_volume_create ## Start the docker containers
docker compose up $(DOCKER_COMPOSE_ARGS) $(ARGS)

.PHONY: up
up: setup docker_pull_or_build docker_compose_up docker_clean_images docker_clean_volumes ## Create and start docker compose
# Explicitly run initialize via the web container as make can get confused
# both routing the command to the web container and
# routing the command to the proper target.
.PHONY: up_post
up_post: docker_clean_images docker_clean_volumes ## Post-up the environment, setup files, volumes and host state
# Explicitly run initialize via the web container as make can get confused
# both routing the command to the web container and
# routing the command to the proper target.
docker compose exec --user olympia web make -f Makefile-docker initialize ARGS=$(shell echo "'$(INITIALIZE_ARGS)'")

.PHONY: up
up: up_pre up_start up_post ## Up the environment

.PHONY: down
down: docker_compose_down docker_clean_images docker_clean_volumes ## Stop the docker containers and clean up non-peristent dangling resources

Expand Down
4 changes: 3 additions & 1 deletion docker-compose.ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ services:
- HOST_UID=9500
volumes:
- storage:/data/olympia/storage
- /data/olympia
- data_olympia_production:/data/olympia

worker:
extends:
Expand All @@ -17,9 +17,11 @@ services:

nginx:
volumes:
- data_olympia_production:/srv
- storage:/srv/storage
depends_on:
- olympia_volumes

volumes:
storage:
data_olympia_production:
76 changes: 48 additions & 28 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ services:
command: ["sleep", "infinity"]
volumes:
- *site-static-mount
- data_olympia_development:/data/olympia
worker:
<<: *olympia
command: [
Expand All @@ -63,24 +64,12 @@ services:
"celery -A olympia.amo.celery:app worker -E -c 2 --loglevel=INFO",
]
volumes:
- .:/data/olympia

- data_olympia_development:/data/olympia
extra_hosts:
- "olympia.test:127.0.0.1"
restart: on-failure:5
# entrypoint.sh takes some time
# we can wait for services to be up and running
healthcheck:
test: ["CMD-SHELL", "DJANGO_SETTINGS_MODULE=olympia celery -A olympia.amo.celery status"]
# The interval is 90s after the start period of 60s
interval: 90s
# 3 failed attempts result in container failure
retries: 3
# While starting, ping faster to get the container healthy as soon as possible
start_interval: 1s
# The start period is 60s
start_period: 120s
depends_on:
- olympia_volumes
- mysqld
- elasticsearch
- redis
Expand All @@ -91,29 +80,21 @@ services:
web:
extends:
service: worker
healthcheck:
test: ["CMD-SHELL", "curl --fail --show-error --include --location http://127.0.0.1:8002/__version__"]
retries: 3
interval: 90s
start_interval: 1s
start_period: 120s
command:
- uwsgi --ini /data/olympia/docker/uwsgi.ini
volumes:
# Don't mount generated files. They only exist in the container
# and would otherwiser be deleted by mounting the cwd volume above
- /data/olympia/static-build
- data_static_build:/data/olympia/static-build
- *site-static-mount
- ./package.json:/deps/package.json
- ./package-lock.json:/deps/package-lock.json
depends_on:
- olympia_volumes

nginx:
image: nginx
volumes:
- ./docker/nginx/addons.conf:/etc/nginx/conf.d/addons.conf
- .:/srv
- data_nginx:/etc/nginx/conf.d
- data_olympia_development:/srv
- *site-static-mount
ports:
- "80:80"
Expand All @@ -138,9 +119,22 @@ services:
- "3306:3306"
volumes:
- data_mysqld:/var/lib/mysql
command:
# Optimize for development speed over durability
- --innodb-flush-log-at-trx-commit=0
- --innodb-buffer-pool-size=64M
- --innodb-log-buffer-size=8M
- --innodb-log-file-size=32M
# Skip DNS lookups
- --skip-name-resolve
# Disable performance schema for faster startup
- --performance-schema=OFF
healthcheck:
test: ["CMD", "mysqladmin", "ping", "-h", "localhost", "--silent"]
test: ["CMD-SHELL", "mysql -u root --silent --execute='SELECT 1;'"]
start_interval: 1s
timeout: 2s
start_period: 10s
retries: 3

elasticsearch:
image: docker.elastic.co/elasticsearch/elasticsearch:7.17.3
Expand Down Expand Up @@ -173,9 +167,9 @@ services:
autograph:
image: mozilla/autograph:3.3.2
platform: linux/amd64
command: /go/bin/autograph -c /autograph_localdev_config.yaml
command: /go/bin/autograph -c /data/autograph/autograph_localdev_config.yaml
volumes:
- ./scripts/autograph_localdev_config.yaml:/autograph_localdev_config.yaml
- data_autograph:/data/autograph

addons-frontend:
<<: *env
Expand All @@ -196,13 +190,39 @@ services:

networks:
default:
driver: bridge
enable_ipv6: false

volumes:
# Volumes for static files that should not be
# mounted from the host.
data_static_build:
data_site_static:
# Volume for rabbitmq/redis to avoid anonymous volumes
data_rabbitmq:
data_redis:
data_olympia_development:
driver: local
driver_opts:
type: none
o: bind
device: ${PWD}
data_mysqld:
# Keep this value in sync with Makefile-os
# External volumes must be manually created/destroyed
name: addons-server_data_mysqld
external: true
# Volume for nginx configuration
data_nginx:
driver: local
driver_opts:
type: none
o: bind
device: ${PWD}/docker/nginx
# Volume for autograph configuration
data_autograph:
driver: local
driver_opts:
type: none
o: bind
device: ${PWD}/docker/autograph
50 changes: 50 additions & 0 deletions scripts/healthcheck.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#!/usr/bin/env python3

import os
import subprocess
import sys
import time


env = os.environ.copy()

env['DJANGO_SETTINGS_MODULE'] = 'olympia'


def worker_healthcheck():
subprocess.run(
['celery', '-A', 'olympia.amo.celery', 'status'],
env=env,
stdout=subprocess.DEVNULL,
)


def web_healthcheck():
subprocess.run(
[
'curl',
'--fail',
'--show-error',
'--include',
'--location',
'--silent',
'http://127.0.0.1:8002/__version__',
],
stdout=subprocess.DEVNULL,
)


TIME = time.time()
TIMEOUT = 60
SLEEP = 1

while time.time() - TIME < TIMEOUT:
try:
worker_healthcheck()
web_healthcheck()
print('OK')
sys.exit(0)
except Exception as e:
print(f'Error: {e}')
time.sleep(SLEEP)
SLEEP *= 2
33 changes: 24 additions & 9 deletions tests/make/make.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,7 @@ describe('docker-compose.yml', () => {
expect(service.extra_hosts).toStrictEqual(['olympia.test=127.0.0.1']);
expect(service.restart).toStrictEqual('on-failure:5');
// Each service should have a healthcheck
expect(service.healthcheck.test).not.toBeUndefined();
expect(service.healthcheck.interval).toStrictEqual('1m30s');
expect(service.healthcheck.retries).toStrictEqual(3);
expect(service.healthcheck.start_interval).toStrictEqual('1s');
expect(service.healthcheck.start_period).toStrictEqual('2m0s');
expect(service).not.toHaveProperty('healthcheck');
// each service should have a command
expect(service.command).not.toBeUndefined();
// each service should have the same dependencies
Expand Down Expand Up @@ -124,11 +120,11 @@ describe('docker-compose.yml', () => {
expect(web.volumes).toEqual(
expect.arrayContaining([
expect.objectContaining({
type: 'volume',
source: 'data_static_build',
target: '/data/olympia/static-build',
}),
expect.objectContaining({
type: 'volume',
source: 'data_site_static',
target: '/data/olympia/site-static',
}),
]),
Expand All @@ -154,8 +150,8 @@ describe('docker-compose.yml', () => {
expect.arrayContaining([
// mapping for nginx conf.d adding addon-server routing
expect.objectContaining({
source: expect.any(String),
target: '/etc/nginx/conf.d/addons.conf',
source: 'data_nginx',
target: '/etc/nginx/conf.d',
}),
// mapping for local host directory to /data/olympia
expect.objectContaining({
Expand Down Expand Up @@ -220,6 +216,25 @@ describe('docker-compose.yml', () => {
}
}
});

it('.services.*.volumes does not contain anonymous or unnamed volumes', () => {
const { services } = getConfig({ COMPOSE_FILE });
for (let [name, config] of Object.entries(services)) {
for (let volume of config.volumes ?? []) {
if (volume.bind) {
throw new Error(
`'.services.${name}.volumes' contains anonymous bind mount: ` +
`'${volume.source}:${volume.target}'. Please use a named volume mount instead.`,
);
} else if (!volume.source) {
throw new Error(
`'.services.${name}.volumes' contains unnamed volume mount: ` +
`'${volume.target}'. Please use a named volume mount instead.`,
);
}
}
}
});
});

// these keys require special handling to prevent runtime errors in make setup
Expand Down
Loading