diff --git a/tests/unit/python/utils/cache/test_variants.py b/tests/unit/python/utils/cache/test_variants.py index 5ec5ff7b40..5af77bc90f 100644 --- a/tests/unit/python/utils/cache/test_variants.py +++ b/tests/unit/python/utils/cache/test_variants.py @@ -294,5 +294,41 @@ class TestMergedApplicationsUsesBaseConfig(unittest.TestCase): ) +class TestReadonlyVariants(unittest.TestCase): + """``readonly=True`` hands out the cached map itself. + + A sweep resolves one round per matrix row and asks for the variants in + each, so the defensive deep copy is paid hundreds of times for data + nobody writes to. The guarantee is cheap to lose silently in a later + refactor, which is why identity is asserted rather than runtime. + """ + + def setUp(self) -> None: + _reset_cache_for_tests() + self.tmp = Path(os.environ.get("TMPDIR", "/tmp")) / "readonly-variants-roles" + _write_role( + self.tmp, + "web-app-one", + config="postgres:\n enabled: true\n", + meta="- services:\n postgres:\n enabled: false\n", + ) + + def test_the_default_hands_out_a_copy(self) -> None: + first = get_variants(roles_dir=str(self.tmp)) + second = get_variants(roles_dir=str(self.tmp)) + self.assertEqual(first, second) + self.assertIsNot(first, second) + + def test_readonly_hands_out_the_cached_object(self) -> None: + first = get_variants(roles_dir=str(self.tmp), readonly=True) + second = get_variants(roles_dir=str(self.tmp), readonly=True) + self.assertIs(first, second) + + def test_a_readonly_caller_sees_what_the_copy_would_have_shown(self) -> None: + shared = get_variants(roles_dir=str(self.tmp), readonly=True) + copied = get_variants(roles_dir=str(self.tmp)) + self.assertEqual(shared, copied) + + if __name__ == "__main__": unittest.main() diff --git a/utils/cache/applications.py b/utils/cache/applications.py index 3ed88471d5..cb71789c7c 100644 --- a/utils/cache/applications.py +++ b/utils/cache/applications.py @@ -339,19 +339,28 @@ def get_application_defaults( def get_variants( - *, roles_dir: str | os.PathLike[str] | None = None + *, roles_dir: str | os.PathLike[str] | None = None, readonly: bool = False ) -> dict[str, list[Any]]: """Return ``{application_id: [variant_0, ...]}`` cached per ``roles_dir``. Each variant is the role's effective configuration after the corresponding `meta/variants.yml` override has been - deep-merged on top of `meta/services.yml`.""" + deep-merged on top of `meta/services.yml`. + + Args: + roles_dir: roles tree to read, defaulting to the project's own. + readonly: hand out the cached structure itself instead of a deep + copy. The copy is what keeps one caller's mutation out of every + later caller's data, so this is only for a caller that provably + reads: a sweep resolves hundreds of rounds, and copying the whole + map per round costs more than building it once did. + """ resolved_roles_dir = _resolve_roles_dir(roles_dir=roles_dir) key = _cache_key(resolved_roles_dir) cached = _VARIANTS_CACHE.get(key) if cached is None: cached = _build_variants(resolved_roles_dir) _VARIANTS_CACHE[key] = cached - return copy.deepcopy(cached) + return cached if readonly else copy.deepcopy(cached) def _build_variant_overrides_only( diff --git a/utils/roles/applications/variants.py b/utils/roles/applications/variants.py index 833f569178..24d0b4279c 100644 --- a/utils/roles/applications/variants.py +++ b/utils/roles/applications/variants.py @@ -147,7 +147,7 @@ def services_overrides_for_round( eliminates the topology-vs-host_vars drift that variant-blind resolution produced. """ - variants_per_app = get_variants(roles_dir=roles_dir) + variants_per_app = get_variants(roles_dir=roles_dir, readonly=True) overrides: dict[str, dict] = {} roles_path = Path(roles_dir)