diff --git a/cli/administration/deploy/development/common.py b/cli/administration/deploy/development/common.py index 6271e88d6f..7b6b1a1d95 100644 --- a/cli/administration/deploy/development/common.py +++ b/cli/administration/deploy/development/common.py @@ -1,17 +1,21 @@ +"""Shared helpers for the development deploy CLI. + +``DEV_INVENTORY_VARS_FILE`` mirrors ``INFINITO_INVENTORY_VARS_FILE`` +from ``default.env``; the pairing is drift-guarded. +""" + from __future__ import annotations import os from typing import TYPE_CHECKING from . import PROJECT_ROOT -from .deps import apps_with_deps, resolve_run_after from .env import resolve_distro if TYPE_CHECKING: from .compose import Compose -# Mirrors INFINITO_INVENTORY_VARS_FILE from default.env; drift-guarded. DEV_INVENTORY_VARS_FILE: str = ( os.environ.get("INFINITO_INVENTORY_VARS_FILE") or "inventories/development/default.yml" @@ -33,22 +37,5 @@ def make_compose() -> Compose: from .compose import Compose distro = resolve_distro() - # Surface env-script gap here rather than as a cryptic compose error later. resolve_container() return Compose(repo_root=PROJECT_ROOT, distro=distro) - - -def resolve_deploy_ids_for_app(compose: Compose, app_id: str) -> list[str]: - deps = resolve_run_after(compose, app_id) - return apps_with_deps(app_id, deps_role_names=deps) - - -def resolve_deploy_ids_for_apps(compose: Compose, app_spec: str) -> list[str]: - """Resolve deploy ids for one or more space- or comma-separated app ids.""" - app_ids = [a.strip() for a in app_spec.replace(",", " ").split() if a.strip()] - result: list[str] = [] - for app_id in app_ids: - for dep in resolve_deploy_ids_for_app(compose, app_id): - if dep not in result: - result.append(dep) - return result diff --git a/cli/administration/deploy/development/deps.py b/cli/administration/deploy/development/deps.py deleted file mode 100644 index 4f8456389d..0000000000 --- a/cli/administration/deploy/development/deps.py +++ /dev/null @@ -1,63 +0,0 @@ -from __future__ import annotations - -import os -from typing import TYPE_CHECKING - -if TYPE_CHECKING: - # Import only for type checking to avoid runtime import cycles. - from .compose import Compose - - -def resolve_run_after( - compose: Compose, - role_name: str, - *, - services_overrides_container_path: str | None = None, -) -> list[str]: - """ - Calls resolver inside the infinito container: - python -m cli.meta.roles.applications.resolution.combined - [--services-overrides ] - - `services_overrides_container_path` is the in-container path to a - JSON file `{role_name: services_map}` whose entries replace each - listed role's `meta/services.yml` for the resolver. Callers - (variant-aware planner) write the file to a path that is shared - with the container before calling. - """ - cmd = [ - "python3", - "-m", - "cli.meta.roles.applications.resolution.combined", - role_name, - ] - if services_overrides_container_path: - cmd.extend(["--services-overrides", services_overrides_container_path]) - r = compose.exec( - cmd, check=False, workdir=os.environ["INFINITO_SRC_DIR"], capture=True - ) - - if r.returncode != 0: - raise RuntimeError( - f"resolution.combined failed for {role_name} (rc={r.returncode})\n" - f"STDOUT:\n{r.stdout}\nSTDERR:\n{r.stderr}" - ) - - txt = (r.stdout or "").strip() - if not txt: - return [] - return [t for t in txt.split() if t.strip()] - - -def apps_with_deps(app_id: str, deps_role_names: list[str]) -> list[str]: - """ - The resolver returns role folder names. In your current inventory groups, - group names == application_ids (folder names). So we can include them directly. - """ - out: list[str] = [] - for d in deps_role_names: - if d != app_id and d not in out: - out.append(d) - if app_id not in out: - out.append(app_id) - return out diff --git a/cli/administration/deploy/development/inventory/legacy_resolver.py b/cli/administration/deploy/development/inventory/legacy_resolver.py index 94e117cfc0..855e79b99f 100644 --- a/cli/administration/deploy/development/inventory/legacy_resolver.py +++ b/cli/administration/deploy/development/inventory/legacy_resolver.py @@ -1,11 +1,13 @@ """Default (non-variant) include resolution for the matrix planner. When NO primary app ships a `meta/variants.yml`, the planner falls back -to this path — the `CombinedResolver` walks both `run_after` and +to this path — the `CombinedResolver` walks `dependencies` and `shared:`-edges in the variant-merged services map and Jinja flags are -evaluated at deploy time against `group_names`. This module owns the -two helpers that path needs and nothing else; the variant-only path -lives in `.variants`. +evaluated at deploy time against `group_names`. `run_after` is NOT +followed for inclusion (it is a pure ordering hint), so a variant that +disables a service does not get the provider re-added through the +ordering edge. This module owns the two helpers that path needs and +nothing else; the variant-only path lives in `.variants`. """ from __future__ import annotations @@ -82,14 +84,18 @@ def _resolve_round_include( round's variant-merged services map. Returns the full include set in stable order (deps first per primary, primary last; primaries iterated in the user-provided order). + + The resolver import is deferred so the host-side import surface + stays lean for callers that never need the variant-aware planner + (e.g. lint/validation). """ - # Late import keeps the host-side import surface lean for callers - # that never need the variant-aware planner (e.g. lint/validation). from cli.meta.roles.applications.resolution.combined.resolver import ( CombinedResolver, ) - resolver = CombinedResolver(services_overrides=services_overrides) + resolver = CombinedResolver( + services_overrides=services_overrides, follow_run_after=False + ) out: list[str] = [] seen: set[str] = set() for app_id in primary_apps: diff --git a/cli/meta/roles/applications/resolution/combined/resolver.py b/cli/meta/roles/applications/resolution/combined/resolver.py index 85034d2f23..ca7ea73b75 100644 --- a/cli/meta/roles/applications/resolution/combined/resolver.py +++ b/cli/meta/roles/applications/resolution/combined/resolver.py @@ -23,7 +23,10 @@ class CombinedResolver: prerequisites(role) = run_after(role) + dependencies(role) + services(role) Notes: - - run_after edges are always followed + - run_after edges are followed only when `follow_run_after` is True. + Inclusion callers pass False so run_after stays a pure ordering hint + (its documented contract) and a variant that disables a service does + not get its provider re-added through the ordering edge. - dependency edges are followed only for application roles (filtered in loader) - services edges are derived from app config flags (filtered in loader) - Cycles do NOT raise; traversal stops expanding the cyclic edge @@ -40,9 +43,12 @@ class CombinedResolver: def __init__( self, services_overrides: dict[str, dict] | None = None, + *, + follow_run_after: bool = True, ) -> None: self._cache: dict[str, RoleEdges] = {} self._services_overrides: dict[str, dict] = dict(services_overrides or {}) + self._follow_run_after = follow_run_after def edges_for(self, role_name: str) -> RoleEdges: if role_name in self._cache: @@ -57,7 +63,6 @@ class CombinedResolver: services_override=self._services_overrides.get(role_name), ) - # Validate referenced roles exist for run_after (deps/services validate internally too) for r in ra: require_role_exists(r) @@ -80,7 +85,6 @@ class CombinedResolver: def dfs(node: str) -> None: if node in stack: - # cycle edge -> stop expansion, do not raise return if node in visited: return @@ -90,9 +94,9 @@ class CombinedResolver: edges = self.edges_for(node) - # Keep stable traversal order - for dep in edges.run_after: - dfs(dep) + if self._follow_run_after: + for dep in edges.run_after: + dfs(dep) for dep in edges.dependencies: dfs(dep) for dep in edges.services: @@ -105,9 +109,3 @@ class CombinedResolver: dfs(start_role) return out - - def resolve_with_edges( - self, start_role: str - ) -> tuple[list[str], dict[str, RoleEdges]]: - resolved = self.resolve(start_role) - return resolved, dict(self._cache) diff --git a/docs/contributing/design/inventory.md b/docs/contributing/design/inventory.md index 8214d474ab..2fce269906 100644 --- a/docs/contributing/design/inventory.md +++ b/docs/contributing/design/inventory.md @@ -18,15 +18,14 @@ After stage 3 the inventory is fully variant-resolved on disk. Ansible loads it ## What decides which roles enter the inventory đŸŽ¯ -The include set passed to stage 2 is built by the deploy CLI in [deploy.py](../../../cli/administration/deploy/development/deploy.py) via `resolve_deploy_ids_for_apps`. It expands the operator's `--apps` selection along three transitive paths, all walked together: +The include set passed to stage 2 is built per round by the matrix planner: [deploy.py](../../../cli/administration/deploy/development/deploy.py) and [init.py](../../../cli/administration/deploy/development/init.py) call `plan_dev_inventory_matrix` in [planner.py](../../../cli/administration/deploy/development/inventory/planner.py), which resolves each round's include via `_resolve_round_include` in [legacy_resolver.py](../../../cli/administration/deploy/development/inventory/legacy_resolver.py) using the [CombinedResolver](../../../cli/meta/roles/applications/resolution/combined/resolver.py) with `follow_run_after=False`. The swarm test path computes its include separately via [derive_includes.py](../../../utils/tests/swarm/derive_includes.py), reading the active variant map from `INFINITO_APP_VARIANTS`. Both expand the selection along two transitive paths: 1. **Ansible meta `dependencies:`** from each role's `meta/main.yml`. -2. **Per-role `run_after:`** from each role's `meta/services.yml..run_after`. -3. **Shared-service auto-include** from each role's `meta/services.yml..` block, when both `enabled is True` AND `shared is True`. The check lives in [service_registry.py](../../../utils/roles/applications/services/registry.py) `resolve_service_dependency_roles_from_config`. +2. **Shared-service auto-include** from each role's `meta/services.yml..` block, when both `enabled` AND `shared` pass `is_explicit_truth` in [registry.py](../../../utils/roles/applications/services/registry.py): the literal `True` or the dynamic form `"{{ '' in group_names }}"`. A literal `false` (as baked by a variant override) does NOT qualify. -The auto-include uses **strict identity** with Python's `True`. A Jinja-templated string such as `"{{ 'web-app-discourse' in group_names }}"` is NOT the value `True`, so it does NOT trigger auto-include at build time. Only literal booleans (or boolean-typed variant overrides that bake one in) qualify. +`run_after:` from `meta/services.yml..run_after` is an ordering hint only (see [base.md](role/services/base.md)); it MUST NOT pull roles into the include set. -This is the mechanism that lets variants control whether a shared service is pulled in: a variant override `services..enabled: true` pins a real boolean; the build-time auto-include picks it up; the dependent role's group materialises in the inventory. +This is the mechanism that lets variants control whether a shared service is pulled in: a variant override pins `services..{enabled,shared}` to a literal boolean; the round's include resolution reads the variant-merged services map; the provider role's group materialises in (or drops out of) the inventory accordingly. ## What lives in the inventory file vs. host_vars 📋 diff --git a/tests/unit/cli/meta/roles/applications/resolution/combined/test_resolver.py b/tests/unit/cli/meta/roles/applications/resolution/combined/test_resolver.py index 2d3bff3a0f..94ec3aaac8 100644 --- a/tests/unit/cli/meta/roles/applications/resolution/combined/test_resolver.py +++ b/tests/unit/cli/meta/roles/applications/resolution/combined/test_resolver.py @@ -9,6 +9,7 @@ from pathlib import Path from unittest.mock import patch from cli.meta.roles.applications.resolution.combined import repo_paths +from cli.meta.roles.applications.resolution.combined.resolver import CombinedResolver from cli.meta.roles.applications.resolution.combined.tree import print_tree from utils.roles.mapping import ROLE_FILE_META_SERVICES, ROLE_FILE_VARS_MAIN @@ -32,10 +33,6 @@ class TestCombinedTree(unittest.TestCase): root = Path(td) (root / "roles").mkdir() - # start app role with dashboard enabled. - # Per run_after lives at - # meta/services.yml..run_after. - # For "start" (no category prefix) the entity name = role name. _write( root / "roles" / "start" / ROLE_FILE_VARS_MAIN, "application_id: start\n", @@ -52,14 +49,11 @@ class TestCombinedTree(unittest.TestCase): ), ) - # keycloak exists, and points back to start to force a visible cycle - # entity name for web-app-keycloak is "keycloak" _write( root / "roles" / "web-app-keycloak" / ROLE_FILE_META_SERVICES, "keycloak:\n run_after:\n - start\n", ) - # required folders exist (root / "roles" / "start").mkdir(parents=True, exist_ok=True) (root / "roles" / "web-app-keycloak").mkdir(parents=True, exist_ok=True) (root / "roles" / "web-app-dashboard").mkdir(parents=True, exist_ok=True) @@ -75,5 +69,34 @@ class TestCombinedTree(unittest.TestCase): self.assertIn("↩ī¸Ž (cycle)", out) +class TestFollowRunAfter(unittest.TestCase): + def test_run_after_only_role_gated_by_flag(self) -> None: + """A role reachable ONLY via run_after (no dependency/service edge) + is included when run_after is followed and excluded when it is not.""" + with tempfile.TemporaryDirectory() as td: + root = Path(td) + (root / "roles").mkdir() + _write( + root / "roles" / "start" / ROLE_FILE_VARS_MAIN, + "application_id: start\n", + ) + _write( + root / "roles" / "start" / ROLE_FILE_META_SERVICES, + "start:\n run_after:\n - web-app-extra\n", + ) + (root / "roles" / "web-app-extra").mkdir(parents=True, exist_ok=True) + _write( + root / "roles" / "web-app-extra" / ROLE_FILE_META_SERVICES, + "extra: {}\n", + ) + + with patch.object(repo_paths, "PROJECT_ROOT", root): + followed = CombinedResolver(follow_run_after=True).resolve("start") + gated = CombinedResolver(follow_run_after=False).resolve("start") + + self.assertIn("web-app-extra", followed) + self.assertNotIn("web-app-extra", gated) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/utils/tests/swarm/test_derive_includes.py b/tests/unit/utils/tests/swarm/test_derive_includes.py new file mode 100644 index 0000000000..78233c2d19 --- /dev/null +++ b/tests/unit/utils/tests/swarm/test_derive_includes.py @@ -0,0 +1,110 @@ +from __future__ import annotations + +import unittest +import unittest.mock + +from utils.tests.swarm import derive_includes as mod + + +class TestActiveVariantMap(unittest.TestCase): + def _with_env(self, value): + return unittest.mock.patch.dict( + mod.os.environ, {"INFINITO_APP_VARIANTS": value}, clear=False + ) + + def test_absent_env_returns_empty(self): + with unittest.mock.patch.dict(mod.os.environ, {}, clear=True): + self.assertEqual(mod._active_variant_map(), {}) + + def test_full_map_read(self): + with self._with_env('{"web-app-x": 1, "web-app-y": 0}'): + self.assertEqual( + mod._active_variant_map(), {"web-app-x": 1, "web-app-y": 0} + ) + + def test_malformed_json_returns_empty(self): + with self._with_env("not-json"): + self.assertEqual(mod._active_variant_map(), {}) + + def test_non_dict_returns_empty(self): + with self._with_env("[1, 2]"): + self.assertEqual(mod._active_variant_map(), {}) + + def test_non_int_and_bool_indices_dropped(self): + with self._with_env('{"web-app-x": "1", "web-app-y": true, "web-app-z": 2}'): + self.assertEqual(mod._active_variant_map(), {"web-app-z": 2}) + + +class TestApplicationsForActiveVariants(unittest.TestCase): + def setUp(self): + self.base = { + "web-app-x": {"services": {"sso": {"enabled": "{{ 'k' in group_names }}"}}}, + "web-app-k": {"services": {}}, + } + + def _map(self, value): + return unittest.mock.patch.object( + mod, "_active_variant_map", return_value=value + ) + + def test_empty_map_keeps_base(self): + with self._map({}): + self.assertIs( + mod._applications_for_active_variants(self.base), + self.base, + ) + + def test_variant_zero_is_swapped(self): + """Variant 0 MAY disable flags the base's dynamic-Jinja form counts + as enabled (web-app-nextcloud), so index 0 must swap too.""" + v0 = {"services": {"sso": {"enabled": False}}} + with ( + self._map({"web-app-x": 0}), + unittest.mock.patch.object( + mod, "get_variants", return_value={"web-app-x": [v0]} + ), + ): + result = mod._applications_for_active_variants(self.base) + self.assertEqual(result["web-app-x"], v0) + + def test_all_map_entries_swapped_not_only_primary(self): + vx = {"services": {"sso": {"enabled": False}}} + vk = {"services": {"postgres": {"enabled": True}}} + with ( + self._map({"web-app-x": 1, "web-app-k": 1}), + unittest.mock.patch.object( + mod, + "get_variants", + return_value={"web-app-x": [{}, vx], "web-app-k": [{}, vk]}, + ), + ): + result = mod._applications_for_active_variants(self.base) + self.assertEqual(result["web-app-x"], vx) + self.assertEqual(result["web-app-k"], vk) + + def test_out_of_range_entry_keeps_base_others_swap(self): + vk = {"services": {"postgres": {"enabled": True}}} + with ( + self._map({"web-app-x": 9, "web-app-k": 1}), + unittest.mock.patch.object( + mod, + "get_variants", + return_value={"web-app-x": [{}], "web-app-k": [{}, vk]}, + ), + ): + result = mod._applications_for_active_variants(self.base) + self.assertIs(result["web-app-x"], self.base["web-app-x"]) + self.assertEqual(result["web-app-k"], vk) + + def test_unknown_app_in_map_ignored(self): + with ( + self._map({"web-app-ghost": 1}), + unittest.mock.patch.object(mod, "get_variants", return_value={}), + ): + result = mod._applications_for_active_variants(self.base) + self.assertEqual(result, self.base) + self.assertNotIn("web-app-ghost", result) + + +if __name__ == "__main__": + unittest.main() diff --git a/utils/tests/swarm/derive_includes.py b/utils/tests/swarm/derive_includes.py index 011a6a50f9..a2d342202a 100755 --- a/utils/tests/swarm/derive_includes.py +++ b/utils/tests/swarm/derive_includes.py @@ -10,17 +10,26 @@ locally-built images, but not declared as a meta-dep so it stays out of compose-mode deploys) are added explicitly because the dep-walker only knows roles that are in the ``applications`` dict. -Input (env): ``APP_ID``. +Input (env): ``APP_ID``. Optional ``INFINITO_APP_VARIANTS`` (JSON +``{app_id: variant_index}``, set per round by the matrix orchestrator) +selects the active variant so a variant that pins ``services.*`` flags +to ``false`` prunes those providers from the closure instead of the +provision step pulling them in from the variant-free base config. """ from __future__ import annotations +import json import os import sys +from typing import Any from utils import PROJECT_ROOT -from utils.cache.applications import get_merged_applications +from utils.cache.applications import get_merged_applications, get_variants from utils.roles.applications.in_group_deps import applications_if_group_and_all_deps +from utils.roles.applications.services.registry import ( + build_service_registry_from_applications, +) _ROLES_DIR = PROJECT_ROOT / "roles" @@ -31,13 +40,63 @@ _EXPLICIT_INCLUDES: tuple[str, ...] = ( ) +def _active_variant_map() -> dict[str, int]: + raw = os.environ.get("INFINITO_APP_VARIANTS") + if not raw: + return {} + try: + mapping = json.loads(raw) + except json.JSONDecodeError: + return {} + if not isinstance(mapping, dict): + return {} + return { + app_id: index + for app_id, index in mapping.items() + if isinstance(app_id, str) + and isinstance(index, int) + and not isinstance(index, bool) + } + + +def _applications_for_active_variants( + base_applications: dict[str, Any], +) -> dict[str, Any]: + """Swap every app listed in ``INFINITO_APP_VARIANTS`` to its variant + config. Index 0 is swapped too: a variant-0 override MAY disable flags + the base config's dynamic-Jinja form counts as enabled (web-app-nextcloud + pins 11 partner services off in variant 0), so "variant 0 == base" does + not hold. Every entry in the map is swapped, not just the primary app, + because host_vars baking honours the dep roles' indices as well and the + include closure must match that topology.""" + overrides = _active_variant_map() + if not overrides: + return base_applications + variants_per_app = get_variants(roles_dir=str(_ROLES_DIR)) + applications = dict(base_applications) + for app_id, index in overrides.items(): + app_variants = variants_per_app.get(app_id) or [] + if 0 <= index < len(app_variants): + applications[app_id] = app_variants[index] + return applications + + def derive_includes(app_id: str) -> list[str]: - applications = get_merged_applications(roles_dir=str(_ROLES_DIR)) + """Resolve APP_ID's transitive include set under the active variants. + + The service registry (service key -> provider role) stays built from + the full rendered base set so provider discovery is unaffected by the + active variants; only the dep-walk sees the variant configs. + """ + base_applications = get_merged_applications(roles_dir=str(_ROLES_DIR)) + service_registry = build_service_registry_from_applications(base_applications) + applications = _applications_for_active_variants(base_applications) transitive = applications_if_group_and_all_deps( applications, [app_id], project_root=str(PROJECT_ROOT), roles_dir=str(_ROLES_DIR), + service_registry=service_registry, ) found = set(transitive) | set(_EXPLICIT_INCLUDES) | {app_id} return sorted(found)