Skip to content

Commit

Permalink
Merge pull request #100 from shanejbrown/main
Browse files Browse the repository at this point in the history
Add no-cache, cache_from and pull MP build params
  • Loading branch information
shanejbrown authored Nov 14, 2023
2 parents 70e9a99 + 2259d68 commit 58f2ddf
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 3 deletions.
25 changes: 23 additions & 2 deletions buildrunner/docker/multiplatform_image_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,9 @@ def _build_with_inject(
file: str,
build_args: dict,
builder: Optional[str],
cache: bool = False,
cache_from: List[str] = None,
pull: bool = False,
) -> None:

if not path or not os.path.isdir(path):
Expand Down Expand Up @@ -279,7 +282,10 @@ def _build_with_inject(
load=True,
file=file,
builder=builder,
build_args=build_args
build_args=build_args,
cache=cache,
cache_from=cache_from,
pull=pull,
)

# pylint: disable=too-many-arguments
Expand All @@ -298,7 +304,10 @@ def _build_single_image(
tags: List[str],
build_args: dict,
mp_image_name: str,
inject: dict) -> None:
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 @@ -330,6 +339,9 @@ def _build_single_image(
file=file,
build_args=build_args,
builder=builder,
cache=cache,
cache_from=cache_from,
pull=pull,
)
else:
docker.buildx.build(
Expand All @@ -340,6 +352,9 @@ def _build_single_image(
file=file,
build_args=build_args,
builder=builder,
cache=cache,
cache_from=cache_from,
pull=pull,
)
# Push after the initial load to support remote builders that cannot access the local registry
docker.push(tagged_names)
Expand Down Expand Up @@ -401,6 +416,9 @@ def build_multiple_images(
do_multiprocessing: bool = True,
build_args: dict = None,
inject: dict = None,
cache: bool = False,
cache_from: List[str] = None,
pull: bool = False,
) -> List[ImageInfo]:
"""
Builds multiple images for the given platforms. One image will be built for each platform.
Expand Down Expand Up @@ -487,6 +505,9 @@ def get_path(file):
build_args,
mp_image_name,
inject,
cache,
cache_from,
pull,
)
logger.debug(f"Building {platform_image_name} for {platform}")
if do_multiprocessing:
Expand Down
5 changes: 4 additions & 1 deletion buildrunner/steprunner/tasks/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ def __init__(
def run(self, context):
# 'import' will override other configuration and perform a 'docker
# import'
if self._import:
if self._import and not self.platforms:
self.step_runner.log.write(f' Importing {self._import} as a Docker image\n')
context['image'] = DockerImporter(
self._import,
Expand Down Expand Up @@ -223,6 +223,9 @@ def run(self, context):
mp_image_name=self.get_unique_build_name(),
build_args=self.buildargs,
inject=self.to_inject,
cache=not self.nocache,
cache_from=self.cache_from,
pull=self.pull,
)

number_of_images = len(self.platforms)
Expand Down
3 changes: 3 additions & 0 deletions buildrunner/validation/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,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.import_param:
raise ValueError(f'import is not allowed in multi-platform build step {step_name}')

# Check for valid push section, duplicate mp tags are not allowed
validate_push(step.push, mp_push_tags, step_name)

Expand Down
3 changes: 3 additions & 0 deletions buildrunner/validation/step.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ class StepBuild(BaseModel, extra='forbid'):
inject: Optional[Dict[str, Optional[str]]] = None
no_cache: Optional[bool] = Field(alias='no-cache', default=None)
buildargs: Optional[Dict[str, Any]] = None
cache_from: Optional[List[str]] = None
# import is a python reserved keyword so we need to alias it
import_param: Optional[str] = Field(alias='import', default=None)


class RunAndServicesBase(BaseModel):
Expand Down
56 changes: 56 additions & 0 deletions tests/test_config_validation/test_validation_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,30 @@ def test_valid_platforms():
assert errors is None


def test_valid_platforms():
config_yaml = """
steps:
build-container-multi-platform:
build:
path: .
dockerfile: Dockerfile
pull: false
platforms:
- linux/amd64
- linux/arm64
no-cache: true
cache_from:
- mytest-reg/buildrunner-test-multi-platform:latest
push:
repository: mytest-reg/buildrunner-test-multi-platform
tags:
- latest
"""
config = yaml.load(config_yaml, Loader=yaml.Loader)
errors = validate_config(**config)
assert errors is None


def test_duplicate_mp_tags_dictionary_invalid():
# Invalid to have duplicate multi-platform tag
config_yaml = """
Expand Down Expand Up @@ -279,6 +303,38 @@ def test_pypi_push():
assert errors is None


def test_invalid_mp_import():
config_yaml = """
steps:
build-container-multi-platform:
build:
path: .
dockerfile: Dockerfile
platforms:
- linux/amd64
- linux/arm64
import: mytest-reg/buildrunner-test-multi-platform:latest
"""
config = yaml.load(config_yaml, Loader=yaml.Loader)
errors = validate_config(**config)
assert isinstance(errors, Errors)
assert errors.count() == 1


def test_valid_import():
config_yaml = """
steps:
build-container-multi-platform:
build:
path: .
dockerfile: Dockerfile
import: mytest-reg/buildrunner-test-multi-platform:latest
"""
config = yaml.load(config_yaml, Loader=yaml.Loader)
errors = validate_config(**config)
assert errors is None


def test_services():
config_yaml = """
steps:
Expand Down
12 changes: 12 additions & 0 deletions tests/test_multiplatform.py
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,9 @@ def test_build_multiple_builds(mock_build, mock_pull, mock_push, mock_inspect, m
file='tests/test-files/multiplatform/Dockerfile',
build_args={'DOCKER_REGISTRY': None},
builder=None,
cache=False,
cache_from=None,
pull=False
),
call(
'tests/test-files/multiplatform',
Expand All @@ -484,6 +487,9 @@ def test_build_multiple_builds(mock_build, mock_pull, mock_push, mock_inspect, m
file='tests/test-files/multiplatform/Dockerfile',
build_args={'DOCKER_REGISTRY': None},
builder=None,
cache=False,
cache_from=None,
pull=False
),
call(
'tests/test-files/multiplatform',
Expand All @@ -493,6 +499,9 @@ def test_build_multiple_builds(mock_build, mock_pull, mock_push, mock_inspect, m
file='tests/test-files/multiplatform/Dockerfile',
build_args={'DOCKER_REGISTRY': None},
builder=None,
cache=False,
cache_from=None,
pull=False
),
call(
'tests/test-files/multiplatform',
Expand All @@ -502,6 +511,9 @@ def test_build_multiple_builds(mock_build, mock_pull, mock_push, mock_inspect, m
file='tests/test-files/multiplatform/Dockerfile',
build_args={'DOCKER_REGISTRY': None},
builder=None,
cache=False,
cache_from=None,
pull=False
),
]
assert mock_push.call_count == 4
Expand Down

0 comments on commit 58f2ddf

Please sign in to comment.