Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge Docker image_name into the repository field. #13225

Merged
merged 3 commits into from Oct 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
73 changes: 40 additions & 33 deletions src/python/pants/backend/docker/goals/package_image.py
Expand Up @@ -10,13 +10,11 @@
from pants.backend.docker.registries import DockerRegistries
from pants.backend.docker.subsystems.docker_options import DockerEnvironmentVars, DockerOptions
from pants.backend.docker.target_types import (
DockerImageName,
DockerImageNameTemplate,
DockerImageSources,
DockerImageTags,
DockerImageVersion,
DockerRegistriesField,
DockerRepository,
DockerRepositoryField,
)
from pants.backend.docker.util_rules.docker_binary import DockerBinary
from pants.backend.docker.util_rules.docker_build_context import (
Expand All @@ -36,7 +34,7 @@
logger = logging.getLogger(__name__)


class DockerNameTemplateError(ValueError):
class DockerRepositoryNameError(ValueError):
pass


Expand All @@ -60,10 +58,8 @@ def create(cls, tags: tuple[str, ...]) -> BuiltDockerImage:
class DockerFieldSet(PackageFieldSet, RunFieldSet):
required_fields = (DockerImageSources,)

name: DockerImageName
name_template: DockerImageNameTemplate
registries: DockerRegistriesField
repository: DockerRepository
repository: DockerRepositoryField
sources: DockerImageSources
tags: DockerImageTags
version: DockerImageVersion
Expand All @@ -78,50 +74,60 @@ def dockerfile_relpath(self) -> str:
def dockerfile_path(self) -> str:
return path.join(self.address.spec_path, self.dockerfile_relpath)

def image_names(
def image_refs(
self,
default_name_template: str,
default_repository: str,
registries: DockerRegistries,
version_context: FrozenDict[str, DockerVersionContextValue],
) -> tuple[str, ...]:
"""This method will always return a non-empty tuple."""
default_parent = path.basename(path.dirname(self.address.spec_path))
default_repo = path.basename(self.address.spec_path)
repo = self.repository.value or default_repo
name_template = self.name_template.value or default_name_template
"""The image refs are the full image name, including any registry and version tag.

In the Docker world, the term `tag` is used both for what we here prefer to call the image
`ref`, as well as for the image version, or tag, that is at the end of the image name
separated with a colon. By introducing the image `ref` we can retain the use of `tag` for
the version part of the image name.

Returns all image refs to apply to the Docker image, on the form:

[<registry>/]<repository-name>[:<tag>]

Where the `<repository-name>` may have contain any number of separating slashes `/`,
depending on the `default_repository` from configuration or the `repository` field
on the target `docker_image`.

This method will always return a non-empty tuple.
"""
directory = path.basename(self.address.spec_path)
parent_directory = path.basename(path.dirname(self.address.spec_path))
repository_fmt = self.repository.value or default_repository
try:
image_name = name_template.format(
name=self.name.value or self.address.target_name,
repository=repo,
sub_repository="/".join(
[default_repo if self.repository.value else default_parent, repo]
),
repository = repository_fmt.format(
name=self.address.target_name,
directory=directory,
parent_directory=parent_directory,
)
except KeyError as e:
if self.name_template.value:
source = (
"from the `image_name_template` field of the docker_image target "
f"at {self.address}"
)
if self.repository.value:
source = "`repository` field of the `docker_image` target " f"at {self.address}"
else:
source = "from the [docker].default_image_name_template configuration option"
source = "`[docker].default_repository` configuration option"

raise DockerNameTemplateError(
f"Invalid image name template {source}: {name_template!r}. Unknown key: {e}.\n\n"
f"Use any of 'name', 'repository' or 'sub_repository' in the template string."
raise DockerRepositoryNameError(
f"Invalid value for the {source}: {repository_fmt!r}. Unknown key: {e}.\n\n"
f"You may only reference any of `name`, `directory` or `parent_directory`."
) from e

try:
image_names = tuple(
":".join(s for s in [image_name, tag] if s)
":".join(s for s in [repository, tag] if s)
for tag in [
cast(str, self.version.value).format(**version_context),
*(self.tags.value or []),
]
)
except (KeyError, ValueError) as e:
msg = (
"Invalid format string for the `version` field of the docker_image target at "
"Invalid format string for the `version` field of the `docker_image` target at "
f"{self.address}: {self.version.value!r}.\n\n"
)
if isinstance(e, KeyError):
Expand All @@ -139,6 +145,7 @@ def image_names(

registries_options = tuple(registries.get(*(self.registries.value or [])))
if not registries_options:
# The image name is also valid as image ref without registry.
return image_names

return tuple(
Expand Down Expand Up @@ -172,8 +179,8 @@ async def build_docker_image(

version_context = context.version_context.merge({"build_args": build_args_context})

tags = field_set.image_names(
default_name_template=options.default_image_name_template,
tags = field_set.image_refs(
default_repository=options.default_repository,
registries=options.registries(),
version_context=version_context,
)
Expand Down
Expand Up @@ -57,4 +57,4 @@ def test_docker_build(rule_runner) -> None:
target = rule_runner.get_target(Address("src", target_name="test-image"))
result = run_docker(rule_runner, target)
assert len(result.artifacts) == 1
assert "Built docker image: src/test-image:1.0" in result.artifacts[0].extra_log_lines
assert "Built docker image: test-image:1.0" in result.artifacts[0].extra_log_lines
47 changes: 24 additions & 23 deletions src/python/pants/backend/docker/goals/package_image_test.py
Expand Up @@ -10,7 +10,7 @@

from pants.backend.docker.goals.package_image import (
DockerFieldSet,
DockerNameTemplateError,
DockerRepositoryNameError,
build_docker_image,
)
from pants.backend.docker.registries import DockerRegistries
Expand Down Expand Up @@ -72,7 +72,7 @@ def run_process_mock(process: Process) -> ProcessResult:
if options:
opts = options or {}
opts.setdefault("registries", {})
opts.setdefault("default_image_name_template", "{repository}/{name}")
opts.setdefault("default_repository", "{directory}/{name}")
opts.setdefault("build_args", [])
opts.setdefault("env_vars", [])

Expand Down Expand Up @@ -124,7 +124,7 @@ def test_build_docker_image(rule_runner: RuleRunner) -> None:
docker_image(
name="test1",
version="1.2.3",
image_name_template="{name}",
repository="{directory}/{name}",
)
docker_image(
name="test2",
Expand All @@ -133,22 +133,20 @@ def test_build_docker_image(rule_runner: RuleRunner) -> None:
docker_image(
name="test3",
version="1.2.3",
image_name_template="{sub_repository}/{name}",
repository="{parent_directory}/{directory}/{name}",
)
docker_image(
name="test4",
version="1.2.3",
image_name="test-four",
image_name_template="{sub_repository}/{name}",
repository="four",
repository="{directory}/four/test-four",
)
docker_image(
name="test5",
image_tags=["alpha-1.0", "alpha-1"],
)
docker_image(
name="err1",
image_name_template="{bad_template}",
repository="{bad_template}",
)
"""
),
Expand All @@ -157,12 +155,14 @@ def test_build_docker_image(rule_runner: RuleRunner) -> None:
)

assert_build(
rule_runner, Address("docker/test", target_name="test1"), "Built docker image: test1:1.2.3"
rule_runner,
Address("docker/test", target_name="test1"),
"Built docker image: test/test1:1.2.3",
)
assert_build(
rule_runner,
Address("docker/test", target_name="test2"),
"Built docker image: test/test2:1.2.3",
"Built docker image: test2:1.2.3",
)
assert_build(
rule_runner,
Expand All @@ -183,14 +183,15 @@ def test_build_docker_image(rule_runner: RuleRunner) -> None:
" * test/test5:alpha-1.0\n"
" * test/test5:alpha-1"
),
options=dict(default_repository="{directory}/{name}"),
)

err1 = (
r"Invalid image name template from the `image_name_template` field of the docker_image "
r"target at docker/test:err1: '{bad_template}'\. Unknown key: 'bad_template'\.\n\n"
r"Use any of 'name', 'repository' or 'sub_repository' in the template string\."
r"Invalid value for the `repository` field of the `docker_image` target at "
r"docker/test:err1: '{bad_template}'\. Unknown key: 'bad_template'\.\n\n"
r"You may only reference any of `name`, `directory` or `parent_directory`\."
)
with pytest.raises(DockerNameTemplateError, match=err1):
with pytest.raises(DockerRepositoryNameError, match=err1):
assert_build(
rule_runner,
Address("docker/test", target_name="err1"),
Expand Down Expand Up @@ -218,7 +219,7 @@ def test_build_image_with_registries(rule_runner: RuleRunner) -> None:
)

options = {
"default_image_name_template": "{name}",
"default_repository": "{name}",
"registries": {
"reg1": {"address": "myregistry1domain:port"},
"reg2": {"address": "myregistry2domain:port", "default": "true"},
Expand Down Expand Up @@ -299,7 +300,7 @@ def test_dynamic_image_version(rule_runner: RuleRunner) -> None:
def assert_tags(name: str, *expect_tags: str) -> None:
tgt = rule_runner.get_target(Address("docker/test", target_name=name))
fs = DockerFieldSet.create(tgt)
tags = fs.image_names(
tags = fs.image_refs(
"image",
DockerRegistries.from_dict({}),
version_context,
Expand Down Expand Up @@ -327,17 +328,17 @@ def assert_tags(name: str, *expect_tags: str) -> None:
assert_tags("ver_2", "image:3.8-latest", "image:beta")

err_1 = (
r"Invalid format string for the `version` field of the docker_image target at docker/test:err_1: "
r"'{unknown_stage}'\.\n\n"
r"Invalid format string for the `version` field of the `docker_image` target at "
r"docker/test:err_1: '{unknown_stage}'\.\n\n"
r"The key 'unknown_stage' is unknown\. Try with one of: baseimage, stage0, interim, "
r"stage2, output\."
)
with pytest.raises(DockerVersionContextError, match=err_1):
assert_tags("err_1")

err_2 = (
r"Invalid format string for the `version` field of the docker_image target at docker/test:err_2: "
r"'{stage0.unknown_value}'\.\n\n"
r"Invalid format string for the `version` field of the `docker_image` target at "
r"docker/test:err_2: '{stage0.unknown_value}'\.\n\n"
r"The key 'unknown_value' is unknown\. Try with one of: tag\."
)
with pytest.raises(DockerVersionContextError, match=err_2):
Expand All @@ -359,7 +360,7 @@ def check_docker_proc(process: Process):
"/dummy/docker",
"build",
"-t",
"test/env1:1.2.3",
"env1:1.2.3",
"-f",
"docker/test/Dockerfile",
".",
Expand Down Expand Up @@ -393,7 +394,7 @@ def check_docker_proc(process: Process):
"/dummy/docker",
"build",
"-t",
"test/args1:1.2.3",
"args1:1.2.3",
"--build-arg",
"INHERIT",
"--build-arg",
Expand Down Expand Up @@ -431,5 +432,5 @@ def test_docker_image_version_from_build_arg(rule_runner: RuleRunner) -> None:
assert_build(
rule_runner,
Address("docker/test", target_name="ver1"),
"Built docker image: test/ver1:1.2.3",
"Built docker image: ver1:1.2.3",
)
6 changes: 3 additions & 3 deletions src/python/pants/backend/docker/goals/publish_test.py
Expand Up @@ -59,8 +59,8 @@ def build(tgt: DockerImage, options: DockerOptions):
EMPTY_DIGEST,
(
BuiltDockerImage.create(
fs.image_names(
options.default_image_name_template,
fs.image_refs(
options.default_repository,
options.registries(),
FrozenDict(),
),
Expand All @@ -75,7 +75,7 @@ def run_publish(
) -> tuple[PublishProcesses, DockerBinary]:
opts = options or {}
opts.setdefault("registries", {})
opts.setdefault("default_image_name_template", "{repository}/{name}")
opts.setdefault("default_repository", "{directory}/{name}")
docker_options = create_subsystem(DockerOptions, **opts)
tgt = cast(DockerImage, rule_runner.get_target(address))
fs = PublishDockerImageFieldSet.create(tgt)
Expand Down
34 changes: 16 additions & 18 deletions src/python/pants/backend/docker/subsystems/docker_options.py
Expand Up @@ -46,28 +46,26 @@ def register_options(cls, register):
'or with an alias of `"default"`.'
)
)
image_name_default = "{repository}/{name}"
image_name_help = (
"Configure the default template used to construct the final Docker image name.\n\n"
"The template is a format string that may use these variables:\n\n"
+ bullet_list(["name", "repository", "sub_repository"])
default_repository_help = (
"Configure the default repository name used in the Docker image tag.\n\n"
"The value is formatted and may reference these variables:\n\n"
+ bullet_list(["name", "directory", "parent_directory"])
+ "\n\n"
"The `name` is the value of the `docker_image(image_name)` field, which defaults to "
"the target name, and the `repository` is the `docker_image(repository)` field, which "
"defaults to the name of the directory in which the BUILD file is for the target, and "
"finally the `sub_repository` is like that of repository but including the parent "
"directory as well.\n\n"
"Use the `docker_image(image_name_template)` field to override this default template.\n"
"Any registries or tags are added to the image name as required, and should not be "
"part of the name template."
'Example: `--default-repository="{directory}/{name}"`.\n\n'
"The `name` variable is the `docker_image`'s target name, `directory` and "
"`parent_directory` are the name of the directory in which the BUILD file is for the "
"target, and its parent directory respectively.\n\n"
"Use the `repository` field to set this value directly on a `docker_image` "
"target.\nAny registries or tags are added to the image name as required, and should "
"not be part of the repository name."
)
super().register_options(register)
register("--registries", type=dict, fromfile=True, help=registries_help)
register(
"--default-image-name-template",
"--default-repository",
type=str,
help=image_name_help,
default=image_name_default,
help=default_repository_help,
default="{name}",
)

register(
Expand Down Expand Up @@ -110,8 +108,8 @@ def env_vars_to_pass_to_docker(self) -> tuple[str, ...]:
)

@property
def default_image_name_template(self) -> str:
return cast(str, self.options.default_image_name_template)
def default_repository(self) -> str:
return cast(str, self.options.default_repository)

@memoized_method
def registries(self) -> DockerRegistries:
Expand Down