Extract registry cleanup from BakeryConfig

Move clean_caches() and clean_temporary() out of config/config.py into
posit_bakery/registry_management/clean.py. Both functions now accept a
list of ImageTarget objects instead of reading from self.targets.

config/config.py drops its last imports of ghcr, timedelta,
ImageBuildStrategy, python_on_whales, and DockerException. It no
longer depends on registry_management or build internals.

- cli/clean.py calls the new standalone functions
- Test mocks updated to patch the new function locations
- _merge_sequential_build_metadata_files test removed (logic was
  inlined into build_targets in previous commit)
- Build retry test updated to import from image.build

Author: bschwedler

posit-bakery/posit_bakery/cli/clean.py

@@ -11,6 +11,8 @@
 from posit_bakery.config.config import BakerySettings, BakeryConfigFilter
 from posit_bakery.config.image.posit_product.const import ReleaseStreamEnum
 from posit_bakery.const import DevVersionInclusionEnum, MatrixVersionInclusionEnum
+from posit_bakery.registry_management.clean import clean_caches as do_clean_caches
+from posit_bakery.registry_management.clean import clean_temporary as do_clean_temporary
 from posit_bakery.util import auto_path
 
 app = typer.Typer()
@@ -106,7 +108,8 @@ def cache_registry(
 
     log.info(f"Cleaning cache registries in {registry}")
 
-    errors = config.clean_caches(
+    errors = do_clean_caches(
+        targets=config.targets,
         remove_untagged=untagged,
         remove_older_than=timedelta(days=older_than) if older_than else None,
         dry_run=dry_run,
@@ -199,7 +202,8 @@ def temp_registry(
 
     log.info(f"Cleaning temporary registries in {registry}")
 
-    errors = config.clean_temporary(
+    errors = do_clean_temporary(
+        targets=config.targets,
         remove_untagged=untagged,
         remove_older_than=timedelta(days=older_than) if older_than else None,
         dry_run=dry_run,

posit-bakery/posit_bakery/config/config.py

@@ -5,7 +5,6 @@
 import os
 import re
 import shutil
-from datetime import timedelta
 from pathlib import Path
 from typing import Annotated, Self, Any
 
@@ -33,8 +32,7 @@
     BakeryRenderErrorGroup,
 )
 from posit_bakery.image.image_metadata import MetadataFile
-from posit_bakery.image.image_target import ImageTarget, ImageBuildStrategy, ImageTargetSettings
-from posit_bakery.registry_management import ghcr
+from posit_bakery.image.image_target import ImageTarget, ImageTargetSettings
 
 log = logging.getLogger(__name__)
 
@@ -895,57 +893,3 @@ def load_build_metadata_from_file(self, metadata_file: Path) -> list[str]:
                 log.info(f"Loaded build metadata for target '{target}' from file '{metadata_file.filepath}'.")
 
         return targets_loaded
-
-    def clean_caches(
-        self,
-        remove_untagged: bool = True,
-        remove_older_than: timedelta | None = None,
-        dry_run: bool = False,
-    ):
-        """Cleans up dangling caches in the specified registry for all generated image targets.
-
-        :param remove_untagged: If True, remove untagged caches.
-        :param remove_older_than: Optional timedelta to remove caches older than the specified duration.
-        :param dry_run: If True, print what would be deleted without actually deleting anything.
-        """
-        target_caches = list(set([cn.split(":")[0] for target in self.targets if (cn := target.cache_name())]))
-
-        errors = []
-        for target_cache in target_caches:
-            errors.extend(
-                ghcr.clean_temporary_artifacts(
-                    ghcr_registry=target_cache,
-                    remove_untagged=remove_untagged,
-                    remove_older_than=remove_older_than,
-                    dry_run=dry_run,
-                )
-            )
-
-        return errors
-
-    def clean_temporary(
-        self,
-        remove_untagged: bool = True,
-        remove_older_than: timedelta | None = None,
-        dry_run: bool = False,
-    ):
-        """Cleans up temporary images in the specified registry for all generated image targets.
-
-        :param remove_untagged: If True, remove untagged images.
-        :param remove_older_than: Optional timedelta to remove images older than the specified duration.
-        :param dry_run: If True, print what would be deleted without actually deleting anything.
-        """
-        target_caches = list(set([target.temp_name for target in self.targets]))
-
-        errors = []
-        for target_cache in target_caches:
-            errors.extend(
-                ghcr.clean_temporary_artifacts(
-                    ghcr_registry=target_cache,
-                    remove_untagged=remove_untagged,
-                    remove_older_than=remove_older_than,
-                    dry_run=dry_run,
-                )
-            )
-
-        return errors

posit-bakery/posit_bakery/registry_management/clean.py

@@ -0,0 +1,67 @@
+import logging
+from datetime import timedelta
+
+from posit_bakery.image.image_target import ImageTarget
+from posit_bakery.registry_management import ghcr
+
+log = logging.getLogger(__name__)
+
+
+def clean_caches(
+    targets: list[ImageTarget],
+    remove_untagged: bool = True,
+    remove_older_than: timedelta | None = None,
+    dry_run: bool = False,
+) -> list[Exception]:
+    """Clean up dangling caches in the registry for the given image targets.
+
+    :param targets: Image targets whose cache registries should be cleaned.
+    :param remove_untagged: If True, remove untagged caches.
+    :param remove_older_than: Remove caches older than the specified duration.
+    :param dry_run: If True, print what would be deleted without deleting.
+    :return: List of errors encountered during cleanup.
+    """
+    target_caches = list(set([cn.split(":")[0] for target in targets if (cn := target.cache_name())]))
+
+    errors = []
+    for target_cache in target_caches:
+        errors.extend(
+            ghcr.clean_temporary_artifacts(
+                ghcr_registry=target_cache,
+                remove_untagged=remove_untagged,
+                remove_older_than=remove_older_than,
+                dry_run=dry_run,
+            )
+        )
+
+    return errors
+
+
+def clean_temporary(
+    targets: list[ImageTarget],
+    remove_untagged: bool = True,
+    remove_older_than: timedelta | None = None,
+    dry_run: bool = False,
+) -> list[Exception]:
+    """Clean up temporary images in the registry for the given image targets.
+
+    :param targets: Image targets whose temporary registries should be cleaned.
+    :param remove_untagged: If True, remove untagged images.
+    :param remove_older_than: Remove images older than the specified duration.
+    :param dry_run: If True, print what would be deleted without deleting.
+    :return: List of errors encountered during cleanup.
+    """
+    target_caches = list(set([target.temp_name for target in targets]))
+
+    errors = []
+    for target_cache in target_caches:
+        errors.extend(
+            ghcr.clean_temporary_artifacts(
+                ghcr_registry=target_cache,
+                remove_untagged=remove_untagged,
+                remove_older_than=remove_older_than,
+                dry_run=dry_run,
+            )
+        )
+
+    return errors

posit-bakery/test/cli/test_clean.py

@@ -31,11 +31,13 @@
 
 @pytest.fixture
 def mock_config():
-    """Mock BakeryConfig to capture settings without making API calls."""
-    with patch("posit_bakery.cli.clean.BakeryConfig") as mock:
+    """Mock BakeryConfig and clean functions to capture settings without making API calls."""
+    with (
+        patch("posit_bakery.cli.clean.BakeryConfig") as mock,
+        patch("posit_bakery.cli.clean.do_clean_caches", return_value=[]),
+        patch("posit_bakery.cli.clean.do_clean_temporary", return_value=[]),
+    ):
         instance = MagicMock()
-        instance.clean_caches.return_value = []
-        instance.clean_temporary.return_value = []
         mock.from_context.return_value = instance
         yield mock
 

posit-bakery/test/config/test_build_retry.py

@@ -5,7 +5,7 @@
 import pytest
 from python_on_whales import DockerException
 
-from posit_bakery.config.config import _retry_build, _RETRY_DELAY_SECONDS
+from posit_bakery.image.build import _retry_build, _RETRY_DELAY_SECONDS
 from posit_bakery.error import BakeryFileError, BakeryToolRuntimeError
 
 pytestmark = [
@@ -25,7 +25,7 @@ def test_successful_build_no_retry_needed(self):
 
         mock_fn.assert_called_once()
 
-    @patch("posit_bakery.config.config.time.sleep")
+    @patch("posit_bakery.image.build.time.sleep")
     def test_retry_on_docker_exception_then_success(self, mock_sleep):
         """Test that DockerException triggers retry and succeeds on second attempt."""
         mock_fn = MagicMock(side_effect=[DockerException(["docker", "build"], 1), None])
@@ -35,7 +35,7 @@ def test_retry_on_docker_exception_then_success(self, mock_sleep):
         assert mock_fn.call_count == 2
         mock_sleep.assert_called_once_with(_RETRY_DELAY_SECONDS)
 
-    @patch("posit_bakery.config.config.time.sleep")
+    @patch("posit_bakery.image.build.time.sleep")
     def test_retry_on_bakery_tool_runtime_error_then_success(self, mock_sleep):
         """Test that BakeryToolRuntimeError triggers retry and succeeds on second attempt."""
         mock_fn = MagicMock(side_effect=[BakeryToolRuntimeError("Build failed", cmd=["docker", "build"]), None])
@@ -45,7 +45,7 @@ def test_retry_on_bakery_tool_runtime_error_then_success(self, mock_sleep):
         assert mock_fn.call_count == 2
         mock_sleep.assert_called_once_with(_RETRY_DELAY_SECONDS)
 
-    @patch("posit_bakery.config.config.time.sleep")
+    @patch("posit_bakery.image.build.time.sleep")
     def test_all_retries_exhausted_raises_exception(self, mock_sleep):
         """Test that exception is raised when all retries are exhausted."""
         error = DockerException(["docker", "build"], 1)
@@ -68,7 +68,7 @@ def test_bakery_file_error_not_retried(self):
         # Should only be called once, no retries
         mock_fn.assert_called_once()
 
-    @patch("posit_bakery.config.config.time.sleep")
+    @patch("posit_bakery.image.build.time.sleep")
     def test_retry_zero_means_no_retries(self, mock_sleep):
         """Test that retry=0 means no retries, just one attempt."""
         error = DockerException(["docker", "build"], 1)
@@ -80,7 +80,7 @@ def test_retry_zero_means_no_retries(self, mock_sleep):
         mock_fn.assert_called_once()
         mock_sleep.assert_not_called()
 
-    @patch("posit_bakery.config.config.time.sleep")
+    @patch("posit_bakery.image.build.time.sleep")
     def test_multiple_retries_then_success(self, mock_sleep):
         """Test multiple failures before eventual success."""
         mock_fn = MagicMock(
@@ -96,7 +96,7 @@ def test_multiple_retries_then_success(self, mock_sleep):
         assert mock_fn.call_count == 3
         assert mock_sleep.call_count == 2
 
-    @patch("posit_bakery.config.config.time.sleep")
+    @patch("posit_bakery.image.build.time.sleep")
     def test_logs_warning_on_retry(self, mock_sleep, caplog):
         """Test that a warning is logged when retrying."""
         import logging

posit-bakery/test/config/test_config.py

@@ -1733,19 +1733,6 @@ def test_remove_version_does_not_exist(self, get_tmpcontext):
         with pytest.raises(ValueError, match=f"Version 'non-existent' does not exist for image '{image_name}'"):
             config.remove_version(image_name, "non-existent")
 
-    def test__merge_sequential_build_metadata_files(self, get_config_obj):
-        """Test merging sequential build metadata files."""
-        config = get_config_obj("basic")
-        for target in config.targets:
-            metadata_filepath = CONFIG_TESTDATA_DIR / "build_metadata" / f"{target.uid}.json"
-            target.build_metadata.append(BuildMetadata.model_validate_json(metadata_filepath.read_text()))
-
-        merged_metadata = config._merge_sequential_build_metadata_files()
-        with open(CONFIG_TESTDATA_DIR / "build_metadata" / "expected.json", "r") as f:
-            expected_metadata = json.load(f)
-
-        assert merged_metadata == expected_metadata
-
     def test_load_build_metadata_file(self, get_config_obj):
         """Test loading a build metadata file."""
         metadata_filepath = CONFIG_TESTDATA_DIR / "build_metadata" / "expected.json"
@@ -1807,7 +1794,10 @@ def test_clean_caches(
         mock_ghcr_client_instance.get_package_versions.return_value = cache_ghcr_package_versions_data
 
         # Clean caches
-        config.clean_caches(
+        from posit_bakery.registry_management.clean import clean_caches
+
+        clean_caches(
+            targets=config.targets,
             remove_untagged=untagged,
             remove_older_than=timedelta(days=older_than_days) if older_than_days is not None else None,
         )
@@ -1839,7 +1829,10 @@ def test_clean_caches_dry_run(
         mock_ghcr_client_instance.get_package_versions.return_value = cache_ghcr_package_versions_data
 
         # Clean caches
-        config.clean_caches(
+        from posit_bakery.registry_management.clean import clean_caches
+
+        clean_caches(
+            targets=config.targets,
             remove_untagged=True,
             remove_older_than=timedelta(days=14),
             dry_run=True,
@@ -1898,7 +1891,10 @@ def test_clean_temporary(
         mock_ghcr_client_instance.get_package_versions.return_value = temp_ghcr_package_versions_data
 
         # Clean temp images
-        config.clean_temporary(
+        from posit_bakery.registry_management.clean import clean_temporary
+
+        clean_temporary(
+            targets=config.targets,
             remove_untagged=untagged,
             remove_older_than=timedelta(days=older_than_days) if older_than_days is not None else None,
         )
@@ -1930,7 +1926,10 @@ def test_clean_temporary_dry_run(
         mock_ghcr_client_instance.get_package_versions.return_value = temp_ghcr_package_versions_data
 
         # Clean temp images
-        config.clean_temporary(
+        from posit_bakery.registry_management.clean import clean_temporary
+
+        clean_temporary(
+            targets=config.targets,
             remove_untagged=True,
             remove_older_than=timedelta(days=14),
             dry_run=True,