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

docker: include file dependencies in docker build context properly. #12758

Merged
merged 12 commits into from
Sep 8, 2021
8 changes: 4 additions & 4 deletions src/python/pants/backend/docker/docker_binary.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ class DockerBinary(BinaryPath):

DEFAULT_SEARCH_PATH = SearchPath(("/usr/bin", "/bin", "/usr/local/bin"))

def build_image(
self, tag: str, digest: Digest, context_root: str, dockerfile: Optional[str] = None
) -> Process:
def build_image(self, tag: str, digest: Digest, dockerfile: Optional[str] = None) -> Process:
args = [self.path, "build", "-t", tag]
if dockerfile:
args.extend(["-f", dockerfile])
args.append(context_root)

# Add build context root.
args.append(".")

return Process(
argv=tuple(args),
Expand Down
8 changes: 3 additions & 5 deletions src/python/pants/backend/docker/docker_binary_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,22 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from hashlib import sha256
from os import path

from pants.backend.docker.docker_binary import DockerBinary
from pants.engine.fs import Digest
from pants.engine.process import Process


def test_docker_binary_build_image():
source_path = "src/test/repo"
docker_path = "/bin/docker"
dockerfile = path.join(source_path, "Dockerfile")
dockerfile = "src/test/repo/Dockerfile"
docker = DockerBinary(docker_path)
digest = Digest(sha256().hexdigest(), 123)
tag = "test:latest"
build_request = docker.build_image(tag, digest, source_path, dockerfile)
build_request = docker.build_image(tag, digest, dockerfile)

assert build_request == Process(
argv=(docker_path, "build", "-t", tag, "-f", dockerfile, source_path),
argv=(docker_path, "build", "-t", tag, "-f", dockerfile, "."),
input_digest=digest,
description=f"Building docker image {tag}",
)
31 changes: 9 additions & 22 deletions src/python/pants/backend/docker/docker_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@

from pants.backend.docker.docker_binary import DockerBinary, DockerBinaryRequest
from pants.backend.docker.docker_build_context import DockerBuildContext, DockerBuildContextRequest
from pants.backend.docker.target_types import (
DockerContextRoot,
DockerImageSources,
DockerImageVersion,
)
from pants.backend.docker.target_types import DockerImageSources, DockerImageVersion
from pants.core.goals.package import BuiltPackage, BuiltPackageArtifact, PackageFieldSet
from pants.engine.process import Process, ProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, rule
Expand All @@ -24,7 +20,6 @@
class DockerFieldSet(PackageFieldSet):
required_fields = (DockerImageSources,)

context_root_field: DockerContextRoot
image_version: DockerImageVersion
sources: DockerImageSources

Expand All @@ -38,17 +33,6 @@ def dockerfile_relpath(self) -> str:
def dockerfile_path(self) -> str:
return os.path.join(self.address.spec_path, self.dockerfile_relpath)

@property
def context_root(self) -> str:
"""Translates context_root from a users perspective in the BUILD file, to Dockers
perspective of the file system."""
path = self.context_root_field.value or "."
if os.path.isabs(path):
path = os.path.relpath(path, "/")
else:
path = os.path.join(self.address.spec_path, path)
return path

@property
def image_tag(self) -> str:
return ":".join(s for s in [self.address.target_name, self.image_version.value] if s)
Expand All @@ -61,18 +45,21 @@ async def build_docker_image(
docker, context = await MultiGet(
Get(DockerBinary, DockerBinaryRequest()),
Get(
DockerBuildContext, DockerBuildContextRequest(field_set.address, field_set.context_root)
DockerBuildContext,
DockerBuildContextRequest(
address=field_set.address,
build_upstream_images=True,
),
),
)

result = await Get(
ProcessResult,
Process,
docker.build_image(
field_set.image_tag,
context.digest,
field_set.context_root,
field_set.dockerfile_path,
tag=field_set.image_tag,
digest=context.digest,
dockerfile=field_set.dockerfile_path,
),
)

Expand Down
60 changes: 37 additions & 23 deletions src/python/pants/backend/docker/docker_build_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,22 @@

import logging
from dataclasses import dataclass
from itertools import chain

from pants.backend.docker.target_types import DockerImageSources
from pants.core.goals.package import BuiltPackage, PackageFieldSet
from pants.core.target_types import FilesSources, ResourcesSources
from pants.core.target_types import FilesSources
from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
from pants.engine.addresses import Address
from pants.engine.fs import AddPrefix, Digest, MergeDigests
from pants.engine.fs import Digest, MergeDigests
from pants.engine.rules import Get, MultiGet, collect_rules, rule
from pants.engine.target import (
Dependencies,
DependenciesRequest,
FieldSetsPerTarget,
FieldSetsPerTargetRequest,
Sources,
Targets,
TransitiveTargets,
TransitiveTargetsRequest,
)
Expand All @@ -30,19 +34,35 @@ class DockerBuildContext:
@dataclass(frozen=True)
class DockerBuildContextRequest:
address: Address
context_root: str
build_upstream_images: bool = False
benjyw marked this conversation as resolved.
Show resolved Hide resolved


@rule
async def create_docker_build_context(request: DockerBuildContextRequest) -> DockerBuildContext:
# Get all targets to include in context.
transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest([request.address]))

# Get all sources, going into the build context.
# Get the Dockerfile from the root target.
dockerfiles_request = Get(
SourceFiles,
SourceFilesRequest(
sources_fields=[t.get(Sources) for t in transitive_targets.roots],
for_sources_types=(DockerImageSources,),
),
)

# Get all dependencies for the root target.
root_dependencies = await MultiGet(
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, this will only take the direct dependencies. That's probably OK? It means that if there is some intermediate target (e.g., of type target() - which is a generic target useful for aggregating dependencies - we won't traverse it, but that's probably fine for now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes that's true.

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, this will only take the direct dependencies. That's probably OK? It means that if there is some intermediate target (e.g., of type target() - which is a generic target useful for aggregating dependencies) - we won't traverse it, but that's probably fine for now.

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, this will only take the direct dependencies. That's probably OK? It means that if there is some intermediate target (e.g., of type target() - which is a generic target useful for aggregating dependencies) - we won't traverse it, but that's probably fine for now.

Get(Targets, DependenciesRequest(target.get(Dependencies)))
for target in transitive_targets.roots
)

# Get all sources from the root dependencies (i.e. files).
sources_request = Get(
SourceFiles,
SourceFilesRequest(
sources_fields=[t.get(Sources) for t in transitive_targets.closure],
for_sources_types=(DockerImageSources, FilesSources, ResourcesSources),
sources_fields=[t.get(Sources) for t in chain(*root_dependencies)],
for_sources_types=(FilesSources,),
),
)

Expand All @@ -51,37 +71,31 @@ async def create_docker_build_context(request: DockerBuildContextRequest) -> Doc
FieldSetsPerTargetRequest(PackageFieldSet, transitive_targets.dependencies),
)

sources, embedded_pkgs_per_target = await MultiGet(
sources_request, embedded_pkgs_per_target_request
dockerfiles, sources, embedded_pkgs_per_target = await MultiGet(
dockerfiles_request,
sources_request,
embedded_pkgs_per_target_request,
)

# Package binary dependencies for build context.
embedded_pkgs = await MultiGet(
Get(BuiltPackage, PackageFieldSet, field_set)
for field_set in embedded_pkgs_per_target.field_sets
# Exclude docker images, unless build_upstream_images is true.
if request.build_upstream_images
or not isinstance(getattr(field_set, "sources", None), DockerImageSources)
)

packages_str = ", ".join(a.relpath for p in embedded_pkgs for a in p.artifacts if a.relpath)
logger.debug(f"Packages for Docker image: {packages_str}")

embedded_pkgs_digests = tuple(built_package.digest for built_package in embedded_pkgs)
if request.context_root != ".":
# Copy packages to context root tree, unless the context root is at the project root.
embedded_pkgs_digests = await MultiGet(
Get(Digest, AddPrefix(digest, request.context_root)) for digest in embedded_pkgs_digests
)
embedded_pkgs_digest = [built_package.digest for built_package in embedded_pkgs]
all_digests = (dockerfiles.snapshot.digest, sources.snapshot.digest, *embedded_pkgs_digest)

# Merge build context.
# Merge all digests to get the final docker build context.
context = await Get(
Digest,
MergeDigests(
d
for d in (
sources.snapshot.digest,
*embedded_pkgs_digests,
)
if d
),
MergeDigests(d for d in all_digests if d),
)

return DockerBuildContext(context)
Expand Down
Loading