Fix breeze down ignoring the --cleanup-build-cache flag

dev/breeze/doc/03_developer_tasks.rst

@@ -809,6 +809,12 @@ Discovery includes volumes and networks even when no containers remain. Resource
 directly through Docker, so deleted worktrees do not need their Compose files restored.
 ``--preserve-volumes`` keeps named and anonymous volumes, including those from deleted worktrees.
 Shared MyPy and bytecode caches remain controlled by their explicit cleanup flags.
+``down`` keeps the build cache volume of each project (``/root/.cache`` in the container) until
+``--cleanup-build-cache`` is passed, which also works together with ``--preserve-volumes``.
+Without ``--preserve-volumes``, build cache volumes of deleted worktrees are removed without the flag.
+``breeze doctor`` removes them only when you confirm the cache removal. Other commands that bring
+a project down, such as ``breeze shell --restart``, still remove its build cache volume with the
+other volumes.
 Unlike ``breeze cleanup``, ``down`` does not delete local source files, environments, images,
 or build caches unless an explicit cache-cleanup flag is passed.
 

dev/breeze/doc/images/output_down.txt

@@ -1 +1 @@
-61e3246738b37b2284ee184454d8a2b4
+8c22f3aea0baaec2c789c4075359aba2

dev/breeze/src/airflow_breeze/commands/developer_commands.py

@@ -1122,7 +1122,8 @@ def build_docs(
     name="down",
     help=(
         "Stop Breeze projects with no worktree, in this checkout, or belonging to deleted worktrees. "
-        "Removes containers, networks and volumes without changing local files or images. "
+        "Removes containers, networks and volumes, except the build cache volume. "
+        "Local files and images are not changed. "
         "Use --all-worktrees to include other checkouts, or --project-name for one exact project."
     ),
 )
@@ -1146,7 +1147,8 @@ def build_docs(
 @click.option(
     "-b",
     "--cleanup-build-cache",
-    help="Additionally cleanup Build (pip/uv) cache.",
+    help="Also remove the build (pip/uv) cache volumes of the projects being brought down "
+    "(this checkout unless --all-worktrees or --project-name is used).",
     is_flag=True,
 )
 @click.option(
@@ -1179,6 +1181,7 @@ def down(
     current_worktree = str(AIRFLOW_ROOT_PATH.resolve()) if get_main_git_dir_for_worktree() else ""
     brought_down = bring_compose_projects_down(
         preserve_volumes=preserve_volumes,
+        cleanup_build_cache=cleanup_build_cache,
         all_worktrees=all_worktrees,
         only_project=project_name,
         current_worktree=current_worktree,
@@ -1214,9 +1217,6 @@ def down(
                 shutil.rmtree(hook_dir)
     if cleanup_pycache:
         run_command(["docker", "volume", "rm", "--force", PYCACHE_VOLUME_NAME])
-    if cleanup_build_cache:
-        command_to_execute = ["docker", "volume", "rm", "--force", "airflow-cache-volume"]
-        run_command(command_to_execute)
 
 
 @main.command(name="exec", help="Joins the interactive shell of running airflow container.")
@@ -1344,10 +1344,13 @@ def doctor(ctx):
     if not get_dry_run() and given_answer == Answer.YES:
         cleanup_python_generated_files()
 
-    bring_compose_projects_down(all_worktrees=True)
+    cleanup_caches = (
+        user_confirm("Are you sure with the removal of mypy cache, build cache volumes and build cache dir?")
+        == Answer.YES
+    )
+    bring_compose_projects_down(all_worktrees=True, cleanup_build_cache=cleanup_caches)
 
-    given_answer = user_confirm("Are you sure with the removal of mypy cache and build cache dir?")
-    if given_answer == Answer.YES:
+    if cleanup_caches:
         console_print("\n[info]Cleaning mypy cache...\n")
         command_to_execute = ["docker", "volume", "rm", "--force", "mypy-cache-volume"]
         run_command(command_to_execute)
@@ -1356,10 +1359,6 @@ def doctor(ctx):
             console_print(f"\n[info]Removing local mypy cache: {local_mypy_cache}\n")
             shutil.rmtree(local_mypy_cache)
 
-        console_print("\n[info]Cleaning build cache...\n")
-        command_to_execute = ["docker", "volume", "rm", "--force", "airflow-cache-volume"]
-        run_command(command_to_execute)
-
         console_print("\n[info]Deleting .build cache dir...\n")
         dirpath = Path(".build")
         if not get_dry_run() and dirpath.exists() and dirpath.is_dir():

dev/breeze/src/airflow_breeze/utils/docker_command_utils.py

@@ -38,6 +38,7 @@
 from airflow_breeze.utils.host_info_utils import get_host_group_id, get_host_os, get_host_user_id
 from airflow_breeze.utils.path_utils import (
     AIRFLOW_ROOT_PATH,
+    BUILD_CACHE_VOLUME,
     SCRIPTS_DOCKER_PATH,
     cleanup_python_generated_files,
     create_mypy_volume_if_needed,
@@ -1076,9 +1077,26 @@ def _get_compose_resources(
     ], result.returncode != 0
 
 
+def _is_volume_removable(
+    labels: dict[str, str], *, preserve_volumes: bool, cleanup_build_cache: bool
+) -> bool:
+    if labels.get("com.docker.compose.volume") != BUILD_CACHE_VOLUME:
+        return not preserve_volumes
+    # The build cache holds no state, so it outlives the project until it is asked for explicitly.
+    # A deleted worktree will never use its cache again.
+    return cleanup_build_cache or (
+        not preserve_volumes
+        and _belongs_to_deleted_worktree(
+            labels.get("org.apache.airflow.breeze.worktree", ""),
+            labels.get("org.apache.airflow.breeze.host", ""),
+        )
+    )
+
+
 def bring_compose_projects_down(
     *,
     preserve_volumes: bool = False,
+    cleanup_build_cache: bool = False,
     all_worktrees: bool = False,
     only_project: str | None = None,
     current_worktree: str = "",
@@ -1088,8 +1106,6 @@ def bring_compose_projects_down(
     projects: set[str] = set()
     failed = False
     for kind in targets:
-        if kind == "volume" and preserve_volumes:
-            continue
         resources, discovery_failed = _get_compose_resources(kind, stale_only=stale_only)
         failed |= discovery_failed
         for identifier, labels in resources:
@@ -1117,6 +1133,10 @@ def bring_compose_projects_down(
                 )
                 if not stale_only and legacy:
                     selected = True
+            if selected and kind == "volume":
+                selected = _is_volume_removable(
+                    labels, preserve_volumes=preserve_volumes, cleanup_build_cache=cleanup_build_cache
+                )
             if selected:
                 targets[kind].append(identifier)
                 projects.add(project)

dev/breeze/src/airflow_breeze/utils/path_utils.py

@@ -553,6 +553,9 @@ def find_airflow_root_path_to_operate_on() -> Path:
 
 
 PYCACHE_VOLUME_NAME = "airflow-pycache-volume"
+# Key of the `/root/.cache` volume in the compose files. Compose prefixes the real volume name with the
+# project name, so the volume is recognised by the `com.docker.compose.volume` label instead.
+BUILD_CACHE_VOLUME = "airflow-cache-volume"
 
 
 def create_volume_if_missing(volume_name: str):

dev/breeze/tests/test_developer_commands.py

@@ -17,16 +17,17 @@
 
 from __future__ import annotations
 
-from unittest.mock import create_autospec, patch
+from unittest.mock import call, create_autospec, patch
 
 import click
 import pytest
 from click import UsageError
 from click.testing import CliRunner
 
 from airflow_breeze.commands import developer_commands
-from airflow_breeze.commands.developer_commands import build_docs, down, run
+from airflow_breeze.commands.developer_commands import build_docs, doctor, down, run
 from airflow_breeze.global_constants import DEFAULT_PYTHON_MAJOR_MINOR_VERSION
+from airflow_breeze.utils.confirm import Answer
 
 
 @pytest.fixture
@@ -116,6 +117,51 @@ def test_down_lists_projects_left_in_other_worktrees(runner, tmp_path, args, lis
         assert "/work/other" not in output
 
 
+@pytest.mark.parametrize(("args", "cleanup_build_cache"), [([], False), (["--cleanup-build-cache"], True)])
+def test_down_cleans_build_cache_only_on_request(runner, args, cleanup_build_cache):
+    with (
+        patch("airflow_breeze.commands.developer_commands.perform_environment_checks", autospec=True),
+        patch(
+            "airflow_breeze.commands.developer_commands.bring_compose_projects_down",
+            autospec=True,
+            return_value=[],
+        ) as teardown,
+        patch(
+            "airflow_breeze.commands.developer_commands.find_other_worktree_projects",
+            autospec=True,
+            return_value={},
+        ),
+        patch("airflow_breeze.commands.developer_commands.run_command", autospec=True) as run,
+    ):
+        result = runner.invoke(down, args)
+    assert result.exit_code == 0
+    assert teardown.call_args.kwargs["cleanup_build_cache"] is cleanup_build_cache
+    run.assert_not_called()
+
+
+@pytest.mark.parametrize("cache_removal_confirmed", [False, True])
+def test_doctor_removes_build_cache_volumes_after_confirmation(runner, tmp_path, cache_removal_confirmed):
+    answers = [Answer.NO, Answer.YES if cache_removal_confirmed else Answer.NO, Answer.NO]
+    with (
+        patch("airflow_breeze.commands.developer_commands.ShellParams", autospec=True),
+        patch("airflow_breeze.commands.developer_commands.check_docker_resources", autospec=True),
+        patch("airflow_breeze.commands.developer_commands.perform_environment_checks", autospec=True),
+        patch("airflow_breeze.commands.developer_commands.fix_ownership_using_docker", autospec=True),
+        patch("airflow_breeze.commands.developer_commands.run_command", autospec=True),
+        patch("airflow_breeze.commands.developer_commands.AIRFLOW_ROOT_PATH", tmp_path),
+        patch("airflow_breeze.commands.developer_commands.user_confirm", autospec=True, side_effect=answers),
+        patch(
+            "airflow_breeze.commands.developer_commands.bring_compose_projects_down",
+            autospec=True,
+            return_value=[],
+        ) as teardown,
+        runner.isolated_filesystem(temp_dir=tmp_path),
+    ):
+        result = runner.invoke(doctor, catch_exceptions=False)
+    assert result.exit_code == 0
+    assert teardown.call_args_list == [call(all_worktrees=True, cleanup_build_cache=cache_removal_confirmed)]
+
+
 @pytest.fixture
 def docker_mocks(monkeypatch, tmp_path):
     monkeypatch.setenv("SKIP_SAVING_CHOICES", "true")

dev/breeze/tests/test_docker_command_utils.py

@@ -24,6 +24,7 @@
 from unittest.mock import call
 
 import pytest
+import yaml
 
 from airflow_breeze.global_constants import (
     ALLOWED_POSTGRES_VERSIONS,
@@ -45,6 +46,11 @@
     prepare_docker_build_command,
     pull_images_with_retries,
 )
+from airflow_breeze.utils.path_utils import (
+    BUILD_CACHE_VOLUME,
+    SCRIPTS_CI_DOCKER_COMPOSE_LOCAL_ALL_SOURCES_PATH,
+    SCRIPTS_CI_DOCKER_COMPOSE_LOCAL_YAML_PATH,
+)
 
 
 @pytest.mark.parametrize("listing_failed", [False, True])
@@ -745,6 +751,79 @@ def test_down_finds_volumes_without_containers(docker_resources, tmp_path, prese
     )
 
 
+@pytest.mark.parametrize(
+    "compose_file",
+    [SCRIPTS_CI_DOCKER_COMPOSE_LOCAL_YAML_PATH, SCRIPTS_CI_DOCKER_COMPOSE_LOCAL_ALL_SOURCES_PATH],
+    ids=lambda path: path.name,
+)
+def test_build_cache_volume_is_the_one_the_compose_files_mount(compose_file):
+    compose = yaml.safe_load(compose_file.read_text())
+    assert BUILD_CACHE_VOLUME in compose["volumes"]
+    assert f"{BUILD_CACHE_VOLUME}:/root/.cache" in compose["services"]["airflow"]["volumes"]
+
+
+@pytest.mark.parametrize(
+    ("preserve_volumes", "cleanup_build_cache", "expected"),
+    [
+        (False, False, ["database", "deleted-worktree-cache"]),
+        (False, True, ["database", "cache", "deleted-worktree-cache"]),
+        (True, False, []),
+        (True, True, ["cache", "deleted-worktree-cache"]),
+    ],
+)
+def test_down_keeps_build_cache_volume_until_asked(
+    docker_resources, tmp_path, preserve_volumes, cleanup_build_cache, expected
+):
+    resources, run = docker_resources
+    cache = {"com.docker.compose.project": "breeze", "com.docker.compose.volume": "airflow-cache-volume"}
+    resources["volume"] = [
+        {
+            "Name": "database",
+            "Labels": {
+                "com.docker.compose.project": "breeze",
+                "com.docker.compose.volume": "root-airflow-volume",
+            },
+        },
+        {"Name": "cache", "Labels": cache},
+        {
+            "Name": "deleted-worktree-cache",
+            "Labels": {
+                **cache,
+                "org.apache.airflow.breeze": "true",
+                "org.apache.airflow.breeze.worktree": str(tmp_path / "deleted"),
+            },
+        },
+    ]
+
+    docker_command_utils.bring_compose_projects_down(
+        preserve_volumes=preserve_volumes, cleanup_build_cache=cleanup_build_cache
+    )
+
+    removals = [c.args[0] for c in run.call_args_list if c.args[0][2] == "rm"]
+    assert removals == ([["docker", "volume", "rm", *expected]] if expected else [])
+
+
+@mock.patch("airflow_breeze.utils.docker_command_utils.get_host_id", autospec=True, return_value="this-host")
+def test_down_keeps_build_cache_volume_of_another_host(_host_id, docker_resources, tmp_path):
+    resources, run = docker_resources
+    resources["volume"] = [
+        {
+            "Name": "remote-cache",
+            "Labels": {
+                "com.docker.compose.project": "breeze-remote",
+                "com.docker.compose.volume": BUILD_CACHE_VOLUME,
+                "org.apache.airflow.breeze": "true",
+                "org.apache.airflow.breeze.worktree": str(tmp_path / "remote"),
+                "org.apache.airflow.breeze.host": "other-host",
+            },
+        },
+    ]
+
+    docker_command_utils.bring_compose_projects_down(all_worktrees=True)
+
+    assert [c.args[0] for c in run.call_args_list if c.args[0][2] == "rm"] == []
+
+
 @pytest.mark.parametrize("failed_action", ["ls", "inspect", "stop", "rm"])
 def test_startup_cleanup_continues_after_docker_failures(docker_resources, tmp_path, capsys, failed_action):
     resources, run = docker_resources