Skip to content

Commit

Permalink
Add cache_from and cache_to support for multiplatform builds
Browse files Browse the repository at this point in the history
Remove support for older cache_from for multiplatform builds
  • Loading branch information
saville committed Dec 13, 2023
1 parent 86aa8a0 commit 0047f3d
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 14 deletions.
21 changes: 19 additions & 2 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ they are used when put into the global configuration file:
some.other.file.alias: |
The contents of the file...
# The 'caches-root' global configuration specifies the directory to use for
# build caches. The default directory is ~/.buildrunner/caches.
# Specifies the directory to use for the build caches, the default directory
# is ~/.buildrunner/caches
caches-root: ~/.buildrunner/caches
# Change the default docker registry, see the FAQ below for more information
Expand All @@ -196,6 +196,22 @@ they are used when put into the global configuration file:
platform-builders:
platform1: builder1
# Configures caching *for multi-platform builds only*
docker-build-cache:
# An optional list of builders to apply caching options to
# NOTE: These caching options do not work for the default (docker) buildx driver,
# so be careful which builders they are configured for as this could cause
# build failures
builders:
- builder1
# See https://docs.docker.com/build/cache/backends/ for information on how to
# configure the caching backend. These may be strings or dictionaries (both are
# shown below).
to: type=local,dest=/mnt/docker-cache
from:
type: local
src: /mnt/docker-cache
Configuration Locations
-----------------------

Expand Down Expand Up @@ -422,6 +438,7 @@ shows the different configuration options available:
# Buildrunner will attempt to pull these images from the remote registry.
# If the pull is unsuccessful, buildrunner will still pass in the image name
# into --cache-from, allowing a cache check in the host machine cache
# NOTE: Does not work for multi-platform builds
cache_from:
- my-images/image:PR-123
- my-images/image:latest
Expand Down
3 changes: 3 additions & 0 deletions buildrunner/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,9 @@ def run(self): # pylint: disable=too-many-statements,too-many-branches,too-many
temp_dir=self.global_config.get_temp_dir(),
disable_multi_platform=self.disable_multi_platform,
platform_builders=self.global_config.get('platform-builders'),
cache_builders=self.global_config.get_docker_build_cache_config().get('builders'),
cache_from=self.global_config.get_docker_build_cache_config().get('from'),
cache_to=self.global_config.get_docker_build_cache_config().get('to'),
) as multi_platform:
if not os.path.exists(self.build_results_dir):
# create a new results dir
Expand Down
4 changes: 4 additions & 0 deletions buildrunner/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import re
import sys
import tempfile
from typing import Optional

import jinja2

Expand Down Expand Up @@ -396,6 +397,9 @@ def get_docker_registry(self):
"""
return self.global_config.get('docker-registry', 'docker.io')

def get_docker_build_cache_config(self) -> Optional[dict]:
return self.global_config.get('docker-build-cache', {})

def to_abs_path(self, path, return_list=False):
"""
Convert a path to an absolute path (if it isn't one already).
Expand Down
42 changes: 32 additions & 10 deletions buildrunner/docker/multiplatform_image_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import shutil
import tempfile
import uuid
from typing import Dict, List, Optional
from typing import Dict, List, Optional, Union

import python_on_whales
import timeout_decorator
Expand Down Expand Up @@ -130,6 +130,9 @@ def __init__(
temp_dir: str = os.getcwd(),
disable_multi_platform: bool = False,
platform_builders: Optional[Dict[str, str]] = None,
cache_builders: Optional[List[str]] = None,
cache_from: Optional[Union[dict, str]] = None,
cache_to: Optional[Union[dict, str]] = None,
):
self._docker_registry = docker_registry
self._mp_registry_info = None
Expand All @@ -138,6 +141,14 @@ def __init__(
self._temp_dir = temp_dir
self._disable_multi_platform = disable_multi_platform
self._platform_builders = platform_builders
self._cache_builders = set(cache_builders if cache_builders else [])
self._cache_from = cache_from
self._cache_to = cache_to
if self._cache_from or self._cache_to:
print(
f'Configuring multiplatform builds to cache from {cache_from} and to {cache_to} '
f'for builders {", ".join(cache_builders) if cache_builders else "(all)"}'
)

# key is destination image name, value is list of built images
self._intermediate_built_images = {}
Expand Down Expand Up @@ -182,7 +193,7 @@ def registry_port(self) -> int:
return self._mp_registry_info.port

@property
def tagged_images_names(self) -> List[str]:
def tagged_images_names(self) -> Dict[str, List[str]]:
"""Returns a list of all the tagged images names"""
return self._tagged_images_names

Expand Down Expand Up @@ -237,6 +248,19 @@ def _stop_local_registry(self):
else:
logger.warning("Local registry is not running when attempting to stop it")

def _get_build_cache_options(self, builder: Optional[str]) -> dict:
cache_options = {
'cache_from': self._cache_from,
'cache_to': self._cache_to,
}
# If there are no configured cache builders, always return the cache options
if not self._cache_builders:
return cache_options

# If there are cache builders configured, make sure the current builder is in it
actual_builder = builder or 'default'
return cache_options if actual_builder in self._cache_builders else {}

# pylint: disable=too-many-arguments,too-many-locals
def _build_with_inject(
self,
Expand All @@ -248,7 +272,6 @@ def _build_with_inject(
build_args: dict,
builder: Optional[str],
cache: bool = False,
cache_from: List[str] = None,
pull: bool = False,
) -> None:

Expand Down Expand Up @@ -287,8 +310,8 @@ def _build_with_inject(
builder=builder,
build_args=build_args,
cache=cache,
cache_from=cache_from,
pull=pull,
**self._get_build_cache_options(builder),
)

# pylint: disable=too-many-arguments
Expand All @@ -309,7 +332,6 @@ def _build_single_image(
mp_image_name: str,
inject: dict,
cache: bool = False,
cache_from: List[str] = None,
pull: bool = False,) -> None:
"""
Builds a single image for the given platform
Expand Down Expand Up @@ -343,7 +365,6 @@ def _build_single_image(
build_args=build_args,
builder=builder,
cache=cache,
cache_from=cache_from,
pull=pull,
)
else:
Expand All @@ -356,8 +377,8 @@ def _build_single_image(
build_args=build_args,
builder=builder,
cache=cache,
cache_from=cache_from,
pull=pull,
**self._get_build_cache_options(builder),
)
# Push after the initial load to support remote builders that cannot access the local registry
docker.push(tagged_names)
Expand Down Expand Up @@ -420,9 +441,8 @@ def build_multiple_images(
build_args: dict = None,
inject: dict = None,
cache: bool = False,
cache_from: List[str] = None,
pull: bool = False,
) -> List[ImageInfo]:
) -> List[ImageInfo]:
"""
Builds multiple images for the given platforms. One image will be built for each platform.
Expand All @@ -435,6 +455,9 @@ def build_multiple_images(
do_multiprocessing (bool, optional): Whether to use multiprocessing to build the images. Defaults to True.
build_args (dict, optional): The build args to pass to docker. Defaults to None.
inject (dict, optional): The files to inject into the build context. Defaults to None.
cache (bool, optional): If true, enables cache, defaults to False
pull (bool, optional): If true, pulls image before build, defaults to False
inject (dict, optional): The files to inject into the build context. Defaults to None.
Returns:
List[ImageInfo]: The list of intermediate built images, these images are ephemeral
Expand Down Expand Up @@ -509,7 +532,6 @@ def get_path(file):
mp_image_name,
inject,
cache,
cache_from,
pull,
)
logger.debug(f"Building {platform_image_name} for {platform}")
Expand Down
1 change: 0 additions & 1 deletion buildrunner/steprunner/tasks/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ def run(self, context):
build_args=self.buildargs,
inject=self.to_inject,
cache=not self.nocache,
cache_from=self.cache_from,
pull=self.pull,
)

Expand Down
9 changes: 9 additions & 0 deletions buildrunner/validation/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ class SSHKey(BaseModel, extra='forbid'):
prompt_password: Optional[bool] = Field(alias='prompt-password', default=None)
aliases: Optional[List[str]] = None

class DockerBuildCacheConfig(BaseModel, extra='forbid'):
builders: Optional[List[str]] = None
from_config: Optional[Union[dict, str]] = Field(None, alias='from')
to_config: Optional[Union[dict, str]] = Field(None, alias='to')

version: Optional[float] = None
steps: Optional[Dict[str, Step]] = None

Expand All @@ -44,6 +49,7 @@ class SSHKey(BaseModel, extra='forbid'):
# Intentionally has loose restrictions on ssh-keys since documentation isn't clear
ssh_keys: Optional[Union[SSHKey, List[SSHKey]]] = Field(alias='ssh-keys', default=None)
local_files: Optional[Dict[str, str]] = Field(alias='local-files', default=None)
docker_build_cache: Optional[DockerBuildCacheConfig] = Field(None, alias='docker-build-cache')
caches_root: Optional[str] = Field(alias='caches-root', default=None)
docker_registry: Optional[str] = Field(alias='docker-registry', default=None)
temp_dir: Optional[str] = Field(alias='temp-dir', default=None)
Expand Down Expand Up @@ -124,6 +130,9 @@ def validate_multi_platform_build(mp_push_tags: Set[str]):
if not isinstance(step.build.platforms, list):
raise ValueError(f'platforms must be a list in build step {step_name}')

if step.build.cache_from:
raise ValueError(f'cache_from is not allowed in multi-platform build step {step_name}')

if step.build.import_param:
raise ValueError(f'import is not allowed in multi-platform build step {step_name}')

Expand Down
2 changes: 1 addition & 1 deletion tests/test_buildrunner_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from tests import test_runner

test_dir_path = os.path.realpath(os.path.dirname(__file__))
TEST_DIR = os.path.basename(os.path.dirname(__file__))
TEST_DIR = os.path.dirname(__file__)
top_dir_path = os.path.realpath(os.path.dirname(test_dir_path))


Expand Down
15 changes: 15 additions & 0 deletions tests/test_multiplatform.py
Original file line number Diff line number Diff line change
Expand Up @@ -567,3 +567,18 @@ def test_no_images_built():
with MultiplatformImageBuilder() as mp:
image = mp._find_native_platform_images('bogus-image-name')
assert image is None


@pytest.mark.parametrize('builder, cache_builders, return_cache_options', [
('b1', None, True),
('b1', [], True),
('b1', ['b1'], True),
('b2', ['b1'], False),
])
def test__get_build_cache_options(builder, cache_builders, return_cache_options):
multi_platform = MultiplatformImageBuilder(
cache_to='to-loc', cache_from='from-loc', cache_builders=cache_builders,
)
assert multi_platform._get_build_cache_options(builder) == (
{'cache_to': 'to-loc', 'cache_from': 'from-loc'} if return_cache_options else {}
)

0 comments on commit 0047f3d

Please sign in to comment.