Fix docker invalidation bug.

src/python/pants/backend/docker/goals/package_image.py

@@ -256,13 +256,20 @@ async def build_docker_image(
         interpolation_context=context.interpolation_context,
     )
 
+    # Mix the upstream image ids into the env to ensure that Pants invalidates this
+    # image-building process correctly when an upstream image changes, even though the
+    # process itself does not consume this data.
+    env = {
+        **context.build_env.environment,
+        "__UPSTREAM_IMAGE_IDS": ",".join(context.upstream_image_ids),
+    }
     context_root = field_set.get_context_root(options.default_context_root)
     process = docker.build_image(
         build_args=context.build_args,
         digest=context.digest,
         dockerfile=context.dockerfile,
         context_root=context_root,
-        env=context.build_env.environment,
+        env=env,
         tags=tags,
         extra_args=tuple(
             get_build_options(
@@ -317,6 +324,9 @@ async def build_docker_image(
 
 def parse_image_id_from_docker_build_output(*outputs: bytes) -> str:
     """Outputs are typically the stdout/stderr pair from the `docker build` process."""
+    # NB: We use the extracted image id for invalidation. The short_id may theoretically
+    #  not be unique enough, although in a non adversarial situation, this is highly unlikely
+    #  to be an issue in practice.
     image_id_regexp = re.compile(
         "|".join(
             (

src/python/pants/backend/docker/goals/package_image_test.py

@@ -92,6 +92,7 @@ def assert_build(
     def build_context_mock(request: DockerBuildContextRequest) -> DockerBuildContext:
         return DockerBuildContext.create(
             snapshot=build_context_snapshot,
+            upstream_image_ids=[],
             dockerfile_info=DockerfileInfo(
                 request.address,
                 digest=EMPTY_DIGEST,
@@ -444,6 +445,7 @@ def check_docker_proc(process: Process):
             {
                 "INHERIT": "from Pants env",
                 "VAR": "value",
+                "__UPSTREAM_IMAGE_IDS": "",
             }
         )
 
@@ -485,6 +487,7 @@ def check_docker_proc(process: Process):
         assert process.env == FrozenDict(
             {
                 "INHERIT": "from Pants env",
+                "__UPSTREAM_IMAGE_IDS": "",
             }
         )
 
@@ -581,6 +584,7 @@ def check_docker_proc(process: Process):
         assert process.env == FrozenDict(
             {
                 "FROM_ENV": "env value",
+                "__UPSTREAM_IMAGE_IDS": "",
             }
         )
 

src/python/pants/backend/docker/package_types.py

@@ -11,12 +11,17 @@
 
 @dataclass(frozen=True)
 class BuiltDockerImage(BuiltPackageArtifact):
+    # We don't really want a default for this field, but the superclass has a field with
+    # a default, so all subsequent fields must have one too. The `create()` method below
+    # will ensure that this field is properly populated in practice.
+    image_id: str = ""
     tags: tuple[str, ...] = ()
 
     @classmethod
     def create(cls, image_id: str, tags: tuple[str, ...]) -> BuiltDockerImage:
-        tags_string = tags[0] if len(tags) == 1 else (f"\n{bullet_list(tags)}")
+        tags_string = tags[0] if len(tags) == 1 else f"\n{bullet_list(tags)}"
         return cls(
+            image_id=image_id,
             tags=tags,
             relpath=None,
             extra_log_lines=(

src/python/pants/backend/docker/util_rules/docker_binary.py

@@ -88,6 +88,8 @@ def build_image(
             env=self._get_process_environment(env),
             input_digest=digest,
             immutable_input_digests=self.extra_input_digests,
+            # We must run the docker build commands every time, even if nothing has changed,
+            # in case the user ran `docker image rm` outside of Pants.
             cache_scope=ProcessCacheScope.PER_SESSION,
         )
 

src/python/pants/backend/docker/util_rules/docker_build_context.py

@@ -7,10 +7,9 @@
 import re
 from abc import ABC
 from dataclasses import dataclass
-from typing import Mapping
+from typing import Iterable, Mapping
 
 from pants.backend.docker.package_types import BuiltDockerImage
-from pants.backend.docker.subsystems.docker_options import DockerOptions
 from pants.backend.docker.subsystems.dockerfile_parser import DockerfileInfo, DockerfileInfoRequest
 from pants.backend.docker.target_types import DockerImageSourceField
 from pants.backend.docker.util_rules.docker_build_args import (
@@ -104,6 +103,7 @@ class DockerBuildContext:
     build_args: DockerBuildArgs
     digest: Digest
     build_env: DockerBuildEnvironment
+    upstream_image_ids: tuple[str, ...]
     dockerfile: str
     interpolation_context: DockerInterpolationContext
     copy_source_vs_context_source: tuple[tuple[str, str], ...]
@@ -115,6 +115,7 @@ def create(
         build_args: DockerBuildArgs,
         snapshot: Snapshot,
         build_env: DockerBuildEnvironment,
+        upstream_image_ids: Iterable[str],
         dockerfile_info: DockerfileInfo,
     ) -> DockerBuildContext:
         interpolation_context: dict[str, dict[str, str] | DockerInterpolationValue] = {}
@@ -146,6 +147,7 @@ def create(
             digest=snapshot.digest,
             dockerfile=dockerfile_info.source,
             build_env=build_env,
+            upstream_image_ids=tuple(sorted(upstream_image_ids)),
             interpolation_context=DockerInterpolationContext.from_dict(interpolation_context),
             copy_source_vs_context_source=tuple(
                 suggest_renames(
@@ -256,9 +258,7 @@ def _merge_build_args(
 
 
 @rule
-async def create_docker_build_context(
-    request: DockerBuildContextRequest, docker_options: DockerOptions
-) -> DockerBuildContext:
+async def create_docker_build_context(request: DockerBuildContextRequest) -> DockerBuildContext:
     # Get all targets to include in context.
     transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest([request.address]))
     docker_image = transitive_targets.roots[0]
@@ -330,6 +330,7 @@ async def create_docker_build_context(
         context_request, build_args_request, build_env_request
     )
 
+    upstream_image_ids = []
     if request.build_upstream_images:
         # Update build arg values for FROM image build args.
 
@@ -360,6 +361,12 @@ async def create_docker_build_context(
             for image in built.artifacts
             if isinstance(image, BuiltDockerImage)
         }
+        upstream_image_ids = [
+            image.image_id
+            for built in embedded_pkgs
+            for image in built.artifacts
+            if isinstance(image, BuiltDockerImage)
+        ]
         # Create the FROM image build args.
         from_image_build_args = [
             f"{arg_name}={address_to_built_image_tag[addr]}"
@@ -371,6 +378,7 @@ async def create_docker_build_context(
     return DockerBuildContext.create(
         build_args=build_args,
         snapshot=context,
+        upstream_image_ids=upstream_image_ids,
         dockerfile_info=dockerfile_info,
         build_env=build_env,
     )

src/python/pants/backend/docker/util_rules/docker_build_context_test.py

@@ -93,6 +93,7 @@ def assert_build_context(
     expected_files: list[str],
     expected_interpolation_context: dict[str, str | dict[str, str] | DockerInterpolationValue]
     | None = None,
+    expected_num_upstream_images: int = 0,
     pants_args: list[str] | None = None,
     runner_options: dict[str, Any] | None = None,
 ) -> DockerBuildContext:
@@ -127,6 +128,8 @@ def assert_build_context(
             DockerInterpolationContext.from_dict(expected_interpolation_context)
         )
 
+    if build_upstream_images:
+        assert len(context.upstream_image_ids) == expected_num_upstream_images
     return context
 
 
@@ -224,7 +227,7 @@ def test_from_image_build_arg_dependency(rule_runner: RuleRunner) -> None:
                   name="image",
                   repository="upstream/{name}",
                   image_tags=["1.0"],
-                  instructions=["FROM alpine"],
+                  instructions=["FROM alpine:3.16.1"],
                 )
                 """
             ),
@@ -252,6 +255,7 @@ def test_from_image_build_arg_dependency(rule_runner: RuleRunner) -> None:
                 "BASE_IMAGE": "upstream/image:1.0",
             },
         },
+        expected_num_upstream_images=1,
     )
 
 
@@ -343,7 +347,7 @@ def test_interpolation_context_from_dockerfile(rule_runner: RuleRunner) -> None:
             "src/docker/Dockerfile": dedent(
                 """\
                 FROM python:3.8
-                FROM alpine as interim
+                FROM alpine:3.16.1 as interim
                 FROM interim
                 FROM scratch:1-1 as output
                 """
@@ -359,7 +363,7 @@ def test_interpolation_context_from_dockerfile(rule_runner: RuleRunner) -> None:
             "tags": {
                 "baseimage": "3.8",
                 "stage0": "3.8",
-                "interim": "latest",
+                "interim": "3.16.1",
                 "stage2": "latest",
                 "output": "1-1",
             },
@@ -376,7 +380,7 @@ def test_synthetic_dockerfile(rule_runner: RuleRunner) -> None:
                 docker_image(
                   instructions=[
                     "FROM python:3.8",
-                    "FROM alpine as interim",
+                    "FROM alpine:3.16.1 as interim",
                     "FROM interim",
                     "FROM scratch:1-1 as output",
                   ]
@@ -394,7 +398,7 @@ def test_synthetic_dockerfile(rule_runner: RuleRunner) -> None:
             "tags": {
                 "baseimage": "3.8",
                 "stage0": "3.8",
-                "interim": "latest",
+                "interim": "3.16.1",
                 "stage2": "latest",
                 "output": "1-1",
             },
@@ -621,6 +625,7 @@ def test_create_docker_build_context() -> None:
         build_args=DockerBuildArgs.from_strings("ARGNAME=value1"),
         snapshot=EMPTY_SNAPSHOT,
         build_env=DockerBuildEnvironment.create({"ENVNAME": "value2"}),
+        upstream_image_ids=["def", "abc"],
         dockerfile_info=DockerfileInfo(
             address=Address("test"),
             digest=EMPTY_DIGEST,
@@ -633,6 +638,7 @@ def test_create_docker_build_context() -> None:
     )
     assert list(context.build_args) == ["ARGNAME=value1"]
     assert dict(context.build_env.environment) == {"ENVNAME": "value2"}
+    assert context.upstream_image_ids == ("abc", "def")
     assert context.dockerfile == "test/Dockerfile"
     assert context.stages == ("base", "dev", "prod")