From 188f25199ef25eb252ebd830275113451bfd72b7 Mon Sep 17 00:00:00 2001 From: Kevin Meinhardt Date: Mon, 26 Feb 2024 10:33:57 +0100 Subject: [PATCH 1/6] local docker build command --- .dockerignore | 4 ++++ .gitignore | 4 ++++ Makefile-os | 26 ++++++++++++++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/.dockerignore b/.dockerignore index 3327bb406839..7d26d08f23c3 100644 --- a/.dockerignore +++ b/.dockerignore @@ -6,3 +6,7 @@ deps/ node_modules/ storage/ logs/* + +# Don't include the docker cache in the build context or you will get memory leaks +docker-cache/ +docker-cache-new/ diff --git a/.gitignore b/.gitignore index 9d17895c5637..8d6b4d34ee5e 100644 --- a/.gitignore +++ b/.gitignore @@ -59,3 +59,7 @@ private/ !docker-compose.private.yml !private/README.md !deps/.keep + +# Local cache directory for docker layer/build cache +docker-cache/ +docker-cache-new/ diff --git a/Makefile-os b/Makefile-os index 88758dc74962..5ccf8d2e5f20 100644 --- a/Makefile-os +++ b/Makefile-os @@ -3,6 +3,13 @@ GID := $(shell id -g) export UID export GID +export DOCKER_BUILDER=container + +TAG := addons-server-test +PLATFORM := linux/amd64 +PROGRESS := auto +DOCKER_CACHE_DIR := docker-cache + .PHONY: help_redirect help_redirect: @$(MAKE) help --no-print-directory @@ -33,6 +40,25 @@ rootshell: ## connect to a running addons-server docker shell with root user create_env_file: echo "UID=${UID}\nGID=${GID}" > .env +.PHONY: create_docker_builder +create_docker_builder: ## Create a custom builder for buildkit to efficiently build local images + docker buildx use $(DOCKER_BUILDER) 2>/dev/null || docker buildx create \ + --name $(DOCKER_BUILDER) \ + --driver=docker-container + +.PHONY: build_docker_image +build_docker_image: create_docker_builder ## Build the docker image + DOCKER_BUILDKIT=1 docker build \ + -t $(TAG) \ + --load \ + --platform $(PLATFORM) \ + --progress=$(PROGRESS) \ + --cache-to=type=local,dest=$(DOCKER_CACHE_DIR)-new \ + --cache-from=type=local,src=$(DOCKER_CACHE_DIR),mode=max \ + --builder=$(DOCKER_BUILDER) . + rm -rf $(DOCKER_CACHE_DIR) + mv $(DOCKER_CACHE_DIR)-new $(DOCKER_CACHE_DIR) + .PHONY: initialize_docker initialize_docker: create_env_file # Run a fresh container from the base image to install deps. Since /deps is From c07ed36508bd8a1864605468d82633acf72bfdc5 Mon Sep 17 00:00:00 2001 From: Kevin Meinhardt Date: Mon, 26 Feb 2024 10:10:52 +0100 Subject: [PATCH 2/6] Move copy . to after installing packages (prevents always re-installing) --- Dockerfile | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index 7e12814f44ce..29ef674e3585 100644 --- a/Dockerfile +++ b/Dockerfile @@ -63,15 +63,16 @@ ENV HOME /data/olympia # The pipeline v2 standard requires the existence of /app/version.json # inside the docker image, thus it's copied there. COPY version.json /app/version.json -COPY --chown=olympia:olympia . ${HOME} WORKDIR ${HOME} +# give olympia access to the HOME directory +RUN chown -R olympia:olympia ${HOME} # Set up directories and links that we'll need later, before switching to the # olympia user. RUN mkdir /deps \ - && chown olympia:olympia /deps \ + && chown -R olympia:olympia /deps \ && rm -rf ${HOME}/src/olympia.egg-info \ - && mkdir ${HOME}/src/olympia.egg-info \ + && mkdir -p ${HOME}/src/olympia.egg-info \ && chown olympia:olympia ${HOME}/src/olympia.egg-info \ # For backwards-compatibility purposes, set up links to uwsgi. Note that # the target doesn't exist yet at this point, but it will later. @@ -92,7 +93,10 @@ RUN ln -s ${HOME}/package.json /deps/package.json \ && ln -s ${HOME}/package-lock.json /deps/package-lock.json \ && make update_deps_prod +# Only copy our source files after we have installed all dependencies +# TODO: split this into a separate stage to make even blazingly faster WORKDIR ${HOME} +COPY --chown=olympia:olympia . ${HOME} # Build locales, assets, build id. RUN echo "from olympia.lib.settings_base import *\n" \ From 1011cdf0a8da801f1ce86254a93e135aa3e29233 Mon Sep 17 00:00:00 2001 From: Kevin Meinhardt Date: Mon, 26 Feb 2024 10:49:27 +0100 Subject: [PATCH 3/6] Enable buildkit cache mount to cache pip/npm dependencies - cache npm/pip dependencies across builds - add clear logging to npm install for cache hit/miss - move copy npm files to update_assets --- Dockerfile | 19 ++++++++++++++++++- Makefile-docker | 22 +++++++++++++++------- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/Dockerfile b/Dockerfile index 29ef674e3585..f955058f7d0d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -89,7 +89,24 @@ ENV PIP_SRC=/deps/src/ ENV PYTHONUSERBASE=/deps ENV PATH $PYTHONUSERBASE/bin:$PATH ENV NPM_CONFIG_PREFIX=/deps/ -RUN ln -s ${HOME}/package.json /deps/package.json \ +ENV NPM_CACHE_DIR=/deps/cache/npm +ENV NPM_DEBUG=true + +RUN \ + # Files needed to run the make command + --mount=type=bind,source=Makefile,target=${HOME}/Makefile \ + --mount=type=bind,source=Makefile-docker,target=${HOME}/Makefile-docker \ + # Files required to install pip dependencies + --mount=type=bind,source=setup.py,target=${HOME}/setup.py \ + --mount=type=bind,source=./requirements,target=${HOME}/requirements \ + # 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 \ + # Mounts for caching dependencies + --mount=type=cache,target=${PIP_CACHE_DIR},uid=${OLYMPIA_UID},gid=${OLYMPIA_GID} \ + --mount=type=cache,target=${NPM_CACHE_DIR},uid=${OLYMPIA_UID},gid=${OLYMPIA_GID} \ + # Command to install dependencies + ln -s ${HOME}/package.json /deps/package.json \ && ln -s ${HOME}/package-lock.json /deps/package-lock.json \ && make update_deps_prod diff --git a/Makefile-docker b/Makefile-docker index e44053eb46ad..596aef4445b3 100644 --- a/Makefile-docker +++ b/Makefile-docker @@ -17,6 +17,14 @@ ifneq ($(NPM_CONFIG_PREFIX),) NPM_ARGS := --prefix $(NPM_CONFIG_PREFIX) endif +ifneq ($(NPM_CACHE_DIR),) + NPM_ARGS := $(NPM_ARGS) --cache $(NPM_CACHE_DIR) +endif + +ifneq ($(NPM_DEBUG),) + NPM_ARGS := $(NPM_ARGS) --loglevel verbose +endif + NODE_MODULES := $(NPM_CONFIG_PREFIX)node_modules/ STATIC_CSS := static/css/node_lib/ STATIC_JS := static/js/node_lib/ @@ -75,7 +83,6 @@ populate_data: ## populate a new database # Now that addons have been generated, reindex. $(PYTHON_COMMAND) manage.py reindex --force --noinput -.PHONY: update_deps_base update_deps_base: ## update the python and node dependencies # Work arounds "Multiple .dist-info directories" issue. rm -rf /deps/build/* @@ -84,18 +91,14 @@ update_deps_base: ## update the python and node dependencies # pep 517 mode (the default) breaks editable install in our project. https://github.com/mozilla/addons-server/issues/16144 $(PIP_COMMAND) install --no-use-pep517 -e . - npm install $(NPM_ARGS) - for dest in $(NODE_LIBS_CSS) ; do cp $(NODE_MODULES)$$dest $(STATIC_CSS) ; done - for dest in $(NODE_LIBS_JS) ; do cp $(NODE_MODULES)$$dest $(STATIC_JS) ; done - for dest in $(NODE_LIBS_JQUERY_UI) ; do cp $(NODE_MODULES)$$dest $(STATIC_JQUERY_UI) ; done - .PHONY: update_deps update_deps: update_deps_base ## update the python and node dependencies for development $(PIP_COMMAND) install --progress-bar=off --no-deps --exists-action=w -r requirements/dev.txt + npm install $(NPM_ARGS) .PHONY: update_deps_prod update_deps_prod: update_deps_base ## update the python and node dependencies for production - npm prune --omit=dev + npm ci $(NPM_ARGS) .PHONY: update_db update_db: ## run the database migrations @@ -103,6 +106,11 @@ update_db: ## run the database migrations .PHONY: update_assets update_assets: + # Copy files required in compress_assets to the static folder + mkdir -p $(STATIC_CSS) $(STATIC_JS) $(STATIC_JQUERY_UI) + for dest in $(NODE_LIBS_CSS) ; do cp $(NODE_MODULES)$$dest $(STATIC_CSS) ; done + for dest in $(NODE_LIBS_JS) ; do cp $(NODE_MODULES)$$dest $(STATIC_JS) ; done + for dest in $(NODE_LIBS_JQUERY_UI) ; do cp $(NODE_MODULES)$$dest $(STATIC_JQUERY_UI) ; done # If changing this here, make sure to adapt tests in amo/test_commands.py $(PYTHON_COMMAND) manage.py compress_assets $(PYTHON_COMMAND) manage.py collectstatic --noinput From 31f72512218b87876975d9dc87f126cd3ca85207 Mon Sep 17 00:00:00 2001 From: Kevin Meinhardt Date: Mon, 26 Feb 2024 12:50:36 +0100 Subject: [PATCH 4/6] Split build into stages - splitting to stages offers better caching of layers and more efficient use of disk/time - The initial gains will be with better caching of locale compilation, but will expand as we can move more logic from the final stage TODO: split up apt depedencies to specific stages, move update_assets to pre-final stage to prevent re-running on every build with a * file change. --- Dockerfile | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index f955058f7d0d..0537290e79cb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM python:3.10-slim-buster +FROM python:3.10-slim-buster as base # Should change it to use ARG instead of ENV for OLYMPIA_UID/OLYMPIA_GID # once the jenkins server is upgraded to support docker >= v1.9.0 @@ -110,17 +110,30 @@ RUN \ && ln -s ${HOME}/package-lock.json /deps/package-lock.json \ && make update_deps_prod +FROM base as builder +ARG LOCALE_DIR=${HOME}/locale +# Compile locales +# Copy the locale files from the host so it is writable by the olympia user +COPY --chown=olympia:olympia locale ${LOCALE_DIR} +# Copy the executable individually to improve the cache validity +RUN --mount=type=bind,source=locale/compile-mo.sh,target=${HOME}/compile-mo.sh \ + ${HOME}/compile-mo.sh ${LOCALE_DIR} + +FROM base as final # Only copy our source files after we have installed all dependencies # TODO: split this into a separate stage to make even blazingly faster WORKDIR ${HOME} +# Copy compiled locales from builder +COPY --from=builder --chown=olympia:olympia ${HOME}/locale ${HOME}/locale +# Copy the rest of the source files from the host COPY --chown=olympia:olympia . ${HOME} -# Build locales, assets, build id. -RUN echo "from olympia.lib.settings_base import *\n" \ -> settings_local.py && DJANGO_SETTINGS_MODULE='settings_local' locale/compile-mo.sh locale \ - && DJANGO_SETTINGS_MODULE='settings_local' python manage.py compress_assets \ - && DJANGO_SETTINGS_MODULE='settings_local' python manage.py generate_jsi18n_files \ - && DJANGO_SETTINGS_MODULE='settings_local' python manage.py collectstatic --noinput \ +# Finalize the build +# TODO: We should move update_assets to the `builder` stage once we can efficiently +# Run that command without having to copy the whole source code +# This will shave nearly 1 minute off the best case build time +RUN echo "from olympia.lib.settings_base import *" > settings_local.py \ + && DJANGO_SETTINGS_MODULE="settings_local" make update_assets \ && npm prune --production \ && ./scripts/generate_build.py > build.py \ && rm -f settings_local.py settings_local.pyc From b5e7153af5a48bfc3cd9e78cea940947ab19468f Mon Sep 17 00:00:00 2001 From: Kevin Meinhardt Date: Tue, 27 Feb 2024 10:46:40 +0100 Subject: [PATCH 5/6] Document the docker file and build process --- Dockerfile | 4 +++ docs/topics/development/docker.md | 56 +++++++++++++++++++++++++++++++ docs/topics/development/index.rst | 1 + 3 files changed, 61 insertions(+) create mode 100644 docs/topics/development/docker.md diff --git a/Dockerfile b/Dockerfile index 0537290e79cb..52a88e14dc32 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,3 +1,7 @@ +##### Important information for maintaining this Dockerfile ######################################## +# Read the docs/topics/development/docker.md file for more information about this Dockerfile. +#################################################################################################### + FROM python:3.10-slim-buster as base # Should change it to use ARG instead of ENV for OLYMPIA_UID/OLYMPIA_GID diff --git a/docs/topics/development/docker.md b/docs/topics/development/docker.md new file mode 100644 index 000000000000..1c4a4dde02d9 --- /dev/null +++ b/docs/topics/development/docker.md @@ -0,0 +1,56 @@ +# Docker + +## The Dockerfile + +Our Dockerfile is used in both production and development environments, however it is not always and not entirely used in CI (for now at least). + +The Dockerfile builds addons-server and runs using docker-compose by specifying the latest image pushed to dockerhub. Keep in mind during local development you are likely not running the current image in your git repository but the latest push to master in github. + +### Best Practices for the Dockerfile + +- Use as few instructions as possible +- Split long running tasks into distinct stages to improve caching/concurrency +- Prefer --mount=type=bind over COPY for files that are needed for a single command + + > bind mounts files as root/docker user, so run the stage from base and chown them to olympia. + > bind mounts do not persist data, so if you modify any files, they will **not** be in the final layer. + +- If you do use COPY for files that are executed, prefer copying individual files over directories. + + > The larger the directory, the more likely it is to have false cache hits. + > Link: + +- Use --mount=type=cache for caching caches npm/pip etc. + + > cache mounts are not persisted in CI due to an existing bug in buildkit. Link: + +- Delay copying source files until the end of the Dockerfile to imrove cache validity + +## Building locally + +To build the Dockerfile locally, run the following command: + +```bash +make build_docker_image +``` + +This will build the Dockerfile locally with buildkit and tag it as `addons-server-test` by default. You can control several parameters including the tag and platform. This can be very useful if you are testing a new image or want to test a new platform. + +We utilize buildkit layer and mount caching to build extremely efficiently. There are more improvements we can make. + +## Clearing cache + +Because we use a custom builder to take full advantage of buildkit mount caching clearing your cache means clearing +the specific builder cache we use, not the docker cache. + +Do: + +```bash +docker builder prune +``` + +Don't do: + +```bash +docker system prune +``` diff --git a/docs/topics/development/index.rst b/docs/topics/development/index.rst index c67c7e70726e..d3a427c06ac4 100644 --- a/docs/topics/development/index.rst +++ b/docs/topics/development/index.rst @@ -8,6 +8,7 @@ Development tests debugging dependencies + docker error_pages testing style From affb73ba8ccc147ba23c96097121e6a6392c7e38 Mon Sep 17 00:00:00 2001 From: Kevin Meinhardt Date: Fri, 1 Mar 2024 11:23:36 +0100 Subject: [PATCH 6/6] update docs --- docs/topics/development/dependencies.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/topics/development/dependencies.md b/docs/topics/development/dependencies.md index e6b0c07fc83f..38378691d5c9 100644 --- a/docs/topics/development/dependencies.md +++ b/docs/topics/development/dependencies.md @@ -61,3 +61,9 @@ make -f Makefile-docker update_deps ``` This is used in github actions for example that do not need a full container to run. + +> Note: If you are adding a new dependency, make sure to update static assets imported from the new versions. + +```bash +make update_assets +```