From b9677d0df400c996d041fd70402035b7793c36b7 Mon Sep 17 00:00:00 2001 From: riisi Date: Mon, 11 Mar 2024 16:23:22 +1100 Subject: [PATCH] Docker: Support multiple `cache_from` values for `docker_image`. (#20600) Fixes #20596 --------- Co-authored-by: Rhys Madigan --- docs/docs/docker/index.mdx | 4 +- .../backend/docker/goals/package_image.py | 2 + .../docker/goals/package_image_test.py | 5 +- .../pants/backend/docker/target_types.py | 48 +++++++++++++++---- src/python/pants/engine/target.py | 33 +++++++++++++ src/python/pants/engine/target_test.py | 29 +++++++++++ 6 files changed, 108 insertions(+), 13 deletions(-) diff --git a/docs/docs/docker/index.mdx b/docs/docs/docker/index.mdx index defe5d82c43..01de998ad64 100644 --- a/docs/docs/docker/index.mdx +++ b/docs/docs/docker/index.mdx @@ -217,10 +217,10 @@ docker_image( "type": "local", "dest": "/tmp/docker-cache/pants-example" }, - cache_from={ + cache_from=[{ "type": "local", "src": "/tmp/docker-cache/pants-example" - } + }] ) ``` diff --git a/src/python/pants/backend/docker/goals/package_image.py b/src/python/pants/backend/docker/goals/package_image.py index 91e1f056c9e..4d4219ab4b4 100644 --- a/src/python/pants/backend/docker/goals/package_image.py +++ b/src/python/pants/backend/docker/goals/package_image.py @@ -18,6 +18,7 @@ from pants.backend.docker.subsystems.docker_options import DockerOptions from pants.backend.docker.target_types import ( DockerBuildKitOptionField, + DockerBuildOptionFieldListOfMultiValueDictMixin, DockerBuildOptionFieldMixin, DockerBuildOptionFieldMultiValueDictMixin, DockerBuildOptionFieldMultiValueMixin, @@ -336,6 +337,7 @@ def get_build_options( ( DockerBuildOptionFieldMixin, DockerBuildOptionFieldMultiValueDictMixin, + DockerBuildOptionFieldListOfMultiValueDictMixin, DockerBuildOptionFieldValueMixin, DockerBuildOptionFieldMultiValueMixin, DockerBuildOptionFlagFieldMixin, diff --git a/src/python/pants/backend/docker/goals/package_image_test.py b/src/python/pants/backend/docker/goals/package_image_test.py index b8f15d6fdc2..679b719c46a 100644 --- a/src/python/pants/backend/docker/goals/package_image_test.py +++ b/src/python/pants/backend/docker/goals/package_image_test.py @@ -1152,7 +1152,7 @@ def test_docker_cache_from_option(rule_runner: RuleRunner) -> None: """\ docker_image( name="img1", - cache_from={"type": "local", "dest": "/tmp/docker/pants-test-cache"}, + cache_from=[{"type": "local", "dest": "/tmp/docker/pants-test-cache1"}, {"type": "local", "dest": "/tmp/docker/pants-test-cache2"}], ) """ ), @@ -1164,7 +1164,8 @@ def check_docker_proc(process: Process): "/dummy/docker", "buildx", "build", - "--cache-from=type=local,dest=/tmp/docker/pants-test-cache", + "--cache-from=type=local,dest=/tmp/docker/pants-test-cache1", + "--cache-from=type=local,dest=/tmp/docker/pants-test-cache2", "--output=type=docker", "--pull=False", "--tag", diff --git a/src/python/pants/backend/docker/target_types.py b/src/python/pants/backend/docker/target_types.py index 276f1458a45..f8ae4869a14 100644 --- a/src/python/pants/backend/docker/target_types.py +++ b/src/python/pants/backend/docker/target_types.py @@ -30,6 +30,7 @@ DictStringToStringField, Field, InvalidFieldException, + ListOfDictStringToStringField, OptionalSingleSourceField, StringField, StringSequenceField, @@ -313,6 +314,23 @@ def options(self, value_formatter: OptionValueFormatter, **kwargs) -> Iterator[s ) +class DockerBuildOptionFieldListOfMultiValueDictMixin(ListOfDictStringToStringField): + """Inherit this mixin class to provide multiple key-value options to docker build: + + `--flag=key1=value1,key2=value2 --flag=key3=value3,key4=value4` + """ + + docker_build_option: ClassVar[str] + + @final + def options(self, value_formatter: OptionValueFormatter, **kwargs) -> Iterator[str]: + if self.value: + for item in self.value: + yield f"{self.docker_build_option}=" + ",".join( + f"{key}={value_formatter(value)}" for key, value in item.items() + ) + + class DockerBuildKitOptionField: """Mixin to indicate a BuildKit-specific option.""" @@ -331,6 +349,10 @@ class DockerImageBuildImageCacheToField( f""" Export image build cache to an external cache destination. + Note that Docker [supports](https://docs.docker.com/build/cache/backends/#multiple-caches) + multiple cache sources - Pants will pass these as multiple `--cache_from` arguments to the + Docker CLI. Docker will only use the first cache hit (i.e. the image exists) in the build. + {DockerBuildKitOptionField.required_help} Example: @@ -341,10 +363,10 @@ class DockerImageBuildImageCacheToField( "type": "local", "dest": "/tmp/docker-cache/example" }}, - cache_from={{ + cache_from=[{{ "type": "local", "src": "/tmp/docker-cache/example" - }} + }}] ) {_interpolation_help.format(kind="Values")} @@ -354,12 +376,14 @@ class DockerImageBuildImageCacheToField( class DockerImageBuildImageCacheFromField( - DockerBuildOptionFieldMultiValueDictMixin, DictStringToStringField, DockerBuildKitOptionField + DockerBuildOptionFieldListOfMultiValueDictMixin, + ListOfDictStringToStringField, + DockerBuildKitOptionField, ): alias = "cache_from" help = help_text( f""" - Use an external cache source when building the image. + Use external cache sources when building the image. {DockerBuildKitOptionField.required_help} @@ -369,12 +393,18 @@ class DockerImageBuildImageCacheFromField( name="example-local-cache-backend", cache_to={{ "type": "local", - "dest": "/tmp/docker-cache/example" + "dest": "/tmp/docker-cache/primary" }}, - cache_from={{ - "type": "local", - "src": "/tmp/docker-cache/example" - }} + cache_from=[ + {{ + "type": "local", + "src": "/tmp/docker-cache/primary" + }}, + {{ + "type": "local", + "src": "/tmp/docker-cache/secondary" + }} + ] ) {_interpolation_help.format(kind="Values")} diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py index 88f495a84d6..8627b18bae9 100644 --- a/src/python/pants/engine/target.py +++ b/src/python/pants/engine/target.py @@ -1918,6 +1918,39 @@ def compute_value( return FrozenDict(value_or_default) +class ListOfDictStringToStringField(Field): + value: Optional[Tuple[FrozenDict[str, str]]] + default: ClassVar[Optional[list[FrozenDict[str, str]]]] = None + + @classmethod + def compute_value( + cls, raw_value: Optional[list[Dict[str, str]]], address: Address + ) -> Optional[Tuple[FrozenDict[str, str], ...]]: + value_or_default = super().compute_value(raw_value, address) + if value_or_default is None: + return None + invalid_type_exception = InvalidFieldTypeException( + address, + cls.alias, + raw_value, + expected_type="a list of dictionaries (or a single dictionary) of string -> string", + ) + + # Also support passing in a single dictionary by wrapping it + if not isinstance(value_or_default, list): + value_or_default = [value_or_default] + + result_lst: list[FrozenDict[str, str]] = [] + for item in value_or_default: + if not isinstance(item, collections.abc.Mapping): + raise invalid_type_exception + if not all(isinstance(k, str) and isinstance(v, str) for k, v in item.items()): + raise invalid_type_exception + result_lst.append(FrozenDict(item)) + + return tuple(result_lst) + + class NestedDictStringToStringField(Field): value: Optional[FrozenDict[str, FrozenDict[str, str]]] default: ClassVar[Optional[FrozenDict[str, FrozenDict[str, str]]]] = None diff --git a/src/python/pants/engine/target_test.py b/src/python/pants/engine/target_test.py index c2334c95147..74cb89d2edd 100644 --- a/src/python/pants/engine/target_test.py +++ b/src/python/pants/engine/target_test.py @@ -31,6 +31,7 @@ InvalidFieldTypeException, InvalidGeneratedTargetException, InvalidTargetException, + ListOfDictStringToStringField, MultipleSourcesField, NestedDictStringToStringField, OptionalSingleSourceField, @@ -870,6 +871,34 @@ class ExampleDefault(DictStringToStringField): assert ExampleDefault(None, addr).value == FrozenDict({"default": "val"}) +def test_list_of_dict_string_to_string_field() -> None: + class Example(ListOfDictStringToStringField): + alias = "example" + + addr = Address("", target_name="example") + + assert Example(None, addr).value is None + assert Example([{}], addr).value == (FrozenDict(),) + assert Example([{"hello": "world"}], addr).value == (FrozenDict({"hello": "world"}),) + # Test support for single dict not passed in a list + assert Example({"hello": "world"}, addr).value == (FrozenDict({"hello": "world"}),) + + def assert_invalid_type(raw_value: Any) -> None: + with pytest.raises(InvalidFieldTypeException): + Example(raw_value, addr) + + for v in [0, [0], [object()], ["hello"], [["hello"]], [{"hello": 0}], [{0: "world"}]]: + assert_invalid_type(v) + + # Test that a default can be set. + class ExampleDefault(ListOfDictStringToStringField): + alias = "example" + # Note that we use `FrozenDict` so that the object can be hashable. + default = [FrozenDict({"default": "val"})] + + assert ExampleDefault(None, addr).value == (FrozenDict({"default": "val"}),) + + def test_nested_dict_string_to_string_field() -> None: class Example(NestedDictStringToStringField): alias = "example"