From 094732161781e6474c3ad54dac3d7027a7f63ab9 Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Fri, 18 Aug 2023 11:33:41 -0400 Subject: [PATCH] feat!: assume BuildKit is available BuildKit replaces and improves the legacy Docker builder, which was deprecated back in Feb 2023. Assuming BuildKit allows us to simplify the Dockerfile and makes future build performance improvements easier. The Docker versions which Tutor recommends (v20+) all come with BuildKit, so As follow-up work, we will need to remove `is_buildkit_enabled` from the official plugins templates. Relevant discussion: https://github.com/overhangio/tutor/pull/868#issuecomment-1657961059 --- changelog.d/20230818_112124_kyle_buildkit.md | 2 + tests/commands/test_images.py | 6 +- tutor/commands/images.py | 34 +++++------ tutor/env.py | 2 +- tutor/hooks/catalog.py | 5 +- tutor/images.py | 3 +- tutor/templates/build/openedx/Dockerfile | 64 +++++++++++--------- tutor/utils.py | 19 ------ 8 files changed, 57 insertions(+), 78 deletions(-) create mode 100644 changelog.d/20230818_112124_kyle_buildkit.md diff --git a/changelog.d/20230818_112124_kyle_buildkit.md b/changelog.d/20230818_112124_kyle_buildkit.md new file mode 100644 index 00000000000..7084b290415 --- /dev/null +++ b/changelog.d/20230818_112124_kyle_buildkit.md @@ -0,0 +1,2 @@ +- [Deprecation] The template variable ``is_buildkit_enabled``, which now always returns True, is deprecated. Plugin authors should assume BuildKit is enabled and remove the variable from their templates (by @kdmccormick). +- 💥[Deprecation] Tutor no longer supports the legacy Docker builder, which was previously available by setting ``DOCKER_BUILDKIT=0`` in the host environment. Going forward, Tutor will always use BuildKit (a.k.a. ``docker buildx`` in Docker v19-v22, or just ``docker build`` in Docker v23). This transition will improve build performance and should be seamless for Tutor users who are running a supported Docker version (by @kdmccormick). diff --git a/tests/commands/test_images.py b/tests/commands/test_images.py index 7b0957790d9..61c4ccaeb28 100644 --- a/tests/commands/test_images.py +++ b/tests/commands/test_images.py @@ -128,10 +128,8 @@ def test_images_build_plugin_with_args(self, image_build: Mock) -> None: "service1", ] with temporary_root() as root: - utils.is_buildkit_enabled.cache_clear() - with patch.object(utils, "is_buildkit_enabled", return_value=False): - self.invoke_in_root(root, ["config", "save"]) - result = self.invoke_in_root(root, build_args) + self.invoke_in_root(root, ["config", "save"]) + result = self.invoke_in_root(root, build_args) self.assertIsNone(result.exception) self.assertEqual(0, result.exit_code) image_build.assert_called() diff --git a/tutor/commands/images.py b/tutor/commands/images.py index d21e2801cab..d5469f9263f 100644 --- a/tutor/commands/images.py +++ b/tutor/commands/images.py @@ -156,7 +156,7 @@ def images_command() -> None: # Export image to docker. This is necessary to make the image available to docker-compose. # The `--load` option is a shorthand for `--output=type=docker`. default="type=docker", - help="Same as `docker build --output=...`. This option will only be used when BuildKit is enabled.", + help="Same as `docker build --output=...`.", ) @click.option( "-a", @@ -211,7 +211,7 @@ def build( command_args += ["--add-host", add_host] if target: command_args += ["--target", target] - if utils.is_buildkit_enabled() and docker_output: + if docker_output: command_args.append(f"--output={docker_output}") if docker_args: command_args += docker_args @@ -223,27 +223,21 @@ def build( image_build_args = [*command_args, *custom_args] # Registry cache - if utils.is_buildkit_enabled(): - if not no_registry_cache: - image_build_args.append( - f"--cache-from=type=registry,ref={tag}-cache" - ) - if cache_to_registry: - image_build_args.append( - f"--cache-to=type=registry,mode=max,ref={tag}-cache" - ) + if not no_registry_cache: + image_build_args.append( + f"--cache-from=type=registry,ref={tag}-cache" + ) + if cache_to_registry: + image_build_args.append( + f"--cache-to=type=registry,mode=max,ref={tag}-cache" + ) # Build contexts for host_path, stage_name in build_contexts.get(name, []): - if utils.is_buildkit_enabled(): - fmt.echo_info( - f"Adding {host_path} to the build context '{stage_name}' of image '{image}'" - ) - image_build_args.append(f"--build-context={stage_name}={host_path}") - else: - fmt.echo_alert( - f"Unable to add {host_path} to the build context '{stage_name}' of image '{host_path}' because BuildKit is disabled." - ) + fmt.echo_info( + f"Adding {host_path} to the build context '{stage_name}' of image '{image}'" + ) + image_build_args.append(f"--build-context={stage_name}={host_path}") # Build images.build( diff --git a/tutor/env.py b/tutor/env.py index 980bb773490..3183b1b37fe 100644 --- a/tutor/env.py +++ b/tutor/env.py @@ -54,7 +54,7 @@ def _prepare_environment() -> None: ("HOST_USER_ID", utils.get_user_id()), ("TUTOR_APP", __app__.replace("-", "_")), ("TUTOR_VERSION", __version__), - ("is_buildkit_enabled", utils.is_buildkit_enabled), + ("is_buildkit_enabled", lambda: True), # Will be removed soon. ], ) diff --git a/tutor/hooks/catalog.py b/tutor/hooks/catalog.py index 6d6601ffa70..1f4070f925f 100644 --- a/tutor/hooks/catalog.py +++ b/tutor/hooks/catalog.py @@ -255,8 +255,7 @@ def your_filter_callback(some_data): #: names must be prefixed with the plugin name in all-caps. CONFIG_UNIQUE: Filter[list[tuple[str, Any]], []] = Filter() - #: Use this filter to modify the ``docker build`` command. For instance, to replace - #: the ``build`` subcommand by ``buildx build``. + #: Use this filter to modify the ``docker build`` command. #: #: :parameter list[str] command: the full build command, including options and #: arguments. Note that these arguments do not include the leading ``docker`` command. @@ -339,7 +338,7 @@ def your_filter_callback(some_data): #: - ``HOST_USER_ID``: the numerical ID of the user on the host. #: - ``TUTOR_APP``: the app name ("tutor" by default), used to determine the dev/local project names. #: - ``TUTOR_VERSION``: the current version of Tutor. - #: - ``is_buildkit_enabled``: a boolean function that indicates whether BuildKit is available on the host. + #: - ``is_buildkit_enabled``: a deprecated function which always returns ``True`` now. Will be removed before Quince. #: - ``iter_values_named``: a function to iterate on variables that start or end with a given string. #: - ``iter_mounts``: a function that yields compose-compatible bind-mounts for any given service. #: - ``patch``: a function to incorporate extra content into a template. diff --git a/tutor/images.py b/tutor/images.py index 26f80b1824e..66f2b0b1c96 100644 --- a/tutor/images.py +++ b/tutor/images.py @@ -4,8 +4,7 @@ def build(path: str, tag: str, *args: str) -> None: fmt.echo_info(f"Building image {tag}") build_command = ["build", f"--tag={tag}", *args, path] - if utils.is_buildkit_enabled(): - build_command.insert(0, "buildx") + build_command.insert(0, "buildx") command = hooks.Filters.DOCKER_BUILD_COMMAND.apply(build_command) utils.docker(*command) diff --git a/tutor/templates/build/openedx/Dockerfile b/tutor/templates/build/openedx/Dockerfile index 8df5eb8aeca..e58b1064722 100644 --- a/tutor/templates/build/openedx/Dockerfile +++ b/tutor/templates/build/openedx/Dockerfile @@ -1,11 +1,11 @@ -{% if is_buildkit_enabled() %}# syntax=docker/dockerfile:1.4{% endif %} +# syntax=docker/dockerfile:1.4 ###### Minimal image with base system requirements for most stages FROM docker.io/ubuntu:20.04 as minimal LABEL maintainer="Overhang.io " ENV DEBIAN_FRONTEND=noninteractive -RUN {% if is_buildkit_enabled() %}--mount=type=cache,target=/var/cache/apt,sharing=locked \ - --mount=type=cache,target=/var/lib/apt,sharing=locked{% endif %} \ +RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ + --mount=type=cache,target=/var/lib/apt,sharing=locked \ apt update && \ apt install -y build-essential curl git language-pack-en ENV LC_ALL en_US.UTF-8 @@ -14,8 +14,9 @@ ENV LC_ALL en_US.UTF-8 ###### Install python with pyenv in /opt/pyenv and create virtualenv in /openedx/venv FROM minimal as python # https://github.com/pyenv/pyenv/wiki/Common-build-problems#prerequisites -RUN {% if is_buildkit_enabled() %}--mount=type=cache,target=/var/cache/apt,sharing=locked \ - --mount=type=cache,target=/var/lib/apt,sharing=locked {% endif %}apt update && \ +RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ + --mount=type=cache,target=/var/lib/apt,sharing=locked \ + apt update && \ apt install -y libssl-dev zlib1g-dev libbz2-dev \ libreadline-dev libsqlite3-dev wget curl llvm libncurses5-dev libncursesw5-dev \ xz-utils tk-dev libffi-dev liblzma-dev python-openssl git @@ -77,26 +78,27 @@ ENV PATH /openedx/venv/bin:${PATH} ENV VIRTUAL_ENV /openedx/venv/ ENV XDG_CACHE_HOME /openedx/.cache -RUN {% if is_buildkit_enabled() %}--mount=type=cache,target=/var/cache/apt,sharing=locked \ - --mount=type=cache,target=/var/lib/apt,sharing=locked {% endif %}apt update \ +RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ + --mount=type=cache,target=/var/lib/apt,sharing=locked \ + apt update \ && apt install -y software-properties-common libmysqlclient-dev libxmlsec1-dev libgeos-dev # Install the right version of pip/setuptools -RUN {% if is_buildkit_enabled() %}--mount=type=cache,target=/openedx/.cache/pip,sharing=shared {% endif %}pip install \ +RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ + pip install \ # https://pypi.org/project/setuptools/ # https://pypi.org/project/pip/ # https://pypi.org/project/wheel/ setuptools==67.6.1 pip==23.0.1. wheel==0.40.0 # Install base requirements -{% if not is_buildkit_enabled() %} -COPY --from=edx-platform /requirements/edx/base.txt /openedx/edx-platform/requirements/edx/base.txt -{% endif %} -RUN {% if is_buildkit_enabled() %}--mount=type=bind,from=edx-platform,source=/requirements/edx/base.txt,target=/openedx/edx-platform/requirements/edx/base.txt \ - --mount=type=cache,target=/openedx/.cache/pip,sharing=shared {% endif %}pip install -r /openedx/edx-platform/requirements/edx/base.txt +RUN --mount=type=bind,from=edx-platform,source=/requirements/edx/base.txt,target=/openedx/edx-platform/requirements/edx/base.txt \ + --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ + pip install -r /openedx/edx-platform/requirements/edx/base.txt # Install extra requirements -RUN {% if is_buildkit_enabled() %}--mount=type=cache,target=/openedx/.cache/pip,sharing=shared {% endif %}pip install \ +RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ + pip install \ # Use redis as a django cache https://pypi.org/project/django-redis/ django-redis==5.2.0 \ # uwsgi server https://pypi.org/project/uWSGI/ @@ -106,11 +108,14 @@ RUN {% if is_buildkit_enabled() %}--mount=type=cache,target=/openedx/.cache/pip, # Install private requirements: this is useful for installing custom xblocks. COPY ./requirements/ /openedx/requirements -RUN {% if is_buildkit_enabled() %}--mount=type=cache,target=/openedx/.cache/pip,sharing=shared {% endif %}cd /openedx/requirements/ \ +RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ + cd /openedx/requirements/ \ && touch ./private.txt \ && pip install -r ./private.txt -{% for extra_requirements in OPENEDX_EXTRA_PIP_REQUIREMENTS %}RUN {% if is_buildkit_enabled() %}--mount=type=cache,target=/openedx/.cache/pip,sharing=shared {% endif %}pip install '{{ extra_requirements }}' +{% for extra_requirements in OPENEDX_EXTRA_PIP_REQUIREMENTS %} +RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ + pip install '{{ extra_requirements }}' {% endfor %} ###### Install nodejs with nodeenv in /openedx/nodeenv @@ -126,21 +131,19 @@ RUN nodeenv /openedx/nodeenv --node=16.14.0 --prebuilt # Install nodejs requirements ARG NPM_REGISTRY={{ NPM_REGISTRY }} WORKDIR /openedx/edx-platform -{% if not is_buildkit_enabled() %} -COPY --from=edx-platform /package.json /openedx/edx-platform/package.json -COPY --from=edx-platform /package-lock.json /openedx/edx-platform/package-lock.json -{% endif %} -RUN {% if is_buildkit_enabled() %}--mount=type=bind,from=edx-platform,source=/package.json,target=/openedx/edx-platform/package.json \ +RUN --mount=type=bind,from=edx-platform,source=/package.json,target=/openedx/edx-platform/package.json \ --mount=type=bind,from=edx-platform,source=/package-lock.json,target=/openedx/edx-platform/package-lock.json \ --mount=type=bind,from=edx-platform,source=/scripts/copy-node-modules.sh,target=/openedx/edx-platform/scripts/copy-node-modules.sh \ - --mount=type=cache,target=/root/.npm,sharing=shared {% endif %}npm clean-install --no-audit --registry=$NPM_REGISTRY + --mount=type=cache,target=/root/.npm,sharing=shared \ + npm clean-install --no-audit --registry=$NPM_REGISTRY ###### Production image with system and python requirements FROM minimal as production # Install system requirements -RUN {% if is_buildkit_enabled() %}--mount=type=cache,target=/var/cache/apt,sharing=locked \ - --mount=type=cache,target=/var/lib/apt,sharing=locked {% endif %}apt update \ +RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ + --mount=type=cache,target=/var/lib/apt,sharing=locked \ + apt update \ && apt install -y gettext gfortran graphviz graphviz-dev libffi-dev libfreetype6-dev libgeos-dev libjpeg8-dev liblapack-dev libmysqlclient-dev libpng-dev libsqlite3-dev libxmlsec1-dev lynx mysql-client ntp pkg-config rdfind # From then on, run as unprivileged "app" user @@ -151,7 +154,7 @@ RUN useradd --home-dir /openedx --create-home --shell /bin/bash --uid ${APP_USER USER ${APP_USER_ID} # https://hub.docker.com/r/powerman/dockerize/tags -COPY {% if is_buildkit_enabled() %}--link {% endif %}--from=docker.io/powerman/dockerize:0.19.0 /usr/local/bin/dockerize /usr/local/bin/dockerize +COPY --link --from=docker.io/powerman/dockerize:0.19.0 /usr/local/bin/dockerize /usr/local/bin/dockerize COPY --chown=app:app --from=edx-platform / /openedx/edx-platform COPY --chown=app:app --from=locales /openedx/locale /openedx/locale COPY --chown=app:app --from=python /opt/pyenv /opt/pyenv @@ -245,16 +248,19 @@ FROM production as development # Install useful system requirements (as root) USER root -RUN {% if is_buildkit_enabled() %}--mount=type=cache,target=/var/cache/apt,sharing=locked \ - --mount=type=cache,target=/var/lib/apt,sharing=locked {% endif %}apt update && \ +RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ + --mount=type=cache,target=/var/lib/apt,sharing=locked \ + apt update && \ apt install -y vim iputils-ping dnsutils telnet USER app # Install dev python requirements -RUN {% if is_buildkit_enabled() %}--mount=type=cache,target=/openedx/.cache/pip,sharing=shared {% endif %}pip install -r requirements/edx/development.txt +RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ + pip install -r requirements/edx/development.txt # https://pypi.org/project/ipdb/ # https://pypi.org/project/ipython -RUN {% if is_buildkit_enabled() %}--mount=type=cache,target=/openedx/.cache/pip,sharing=shared {% endif %}pip install ipdb==0.13.13 ipython==8.12.0 +RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \ + pip install ipdb==0.13.13 ipython==8.12.0 # Add ipdb as default PYTHONBREAKPOINT ENV PYTHONBREAKPOINT=ipdb.set_trace diff --git a/tutor/utils.py b/tutor/utils.py index 59adee43778..2d8d3e561e6 100644 --- a/tutor/utils.py +++ b/tutor/utils.py @@ -173,25 +173,6 @@ def docker(*command: str) -> int: return execute("docker", *command) -@lru_cache(maxsize=None) -def is_buildkit_enabled() -> bool: - """ - A helper function to determine whether we can run `docker buildx` with BuildKit. - """ - # First, we respect the DOCKER_BUILDKIT environment variable - enabled_by_env = { - "1": True, - "0": False, - }.get(os.environ.get("DOCKER_BUILDKIT", "")) - if enabled_by_env is not None: - return enabled_by_env - try: - subprocess.run(["docker", "buildx", "version"], capture_output=True, check=True) - return True - except subprocess.CalledProcessError: - return False - - def docker_compose(*command: str) -> int: return execute("docker", "compose", *command)