Skip to content

Commit

Permalink
Add docker image output field to support publish to repository when u…
Browse files Browse the repository at this point in the history
…sing BuildKit (#20154)

Currently, the `publish` goal doesn't work with docker images when
buildkit is enabled, as by [default buildkit doesn't save the build
output locally](docker/buildx#166), and
`publish` expects that the images were saved.

This PR adds support for setting the output type, and defaults it
to`docker`, which is the legacy docker build behavior, i.e. saves to the
local image store.

However, we only want to set that when buildkit is enabled. I thought it
better to add an explicit option for that at the subsystem level; this
allows for validation of buildkit-only options.

This eliminates the need to set `DOCKER_BUILDKIT=1` in env vars - I need
to update the docs on that actually.

I have validated that with this change, docker images can be published
to a registry.

---------

Co-authored-by: Rhys Madigan <rhys.madigan@accenture.com>
  • Loading branch information
riisi and Rhys Madigan authored Nov 13, 2023
1 parent 941ce5b commit c621a71
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 37 deletions.
12 changes: 3 additions & 9 deletions docs/markdown/Docker/docker.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,6 @@ very-secret-value

BuildKit supports exporting build cache to an external location, making it possible to import in future builds. Cache backends can be configured using the [`cache_to`](doc:reference-docker_image#codecache_tocode) and [`cache_from`](doc:reference-docker_image#codecache_fromcode) fields.

Enable BuildKit if necessary (it is the default in later versions of Docker):

```
❯ export DOCKER_BUILDKIT=1
```

Create a builder using a [build driver](https://docs.docker.com/build/drivers/) that is compatible with the cache backend:

```
Expand All @@ -205,17 +199,17 @@ Use the builder:
Optionally, validate a build with the Docker CLI directly:

```
❯ docker build -t pants-cache-test:latest \
❯ docker buildx build -t pants-cache-test:latest \
--cache-to type=local,dest=/tmp/docker/pants-test-cache \
--cache-from type=local,src=/tmp/docker/pants-test-cache .
```

Configure Pants:
Configure Pants to use buildx and pass the BUILDX_BUILDER environment variable:

```toml pants.toml
[docker]
use_buildx = true
env_vars = [
"DOCKER_BUILDKIT",
"BUILDX_BUILDER"
]
```
Expand Down
47 changes: 26 additions & 21 deletions src/python/pants/backend/docker/goals/package_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
from pants.backend.docker.registries import DockerRegistries, DockerRegistryOptions
from pants.backend.docker.subsystems.docker_options import DockerOptions
from pants.backend.docker.target_types import (
DockerBuildKitOptionField,
DockerBuildOptionFieldMixin,
DockerBuildOptionFieldMultiValueDictMixin,
DockerBuildOptionFieldMultiValueMixin,
DockerBuildOptionFieldValueMixin,
DockerBuildOptionFlagFieldMixin,
DockerImageBuildImageCacheFromField,
DockerImageBuildImageCacheToField,
DockerImageContextRootField,
DockerImageRegistriesField,
DockerImageRepositoryField,
Expand Down Expand Up @@ -316,28 +316,31 @@ def get_build_options(
global_target_stage_option: str | None,
global_build_hosts_options: dict | None,
global_build_no_cache_option: bool | None,
use_buildx_option: bool,
target: Target,
) -> Iterator[str]:
# Build options from target fields inheriting from DockerBuildOptionFieldMixin
for field_type in target.field_types:
if issubclass(field_type, DockerBuildOptionFieldMixin):
source = InterpolationContext.TextSource(
address=target.address, target_alias=target.alias, field_alias=field_type.alias
)
format = partial(
context.interpolation_context.format,
source=source,
error_cls=DockerImageOptionValueError,
)
yield from target[field_type].options(format, global_build_hosts_options)
elif issubclass(field_type, DockerBuildOptionFieldValueMixin):
yield from target[field_type].options()
elif issubclass(field_type, DockerBuildOptionFieldMultiValueMixin):
yield from target[field_type].options()
elif issubclass(field_type, DockerBuildOptionFlagFieldMixin):
yield from target[field_type].options()
elif issubclass(field_type, DockerImageBuildImageCacheToField) or issubclass(
field_type, DockerImageBuildImageCacheFromField
if issubclass(field_type, DockerBuildKitOptionField):
if use_buildx_option is not True:
if target[field_type].value != target[field_type].default:
raise DockerImageOptionValueError(
f"The {target[field_type].alias} field on the = `{target.alias}` target in `{target.address}` was set to `{target[field_type].value}`"
f" and buildx is not enabled. Buildx must be enabled via the Docker subsystem options in order to use this field."
)
else:
# Case where BuildKit option has a default value - still should not be generated
continue

if issubclass(
field_type,
(
DockerBuildOptionFieldMixin,
DockerBuildOptionFieldMultiValueDictMixin,
DockerBuildOptionFieldValueMixin,
DockerBuildOptionFieldMultiValueMixin,
DockerBuildOptionFlagFieldMixin,
),
):
source = InterpolationContext.TextSource(
address=target.address, target_alias=target.alias, field_alias=field_type.alias
Expand All @@ -347,7 +350,7 @@ def get_build_options(
source=source,
error_cls=DockerImageOptionValueError,
)
yield from target[field_type].options(format)
yield from target[field_type].options(format, global_build_hosts_options=global_build_hosts_options) # type: ignore[attr-defined]

# Target stage
target_stage = None
Expand Down Expand Up @@ -440,13 +443,15 @@ async def build_docker_image(
context_root=context_root,
env=env,
tags=tags,
use_buildx=options.use_buildx,
extra_args=tuple(
get_build_options(
context=context,
field_set=field_set,
global_target_stage_option=options.build_target_stage,
global_build_hosts_options=options.build_hosts,
global_build_no_cache_option=options.build_no_cache,
use_buildx_option=options.use_buildx,
target=wrapped_target.target,
)
),
Expand Down
72 changes: 72 additions & 0 deletions src/python/pants/backend/docker/goals/package_image_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from pants.backend.docker.goals.package_image import (
DockerBuildTargetStageError,
DockerImageOptionValueError,
DockerImageTagValueError,
DockerInfoV1,
DockerPackageFieldSet,
Expand Down Expand Up @@ -171,6 +172,7 @@ def mock_get_info_file(request: CreateDigest) -> Digest:
opts.setdefault("build_hosts", None)
opts.setdefault("build_verbose", False)
opts.setdefault("build_no_cache", False)
opts.setdefault("use_buildx", False)
opts.setdefault("env_vars", [])

docker_options = create_subsystem(
Expand Down Expand Up @@ -1113,8 +1115,10 @@ def test_docker_cache_to_option(rule_runner: RuleRunner) -> None:
def check_docker_proc(process: Process):
assert process.argv == (
"/dummy/docker",
"buildx",
"build",
"--cache-to=type=local,dest=/tmp/docker/pants-test-cache",
"--output=type=docker",
"--pull=False",
"--tag",
"img1:latest",
Expand All @@ -1127,6 +1131,7 @@ def check_docker_proc(process: Process):
rule_runner,
Address("docker/test", target_name="img1"),
process_assertions=check_docker_proc,
options=dict(use_buildx=True),
)


Expand All @@ -1147,8 +1152,10 @@ def test_docker_cache_from_option(rule_runner: RuleRunner) -> None:
def check_docker_proc(process: Process):
assert process.argv == (
"/dummy/docker",
"buildx",
"build",
"--cache-from=type=local,dest=/tmp/docker/pants-test-cache",
"--output=type=docker",
"--pull=False",
"--tag",
"img1:latest",
Expand All @@ -1161,9 +1168,74 @@ def check_docker_proc(process: Process):
rule_runner,
Address("docker/test", target_name="img1"),
process_assertions=check_docker_proc,
options=dict(use_buildx=True),
)


def test_docker_output_option(rule_runner: RuleRunner) -> None:
"""Testing non-default output type 'image'.
Default output type 'docker' tested implicitly in other scenarios
"""
rule_runner.write_files(
{
"docker/test/BUILD": dedent(
"""\
docker_image(
name="img1",
output={"type": "image"}
)
"""
),
}
)

def check_docker_proc(process: Process):
assert process.argv == (
"/dummy/docker",
"buildx",
"build",
"--output=type=image",
"--pull=False",
"--tag",
"img1:latest",
"--file",
"docker/test/Dockerfile",
".",
)

assert_build(
rule_runner,
Address("docker/test", target_name="img1"),
process_assertions=check_docker_proc,
options=dict(use_buildx=True),
)


def test_docker_output_option_raises_when_no_buildkit(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"docker/test/BUILD": dedent(
"""\
docker_image(
name="img1",
output={"type": "image"}
)
"""
),
}
)

with pytest.raises(
DockerImageOptionValueError,
match=r"Buildx must be enabled via the Docker subsystem options in order to use this field.",
):
assert_build(
rule_runner,
Address("docker/test", target_name="img1"),
)


def test_docker_build_network_option(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
Expand Down
8 changes: 8 additions & 0 deletions src/python/pants/backend/docker/subsystems/docker_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,14 @@ def env_vars(self) -> tuple[str, ...]:
"""
),
)
use_buildx = BoolOption(
default=False,
help=softwrap(
"""
Use [buildx](https://github.com/docker/buildx#buildx) (and BuildKit) for builds.
"""
),
)
_build_args = ShellStrListOption(
help=softwrap(
f"""
Expand Down
52 changes: 46 additions & 6 deletions src/python/pants/backend/docker/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
)
from pants.engine.unions import union
from pants.util.docutil import bin_name, doc_url
from pants.util.frozendict import FrozenDict
from pants.util.strutil import help_text, softwrap

# Common help text to be applied to each field that supports value interpolation.
Expand Down Expand Up @@ -304,21 +305,33 @@ class DockerBuildOptionFieldMultiValueDictMixin(DictStringToStringField):
docker_build_option: ClassVar[str]

@final
def options(self, value_formatter: OptionValueFormatter) -> Iterator[str]:
def options(self, value_formatter: OptionValueFormatter, **kwargs) -> Iterator[str]:
if self.value:
yield f"{self.docker_build_option}=" + ",".join(
f"{key}={value_formatter(value)}" for key, value in self.value.items()
)


class DockerBuildKitOptionField:
"""Mixin to indicate a BuildKit-specific option."""

@abstractmethod
def options(self, value_formatter: OptionValueFormatter) -> Iterator[str]:
...

required_help = "This option requires BuildKit to be enabled via the Docker subsystem options."


class DockerImageBuildImageCacheToField(
DockerBuildOptionFieldMultiValueDictMixin, DictStringToStringField
DockerBuildOptionFieldMultiValueDictMixin, DictStringToStringField, DockerBuildKitOptionField
):
alias = "cache_to"
help = help_text(
f"""
Export image build cache to an external cache destination.
{DockerBuildKitOptionField.required_help}
Example:
docker_image(
Expand All @@ -340,13 +353,15 @@ class DockerImageBuildImageCacheToField(


class DockerImageBuildImageCacheFromField(
DockerBuildOptionFieldMultiValueDictMixin, DictStringToStringField
DockerBuildOptionFieldMultiValueDictMixin, DictStringToStringField, DockerBuildKitOptionField
):
alias = "cache_from"
help = help_text(
f"""
Use an external cache source when building the image.
{DockerBuildKitOptionField.required_help}
Example:
docker_image(
Expand All @@ -367,6 +382,30 @@ class DockerImageBuildImageCacheFromField(
docker_build_option = "--cache-from"


class DockerImageBuildImageOutputField(
DockerBuildOptionFieldMultiValueDictMixin, DictStringToStringField, DockerBuildKitOptionField
):
alias = "output"
default = FrozenDict({"type": "docker"})
help = help_text(
f"""
Sets the export action for the build result.
{DockerBuildKitOptionField.required_help}
When using `pants publish` to publish Docker images to a registry, the output type
must be 'docker', as `publish` expects that the built images exist in the local
image store.
Currently, multi-platform images cannot be exported with the 'docker' export type,
although experimental support is available with the [containerd image store](https://docs.docker.com/desktop/containerd/)
{_interpolation_help.format(kind="Values")}
"""
)
docker_build_option = "--output"


class DockerImageBuildSecretsOptionField(
AsyncFieldMixin, DockerBuildOptionFieldMixin, DictStringToStringField
):
Expand Down Expand Up @@ -441,7 +480,7 @@ class DockerBuildOptionFieldValueMixin(Field):
docker_build_option: ClassVar[str]

@final
def options(self) -> Iterator[str]:
def options(self, *args, **kwargs) -> Iterator[str]:
if self.value is not None:
yield f"{self.docker_build_option}={self.value}"

Expand All @@ -453,7 +492,7 @@ class DockerBuildOptionFieldMultiValueMixin(StringSequenceField):
docker_build_option: ClassVar[str]

@final
def options(self) -> Iterator[str]:
def options(self, *args, **kwargs) -> Iterator[str]:
if self.value:
yield f"{self.docker_build_option}={','.join(list(self.value))}"

Expand All @@ -479,7 +518,7 @@ class DockerBuildOptionFlagFieldMixin(BoolField, ABC):
docker_build_option: ClassVar[str]

@final
def options(self) -> Iterator[str]:
def options(self, *args, **kwargs) -> Iterator[str]:
if self.value:
yield f"{self.docker_build_option}"

Expand Down Expand Up @@ -547,6 +586,7 @@ class DockerImageTarget(Target):
DockerImageBuildPlatformOptionField,
DockerImageBuildImageCacheToField,
DockerImageBuildImageCacheFromField,
DockerImageBuildImageOutputField,
OutputPathField,
RestartableField,
)
Expand Down
Loading

0 comments on commit c621a71

Please sign in to comment.