diff --git a/cli/administration/deploy/development/inventory/planner.py b/cli/administration/deploy/development/inventory/planner.py index d175c694bf..5de21fe475 100644 --- a/cli/administration/deploy/development/inventory/planner.py +++ b/cli/administration/deploy/development/inventory/planner.py @@ -16,13 +16,13 @@ from __future__ import annotations from typing import TYPE_CHECKING from utils.cache.applications import get_variants -from utils.roles.applications.variants import active_variants_for_round - -from . import legacy_resolver +from utils.roles.applications.variants import resolve_round if TYPE_CHECKING: from collections.abc import Mapping, Sequence + from utils.roles.applications.variants import ResolvedRound + from .spec import PlanEntry @@ -72,7 +72,7 @@ def plan_dev_inventory_matrix( raise ValueError("plan_dev_inventory_matrix: primary_apps must not be empty") pins = dict(pinned_variants or {}) - variants_per_app = get_variants(roles_dir=roles_dir) + variants_per_app = get_variants(roles_dir=roles_dir, readonly=True) primary_variant_counts = { app_id: max(1, len(variants_per_app.get(app_id) or [{}])) for app_id in primary_apps @@ -87,7 +87,7 @@ def plan_dev_inventory_matrix( ) base = str(base_inventory_dir).rstrip("/") - per_variant_includes: list[tuple[str, ...]] = [] + resolved_rounds: list[ResolvedRound] = [] primary_round_variants_per_round: list[dict[str, int]] = [] for round_index in range(total_rounds): primary_round_variants = { @@ -97,44 +97,31 @@ def plan_dev_inventory_matrix( for app_id, count in primary_variant_counts.items() } primary_round_variants_per_round.append(primary_round_variants) - - services_overrides = legacy_resolver._build_services_overrides_for_round( - roles_dir=roles_dir, - round_index=round_index, - primary_app_variants=primary_round_variants, - ) - per_variant_includes.append( - legacy_resolver._resolve_round_include( - primary_apps=primary_apps, - services_overrides=services_overrides, + resolved_rounds.append( + resolve_round( + roles_dir=roles_dir, + round_index=round_index, + primary_app_variants=primary_round_variants, + variants_per_app=variants_per_app, ) ) seen_union: set[str] = set() union_apps: list[str] = [] - for variant_include in per_variant_includes: - for dep in variant_include: + for resolved in resolved_rounds: + for dep in resolved.include: if dep not in seen_union: seen_union.add(dep) union_apps.append(dep) purge_set: tuple[str, ...] = tuple(union_apps) plan: list[PlanEntry] = [] - for round_index in range(total_rounds): - primary_round_variants = primary_round_variants_per_round[round_index] - round_include = per_variant_includes[round_index] - + for round_index, resolved in enumerate(resolved_rounds): round_variants = { - **active_variants_for_round( - roles_dir=roles_dir, - round_index=round_index, - primary_app_variants=primary_round_variants, - variants_per_app=variants_per_app, - ), - **primary_round_variants, + **resolved.active_index, + **primary_round_variants_per_round[round_index], } - inv_dir = f"{base}-{round_index}" if total_rounds > 1 else base - plan.append((round_index, inv_dir, round_variants, round_include, purge_set)) + plan.append((round_index, inv_dir, round_variants, resolved.include, purge_set)) return plan diff --git a/cli/meta/roles/applications/resolution/affected/__main__.py b/cli/meta/roles/applications/resolution/affected/__main__.py index b834fd2eb4..55feeac633 100755 --- a/cli/meta/roles/applications/resolution/affected/__main__.py +++ b/cli/meta/roles/applications/resolution/affected/__main__.py @@ -44,11 +44,11 @@ from cli.meta.roles.applications.resolution.combined.role_introspection import ( load_run_after, ) from utils.cache.applications import get_variants -from utils.roles.applications.variants import services_overrides_for_round +from utils.roles.applications.variants import resolve_round from utils.roles.display import VARIANT_SEPARATOR if TYPE_CHECKING: - from collections.abc import Iterable, Sequence + from collections.abc import Iterable, Mapping from pathlib import Path EXIT_NON_MODELLABLE_SEED = 2 @@ -131,34 +131,42 @@ def affected_roles(changed: Iterable[str]) -> list[str]: return sorted(affected) -def _round_resolvers(roles_path: Path, rounds: int) -> list[CombinedResolver]: - """One resolver per deploy round, each walking that round's - variant-merged services maps. +def _round_resolver( + roles_path: Path, role: str, index: int, variants: Mapping[str, list] +) -> CombinedResolver: + """The resolver of the round that deploys *role* in variant *index*. - The maps are built by the same helper - (:func:`utils.roles.applications.variants.services_overrides_for_round`) - the matrix planner feeds its rounds from, so the topology this reads is - the topology round ``i`` deploys -- including every pulled-in role's own - variant, which is what carries the narrowing across generations. - ``run_after`` is not followed, for the reason the planner does not follow - it either: an ordering hint would re-add a provider the variant switched - off. + The round is the one the matrix planner deploys for that variant + (:func:`utils.roles.applications.variants.resolve_round` with *role* as + its only primary), so the topology this reads carries every variant pin + in force there and every pulled-in role's own variant, which is what + carries the narrowing across generations. ``run_after`` is not followed, + for the reason the planner does not follow it either: an ordering hint + would re-add a provider the variant switched off. + + Args: + roles_path: the roles tree. + role: the round's primary app. + index: the variant *role* runs in. + variants: parsed variants per role. """ - return [ - CombinedResolver( - services_overrides=services_overrides_for_round( - roles_dir=str(roles_path), - round_index=index, - primary_app_variants={}, - ), - follow_run_after=False, - ) - for index in range(rounds) - ] + return CombinedResolver( + services_overrides=resolve_round( + roles_dir=str(roles_path), + round_index=index, + primary_app_variants={role: index}, + variants_per_app=variants, + ).services_overrides, + follow_run_after=False, + ) def _pinned( - role: str, seeds: set[str], resolvers: Sequence[CombinedResolver], count: int + role: str, + seeds: set[str], + count: int, + roles_path: Path, + variants: Mapping[str, list], ) -> str: """*role*, narrowed to the variants whose own closure reaches a seed. @@ -172,7 +180,9 @@ def _pinned( hits = [ index for index in range(count) - if seeds.intersection(resolvers[index].resolve(role)) + if seeds.intersection( + _round_resolver(roles_path, role, index, variants).resolve(role) + ) ] except CombinedResolutionError: return role @@ -189,16 +199,12 @@ def affected_selection(changed: Iterable[str]) -> list[str]: """ seeds: set[str] = {r.strip() for r in changed if r and r.strip()} affected = affected_roles(changed) - variants = get_variants(roles_dir=str(roles_dir())) + variants = get_variants(roles_dir=str(roles_dir()), readonly=True) counts = {role: len(variants.get(role) or ()) for role in affected} - rounds = max(counts.values(), default=0) - if rounds < 2: - return affected - resolvers = _round_resolvers(roles_dir(), rounds) return [ role if role in seeds or counts[role] < 2 - else _pinned(role, seeds, resolvers, counts[role]) + else _pinned(role, seeds, counts[role], roles_dir(), variants) for role in affected ] diff --git a/docs/contributing/artefact/files/role/variants.md b/docs/contributing/artefact/files/role/variants.md index 8505fccc12..2803c84c92 100644 --- a/docs/contributing/artefact/files/role/variants.md +++ b/docs/contributing/artefact/files/role/variants.md @@ -51,6 +51,38 @@ The token follows the `role#variant` form used by the priority slots. `#reciproc The marker is a comment, so it never reaches the merged services map or `host_vars`. It applies only to the variant entry it stands in; the same role's other variants are unaffected. A primary app keeps the index the sweep assigned it and no pin overrides it. +A pin is in force only while the role carrying it is part of the round's include and runs the variant the marker stands in. A pin in a role the round does not deploy changes nothing. The round starts every role on its clamped index and recomputes the include and the pins in force until the assignment no longer changes; pins that never settle raise `VariantPinError`. [test_variant_pin_fixed_point.py](../../../../../tests/integration/roles/meta/variants/test_variant_pin_fixed_point.py) settles every round the matrix plans (each role as the only primary, in each of its variants) and fails when a round raises `VariantPinError`, or when a pin in force stands above a service entry the carrier runs with `enabled: false` in that round. Place a pin only above an entry the variant keeps enabled: the guide host `docs` stays on for the primary app alone, so a pin above `docs` fails the guard in any round that pulls the role in as a dependency running that variant. + +### How A Round Settles Its Pins 🔁 + +`resolve_round` in [variants.py](../../../../../utils/roles/applications/variants.py) runs this loop for every round; the planner, `affected` and the budget lint read its result: + +```mermaid +flowchart TD + start["Every role on its clamped index"] --> include["Resolve the round's include from the current assignment"] + include --> pins["Collect the pins of the roles in the include, at their current index"] + pins --> assign["New assignment: clamped indices, overridden by the collected pins"] + assign --> same{"Assignment unchanged?"} + same -->|yes| settled["Settled: the round deploys this include with these variants"] + same -->|no| seen{"Assignment seen before?"} + seen -->|yes| error["VariantPinError"] + seen -->|no| include +``` + +A pin of a role outside the include does not count, and neither does a pin above an entry the carrier runs switched off: + +```mermaid +flowchart LR + subgraph round["Round of the primary app"] + primary["Primary app, keeps its index"] + partner["Partner role"] + carrier["Carrier role in the include"] + end + outside["Carrier role outside the include"] + carrier -->|pin above an enabled entry: in force| partner + outside -.->|pin: not in force| partner +``` + Two roles pinning the same partner to different variants in one round raises `VariantPinConflictError` before an inventory is written. An index that no longer points at the reciprocating variant is caught by [test_variant_pins.py](../../../../../tests/integration/roles/meta/variants/test_variant_pins.py); `#reciprocal` needs exactly one candidate variant on the partner or it raises. ## Credentials Interaction 🔐 diff --git a/roles/web-app-matomo/meta/variants.yml b/roles/web-app-matomo/meta/variants.yml index 8ff5efc3dd..a38694c6f3 100644 --- a/roles/web-app-matomo/meta/variants.yml +++ b/roles/web-app-matomo/meta/variants.yml @@ -34,7 +34,6 @@ tor: enabled: true - services: - # variant-pin: web-app-docs#reciprocal docs: enabled: true shared: true diff --git a/tests/integration/roles/meta/variants/test_resource_budget.py b/tests/integration/roles/meta/variants/test_resource_budget.py index 47725f32eb..0b7d3848a8 100644 --- a/tests/integration/roles/meta/variants/test_resource_budget.py +++ b/tests/integration/roles/meta/variants/test_resource_budget.py @@ -4,11 +4,13 @@ the host memory budget. For every role and every entry in its ``meta/variants.yml``, the deduplicated footprint (the role's own containers plus the shared dependencies it pulls in, each service counted once via the shared-service logic) is summed and checked -against the budget below. Every role that ships variants is evaluated at the -same round index, falling back to variant 0 when it has fewer, because that is -the combination a matrix round actually deploys: a dependency sized for the -production model tier is not what the round runs. ``meta/services.yml`` only -supplies the roles that declare no variant at all. +against the budget below. Every role that ships variants runs the variant +:func:`utils.roles.applications.variants.active_variants_for_round` assigns it +for that round: the round index, falling back to variant 0 when it has fewer, +unless a ``# variant-pin:`` of a role the round includes forces a different +one. That is the combination a matrix round actually deploys: a dependency +sized for the production model tier is not what the round runs. +``meta/services.yml`` only supplies the roles that declare no variant at all. - ``mem_reservation`` total ≤ 32 GB - ``mem_limit`` total ≤ 64 GB @@ -32,12 +34,16 @@ CLI), so this guard and the CLI never drift.""" from __future__ import annotations +import textwrap import unittest from dataclasses import dataclass -from typing import TYPE_CHECKING +from pathlib import Path +from tempfile import TemporaryDirectory +from unittest.mock import patch from humanfriendly import format_size, parse_size +from cli.meta.roles.applications.resolution.combined import repo_paths from utils.annotations.suppress import suppressed_line_numbers from utils.cache.applications import get_variants from utils.cache.files import read_text @@ -49,19 +55,25 @@ from utils.roles.applications.services.resources import ( aggregate, collect_role_resources, ) -from utils.roles.mapping import ROLE_FILE_META_VARIANTS +from utils.roles.applications.variant_pins import RECIPROCAL +from utils.roles.applications.variants import active_variants_for_round +from utils.roles.mapping import ( + ROLE_FILE_META_SERVICES, + ROLE_FILE_META_VARIANTS, + ROLE_FILE_VARS_MAIN, +) from . import PROJECT_ROOT -if TYPE_CHECKING: - from pathlib import Path - ROLES_DIR = PROJECT_ROOT / "roles" MAX_MEM_RESERVATION = parse_size("32GB") MAX_MEM_LIMIT = parse_size("64GB") MEM_LIMIT_RULE = "mem-limit-budget" +PIN_SUBJECT = "web-app-budget-subject" +PIN_PARTNER = "web-app-budget-partner" + @dataclass(frozen=True) class BudgetMeasurement: @@ -147,11 +159,14 @@ def _measure(root: Path) -> list[BudgetMeasurement]: for role, variant_list in sorted(variants.items()): for index, variant_config in enumerate(variant_list): scoped = dict(applications) - for dep_role, dep_variants in variants.items(): - if not dep_variants: - continue - dep_index = index if index < len(dep_variants) else 0 - scoped[dep_role] = dep_variants[dep_index] or {} + active = active_variants_for_round( + roles_dir=str(roles_dir), + round_index=index, + primary_app_variants={role: index}, + variants_per_app=variants, + ) + for dep_role, dep_index in active.items(): + scoped[dep_role] = variants[dep_role][dep_index] or {} scoped[role] = variant_config or {} rows: list = [] collect_role_resources( @@ -199,6 +214,59 @@ def _finding_line(finding: BudgetMeasurement) -> str: ) +def _write_pinned_partner_tree(root: Path, pin: str) -> None: + """Write a roles tree whose subject round 1 exceeds the reservation cap + only when the partner runs its heavy variant 0 instead of the round index. + + Args: + root: directory the ``roles/`` tree is written under. + pin: the line above the subject's partner entry in its variant 1, + ``""`` for none. + """ + files = { + f"{PIN_SUBJECT}/{ROLE_FILE_VARS_MAIN}": f"application_id: {PIN_SUBJECT}\n", + f"{PIN_SUBJECT}/{ROLE_FILE_META_SERVICES}": textwrap.dedent("""\ + budget-subject: + image: subject + mem_reservation: 20GB + mem_limit: 24GB + budget-partner: + enabled: false + shared: true + """), + f"{PIN_SUBJECT}/{ROLE_FILE_META_VARIANTS}": textwrap.dedent(f"""\ + - {{}} + - services: + {pin} + budget-partner: + enabled: true + """), + f"{PIN_PARTNER}/{ROLE_FILE_VARS_MAIN}": f"application_id: {PIN_PARTNER}\n", + f"{PIN_PARTNER}/{ROLE_FILE_META_SERVICES}": textwrap.dedent(f"""\ + budget-partner: + image: partner + shared: true + mem_reservation: 1GB + mem_limit: 2GB + budget-subject: + enabled: "{{{{ '{PIN_SUBJECT}' in group_names }}}}" + """), + f"{PIN_PARTNER}/{ROLE_FILE_META_VARIANTS}": textwrap.dedent("""\ + - services: + budget-partner: + mem_reservation: 20GB + mem_limit: 24GB + budget-subject: + enabled: true + - {} + """), + } + for relative, body in files.items(): + path = root / "roles" / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body) + + class TestVariantResourceBudget(unittest.TestCase): def test_no_variant_exceeds_the_memory_budget(self) -> None: findings = _collect_findings(PROJECT_ROOT) @@ -235,5 +303,36 @@ class TestVariantResourceBudget(unittest.TestCase): ) +class TestPinnedPartnerBudget(unittest.TestCase): + def _tree(self, pin: str) -> Path: + tmp = TemporaryDirectory() + self.addCleanup(tmp.cleanup) + root = Path(tmp.name) + _write_pinned_partner_tree(root, pin) + resolver_root = patch.object(repo_paths, "PROJECT_ROOT", root) + resolver_root.start() + self.addCleanup(resolver_root.stop) + return root + + def test_a_pin_moving_a_partner_over_the_cap_is_reported(self) -> None: + for target in ("0", RECIPROCAL): + with self.subTest(target=target): + root = self._tree(f"# variant-pin: {PIN_PARTNER}#{target}") + self.assertEqual( + [ + (f.role, f.variant, f.mem_reservation) + for f in _collect_findings(root) + ], + [(PIN_SUBJECT, 1, parse_size("40GB"))], + ) + + def test_without_the_pin_the_round_index_composition_fits(self) -> None: + measured = {(m.role, m.variant): m for m in _measure(self._tree(""))} + self.assertEqual(measured[(PIN_SUBJECT, 1)].mem_reservation, parse_size("21GB")) + self.assertFalse( + [m for m in measured.values() if m.over_reservation or m.over_limit] + ) + + if __name__ == "__main__": unittest.main() diff --git a/tests/integration/roles/meta/variants/test_variant_pin_fixed_point.py b/tests/integration/roles/meta/variants/test_variant_pin_fixed_point.py new file mode 100644 index 0000000000..c19bea10f0 --- /dev/null +++ b/tests/integration/roles/meta/variants/test_variant_pin_fixed_point.py @@ -0,0 +1,311 @@ +"""Integration guard: every matrix round settles its variant pins, and no pin +in force stands above a service entry the round switches off. + +A ``# variant-pin:`` in ``R#i`` is in force only while ``R`` is in the round's +include running variant ``i``, and the include can depend on the variant a pin +moves a partner into. :func:`utils.roles.applications.variants.resolve_round` +therefore recomputes the include and the pins in force from the clamped +indices until the assignment reproduces itself. For every round the CI matrix +plans (each role as the only primary, in each of its variants) the guard +requires: + +- ``resolve_round`` raises no ``VariantPinError``: no pins oscillate, + conflict, or name a ``#reciprocal`` the partner cannot resolve; +- no pin in force stands above a service entry the carrier runs with + ``enabled: false`` in the settled round's services map, such as the guide + host every non-primary role has switched off. That pin moves its partner + for a dependency the round never deploys. + +At least one round must move a partner off its clamped index, or the guard +checks a mechanism nothing exercises. +""" + +from __future__ import annotations + +import functools +import textwrap +import unittest +from collections.abc import Mapping +from dataclasses import dataclass +from pathlib import Path +from tempfile import TemporaryDirectory +from typing import TYPE_CHECKING +from unittest.mock import patch + +from cli.meta.roles.applications.resolution.combined import repo_paths +from utils.cache.applications import get_variants +from utils.cache.files import read_text +from utils.roles.applications.variant_pins import ( + PIN, + VariantPinError, + _service_key_lines, +) +from utils.roles.applications.variants import clamped_assignment, resolve_round +from utils.roles.mapping import ( + ROLE_FILE_META_MAIN, + ROLE_FILE_META_SERVICES, + ROLE_FILE_META_VARIANTS, + ROLE_FILE_VARS_MAIN, +) + +from . import PROJECT_ROOT + +if TYPE_CHECKING: + from utils.roles.applications.variants import ResolvedRound + +ROLES_DIR = str(PROJECT_ROOT / "roles") + +HOST = "web-app-pin-host" +LEFT = "web-app-pin-left" +RIGHT = "web-app-pin-right" + + +@dataclass(frozen=True) +class Sweep: + """What the guard finds across every round one roles tree plans. + + Attributes: + unsettled: round -> why its pins never settle. + dead_pins: ``(round, pin)`` per pin in force above a disabled entry. + moving: the rounds in which a pin moves a partner off its clamped index. + """ + + unsettled: dict[str, str] + dead_pins: tuple[tuple[str, str], ...] + moving: tuple[str, ...] + + +def _pinned_keys(role: str, roles_dir: str) -> dict[int, dict[str, str]]: + """Map variant index -> {pinned partner: the service key the pin stands above}. + + Args: + role: the role whose ``meta/variants.yml`` is read. + roles_dir: the roles tree. + """ + path = Path(roles_dir) / role / ROLE_FILE_META_VARIANTS + if not path.is_file(): + return {} + text = read_text(str(path)) + lines = text.splitlines() + found: dict[int, dict[str, str]] = {} + for index, keys in _service_key_lines(text).items(): + for key, line_no in keys.items(): + match = PIN.search(lines[line_no - 1]) if line_no else None + if match: + found.setdefault(index, {})[match.group(1)] = key + return found + + +def _dead_pins(resolved: ResolvedRound, roles_dir: str) -> list[str]: + """The pins in force above a service entry the round runs disabled. + + Args: + resolved: the settled round. + roles_dir: the roles tree the pins are read from. + """ + dead: list[str] = [] + for carrier in resolved.include: + if carrier not in resolved.active_index: + continue + services = resolved.services_overrides.get(carrier) + if not isinstance(services, Mapping): + continue + index = resolved.active_index[carrier] + pinned = _pinned_keys(carrier, roles_dir).get(index, {}) + for partner, key in sorted(pinned.items()): + entry = services.get(key) + if isinstance(entry, Mapping) and entry.get("enabled") is False: + dead.append( + f"{carrier}#{index} pins {partner} above services.{key}, " + "which the round runs with enabled: false" + ) + return dead + + +def _sweep(roles_dir: str) -> Sweep: + """Settle every round the matrix plans for *roles_dir* and judge its pins. + + Args: + roles_dir: the roles tree. + """ + variants = get_variants(roles_dir=roles_dir, readonly=True) + unsettled: dict[str, str] = {} + dead_pins: list[tuple[str, str]] = [] + moving: list[str] = [] + for app, variant_list in sorted(variants.items()): + for index in range(len(variant_list)): + label = f"{app}#{index}" + primaries = {app: index} + try: + resolved = resolve_round( + roles_dir=roles_dir, + round_index=index, + primary_app_variants=primaries, + variants_per_app=variants, + ) + except VariantPinError as exc: + unsettled[label] = str(exc) + continue + clamped = clamped_assignment( + round_index=index, + primary_app_variants=primaries, + variants_per_app=variants, + ) + if resolved.active_index != clamped: + moving.append(label) + dead_pins.extend((label, pin) for pin in _dead_pins(resolved, roles_dir)) + return Sweep(unsettled, tuple(dead_pins), tuple(moving)) + + +@functools.cache +def _repo_sweep() -> Sweep: + return _sweep(ROLES_DIR) + + +def _role(role: str, *, services: str, variants: str) -> dict[str, str]: + """The files of one fixture role. + + Args: + role: the role name; its entity is the name without ``web-app-``. + services: entries appended to the role's ``meta/services.yml``. + variants: the body of the role's ``meta/variants.yml``. + """ + entity = role.removeprefix("web-app-") + return { + f"{role}/{ROLE_FILE_VARS_MAIN}": f"application_id: {role}\n", + f"{role}/{ROLE_FILE_META_SERVICES}": f"{entity}:\n image: {entity}\n" + + textwrap.dedent(services), + f"{role}/{ROLE_FILE_META_VARIANTS}": textwrap.dedent(variants), + } + + +def _oscillating_pair() -> dict[str, str]: + """A tree whose ``HOST#0`` round never settles. + + Returns: + The files of HOST, which depends on LEFT and RIGHT; LEFT#0 pins + RIGHT#1 and RIGHT#1 pins LEFT#1, whose own entry carries no pin. + """ + return { + f"{HOST}/{ROLE_FILE_META_MAIN}": f"dependencies:\n - {LEFT}\n - {RIGHT}\n", + **_role(HOST, services="", variants="- {}\n"), + **_role( + LEFT, + services="", + variants=f"""\ + - services: + # variant-pin: {RIGHT}#1 + pin-right: + enabled: true + - {{}} + """, + ), + **_role( + RIGHT, + services="", + variants=f"""\ + - {{}} + - services: + # variant-pin: {LEFT}#1 + pin-left: + enabled: true + """, + ), + } + + +def _pin_above_disabled_entry() -> dict[str, str]: + """A tree whose ``HOST#0`` round settles on a pin above a disabled entry. + + Returns: + The files of HOST, whose variant 0 pins RIGHT#1 above a + ``pin-right`` entry it switches off, and of RIGHT. + """ + return { + **_role( + HOST, + services="""\ + pin-right: + enabled: true + shared: true + """, + variants=f"""\ + - services: + # variant-pin: {RIGHT}#1 + pin-right: + enabled: false + shared: false + """, + ), + **_role(RIGHT, services="", variants="- {}\n- {}\n"), + } + + +class TestVariantPinFixedPoint(unittest.TestCase): + def test_every_round_settles(self) -> None: + unsettled = _repo_sweep().unsettled + if unsettled: + self.fail( + f"{len(unsettled)} round(s) never settle their variant pins, so " + "the variants files do not determine what the round deploys:\n" + + "\n".join(f" - {label}: {why}" for label, why in unsettled.items()) + + "\n\nDrop the pin, or make the partner variant it selects stop " + "deciding whether the pinning role joins the round." + ) + + def test_no_pin_in_force_stands_above_a_disabled_entry(self) -> None: + dead = _repo_sweep().dead_pins + if dead: + self.fail( + f"{len(dead)} pin(s) move a partner for a service entry the " + "round switches off:\n" + + "\n".join(f" - {label}: {pin}" for label, pin in dead) + + "\n\nDrop the pin, or move it above an entry the variant enables." + ) + + def test_some_round_moves_a_partner(self) -> None: + self.assertTrue( + _repo_sweep().moving, + "no round has a pin that moves a partner off its clamped index, so " + "the guard judges nothing; delete it if the mechanism is gone", + ) + + +class TestVariantPinGuardControls(unittest.TestCase): + def _sweep_tree(self, files: Mapping[str, str]) -> Sweep: + tmp = TemporaryDirectory() + self.addCleanup(tmp.cleanup) + roles = Path(tmp.name) / "roles" + for relative, body in files.items(): + path = roles / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body) + resolver_root = patch.object(repo_paths, "PROJECT_ROOT", Path(tmp.name)) + resolver_root.start() + self.addCleanup(resolver_root.stop) + return _sweep(str(roles)) + + def test_an_oscillating_pin_pair_is_unsettled(self) -> None: + sweep = self._sweep_tree(_oscillating_pair()) + self.assertIn(f"{HOST}#0", sweep.unsettled) + self.assertFalse(sweep.dead_pins) + + def test_a_pin_above_a_disabled_entry_is_dead(self) -> None: + sweep = self._sweep_tree(_pin_above_disabled_entry()) + self.assertFalse(sweep.unsettled) + self.assertEqual( + sweep.dead_pins, + ( + ( + f"{HOST}#0", + ( + f"{HOST}#0 pins {RIGHT} above services.pin-right, " + "which the round runs with enabled: false" + ), + ), + ), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/python/cli/administration/deploy/development/inventory/test_planner.py b/tests/unit/python/cli/administration/deploy/development/inventory/test_planner.py index 47d7e02ffc..516fc2b762 100644 --- a/tests/unit/python/cli/administration/deploy/development/inventory/test_planner.py +++ b/tests/unit/python/cli/administration/deploy/development/inventory/test_planner.py @@ -21,11 +21,6 @@ class TestPlanDevInventoryMatrix(unittest.TestCase): "cli.administration.deploy.development.inventory.legacy_resolver._resolve_round_include", autospec=True, ) - @patch( - "cli.administration.deploy.development.inventory.legacy_resolver._build_services_overrides_for_round", - autospec=True, - return_value={}, - ) @patch( "cli.administration.deploy.development.inventory.planner.get_variants", autospec=True, @@ -33,7 +28,6 @@ class TestPlanDevInventoryMatrix(unittest.TestCase): def test_single_variant_apps_share_one_unsuffixed_folder( self, get_variants_mock: MagicMock, - _overrides_mock: MagicMock, resolve_include_mock: MagicMock, ) -> None: get_variants_mock.return_value = { @@ -63,11 +57,6 @@ class TestPlanDevInventoryMatrix(unittest.TestCase): "cli.administration.deploy.development.inventory.legacy_resolver._resolve_round_include", autospec=True, ) - @patch( - "cli.administration.deploy.development.inventory.legacy_resolver._build_services_overrides_for_round", - autospec=True, - return_value={}, - ) @patch( "cli.administration.deploy.development.inventory.planner.get_variants", autospec=True, @@ -75,7 +64,6 @@ class TestPlanDevInventoryMatrix(unittest.TestCase): def test_two_variant_primary_drives_two_suffixed_folders( self, get_variants_mock: MagicMock, - _overrides_mock: MagicMock, resolve_include_mock: MagicMock, ) -> None: get_variants_mock.return_value = { @@ -115,11 +103,6 @@ class TestPlanDevInventoryMatrix(unittest.TestCase): "cli.administration.deploy.development.inventory.legacy_resolver._resolve_round_include", autospec=True, ) - @patch( - "cli.administration.deploy.development.inventory.legacy_resolver._build_services_overrides_for_round", - autospec=True, - return_value={}, - ) @patch( "cli.administration.deploy.development.inventory.planner.get_variants", autospec=True, @@ -127,7 +110,6 @@ class TestPlanDevInventoryMatrix(unittest.TestCase): def test_round_count_driven_by_primary_max_variants( self, get_variants_mock: MagicMock, - _overrides_mock: MagicMock, resolve_include_mock: MagicMock, ) -> None: get_variants_mock.return_value = { @@ -175,11 +157,6 @@ class TestPlanDevInventoryMatrix(unittest.TestCase): "cli.administration.deploy.development.inventory.legacy_resolver._resolve_round_include", autospec=True, ) - @patch( - "cli.administration.deploy.development.inventory.legacy_resolver._build_services_overrides_for_round", - autospec=True, - return_value={}, - ) @patch( "cli.administration.deploy.development.inventory.planner.get_variants", autospec=True, @@ -187,7 +164,6 @@ class TestPlanDevInventoryMatrix(unittest.TestCase): def test_purge_set_is_union_when_variants_differ( self, get_variants_mock: MagicMock, - _overrides_mock: MagicMock, resolve_include_mock: MagicMock, ) -> None: get_variants_mock.return_value = { @@ -266,11 +242,6 @@ class TestPlanBakesPinnedVariant(unittest.TestCase): autospec=True, return_value=("web-app-bar", "web-app-foo"), ) - @patch( - "cli.administration.deploy.development.inventory.legacy_resolver._build_services_overrides_for_round", - autospec=True, - return_value={}, - ) @patch( "cli.administration.deploy.development.inventory.planner.get_variants", autospec=True, @@ -278,7 +249,6 @@ class TestPlanBakesPinnedVariant(unittest.TestCase): def test_round_variants_carry_the_pinned_index( self, get_variants_mock: MagicMock, - _overrides_mock: MagicMock, _include_mock: MagicMock, _pins_mock: MagicMock, ) -> None: diff --git a/tests/unit/python/cli/meta/roles/applications/resolution/affected/test_main.py b/tests/unit/python/cli/meta/roles/applications/resolution/affected/test_main.py index 40314d62bc..0b8acce987 100644 --- a/tests/unit/python/cli/meta/roles/applications/resolution/affected/test_main.py +++ b/tests/unit/python/cli/meta/roles/applications/resolution/affected/test_main.py @@ -163,6 +163,61 @@ class TestAffected(unittest.TestCase): got = affected_main.affected_selection(["web-app-keycloak"]) self.assertIn("web-app-consumer#0", got) + def test_each_variant_is_judged_with_the_pins_its_own_round_runs(self) -> None: + """Variant 1 reaches the seed only through its own pin moving the + partner, and an unrelated role's pin would move the subject itself; + only the round the planner deploys for each variant keeps both.""" + with tempfile.TemporaryDirectory() as td: + root = Path(td) + for role in ( + "web-app-keycloak", + "web-app-matomo", + "web-app-consumer", + "web-app-other", + ): + _mk_app_role(root, role, role) + off = {"enabled": False, "shared": False} + on = {"enabled": True, "shared": True} + dump_yaml( + root / "roles" / "web-app-matomo" / ROLE_FILE_META_SERVICES, + {"matomo": {}, "sso": off}, + ) + dump_yaml( + root / "roles" / "web-app-matomo" / ROLE_FILE_META_VARIANTS, + [{}, {}, {"services": {"sso": on}}], + ) + dump_yaml( + root / "roles" / "web-app-consumer" / ROLE_FILE_META_SERVICES, + {"consumer": {}, "sso": on, "matomo": on}, + ) + (root / "roles" / "web-app-consumer" / ROLE_FILE_META_VARIANTS).write_text( + "- services:\n" + " matomo: {enabled: false, shared: false}\n" + "- services:\n" + " sso: {enabled: false, shared: false}\n" + " # variant-pin: web-app-matomo#2\n" + " matomo: {enabled: true, shared: true}\n" + "- services:\n" + " sso: {enabled: false, shared: false}\n" + " matomo: {enabled: false, shared: false}\n", + encoding="utf-8", + ) + dump_yaml( + root / "roles" / "web-app-other" / ROLE_FILE_META_SERVICES, + {"other": {}, "matomo": off}, + ) + (root / "roles" / "web-app-other" / ROLE_FILE_META_VARIANTS).write_text( + "- services:\n" + " # variant-pin: web-app-consumer#2\n" + " matomo: {enabled: false, shared: false}\n" + "- {}\n", + encoding="utf-8", + ) + + with patch.object(repo_paths, "PROJECT_ROOT", root): + got = affected_main.affected_selection(["web-app-keycloak"]) + self.assertIn("web-app-consumer#0,1", got) + def test_a_seed_keeps_every_variant_of_its_own_role(self) -> None: with tempfile.TemporaryDirectory() as td: root = Path(td) diff --git a/tests/unit/python/utils/roles/applications/test_variants_pin_scope.py b/tests/unit/python/utils/roles/applications/test_variants_pin_scope.py new file mode 100644 index 0000000000..429ef79d2a --- /dev/null +++ b/tests/unit/python/utils/roles/applications/test_variants_pin_scope.py @@ -0,0 +1,107 @@ +"""Unit tests for the scope of ``# variant-pin:`` in a round's assignment. + +The include resolver is stubbed so each case states the round's include +directly: what matters is which roles carry a pin into the round, not how +the closure is walked. +""" + +from __future__ import annotations + +import textwrap +import unittest +from pathlib import Path +from tempfile import TemporaryDirectory +from unittest.mock import patch + +from utils.roles.applications.variant_pins import VariantPinError +from utils.roles.applications.variants import active_variants_for_round + +CARRIER = "web-app-carrier" +PARTNER = "web-app-partner" +SUBJECT = "web-app-subject" + +_INCLUDE = ( + "cli.administration.deploy.development.inventory.legacy_resolver." + "_resolve_round_include" +) + + +class TestPinScope(unittest.TestCase): + def setUp(self) -> None: + tmp = TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.roles_dir = Path(tmp.name) / "roles" + self.variants = { + SUBJECT: [{}, {}], + CARRIER: [{}, {"services": {"partner": {"enabled": True}}}], + PARTNER: [{}, {}], + } + + def write(self, role: str, name: str, body: str) -> None: + path = self.roles_dir / role / "meta" / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(textwrap.dedent(body)) + + def assign(self, round_index: int) -> dict[str, int]: + return active_variants_for_round( + roles_dir=str(self.roles_dir), + round_index=round_index, + primary_app_variants={SUBJECT: round_index}, + variants_per_app=self.variants, + ) + + def pin_partner_from_carrier_variant_1(self) -> None: + self.write( + CARRIER, + "variants.yml", + f"""\ + - {{}} + - services: + # variant-pin: {PARTNER}#0 + partner: + enabled: true + """, + ) + + def test_a_pin_of_a_role_outside_the_include_is_not_in_force(self) -> None: + self.pin_partner_from_carrier_variant_1() + with patch(_INCLUDE, autospec=True, return_value=(PARTNER, SUBJECT)): + self.assertEqual(self.assign(1)[PARTNER], 1) + + def test_a_pin_of_a_role_inside_the_include_is_in_force(self) -> None: + self.pin_partner_from_carrier_variant_1() + with patch(_INCLUDE, autospec=True, return_value=(CARRIER, PARTNER, SUBJECT)): + self.assertEqual(self.assign(1)[PARTNER], 0) + + def test_a_pin_that_drops_its_own_carrier_raises(self) -> None: + self.write( + CARRIER, + "variants.yml", + f"""\ + - services: + # variant-pin: {PARTNER}#1 + partner: + enabled: true + - {{}} + """, + ) + self.write(PARTNER, "services.yml", "carrier:\n enabled: false\n") + self.variants[PARTNER] = [ + {"services": {"carrier": {"enabled": True}}}, + {"services": {"carrier": {"enabled": False}}}, + ] + + def include(*, primary_apps, services_overrides): + carried = services_overrides[PARTNER]["carrier"]["enabled"] is True + return (CARRIER, PARTNER, *primary_apps) if carried else (PARTNER,) + + with ( + patch(_INCLUDE, autospec=True, side_effect=include), + self.assertRaises(VariantPinError) as caught, + ): + self.assign(0) + self.assertIn("without settling", str(caught.exception)) + + +if __name__ == "__main__": + unittest.main() diff --git a/utils/roles/applications/variants.py b/utils/roles/applications/variants.py index 50eeba9a31..c734d4789b 100644 --- a/utils/roles/applications/variants.py +++ b/utils/roles/applications/variants.py @@ -15,6 +15,7 @@ resolves its closure on a runner that has no inventory, no container and no from __future__ import annotations from collections.abc import Mapping +from dataclasses import dataclass from pathlib import Path from typing import Any @@ -27,22 +28,43 @@ from utils.roles.applications.services.registry import ( build_service_registry_from_roles_dir, ) from utils.roles.applications.topics import CONFIG_TOPICS, apply_topic, provider_of -from utils.roles.applications.variant_pins import resolve_pins +from utils.roles.applications.variant_pins import VariantPinError, resolve_pins from utils.roles.guide import declares_guide, guide_off from utils.roles.mapping import ROLE_FILE_META_SERVICES __all__ = [ "NestedOverrideConflictError", + "ResolvedRound", "active_variants_for_round", + "clamped_assignment", "collect_provider_overrides", + "resolve_round", + "round_under", "services_overrides_for_round", ] +_MERGED_SERVICES: dict[tuple[str, int], tuple[tuple[int, int], Any, Any]] = {} + class NestedOverrideConflictError(RuntimeError): """Two roles in one round demand different config for one provider.""" +@dataclass(frozen=True) +class ResolvedRound: + """One round once its variant pins have settled. + + Attributes: + active_index: role -> the variant index the round runs it in. + services_overrides: role -> its services map under that variant. + include: the round's include set, deps first and primaries last. + """ + + active_index: dict[str, int] + services_overrides: dict[str, dict] + include: tuple[str, ...] + + def _claims(payloads: Mapping[str, Any]) -> list[tuple[str, str, str, Any]]: """Return the ``(role, service_key, topic, value)`` a round's payloads carry. @@ -125,6 +147,227 @@ def _apply_nested_service_maps(overrides: dict[str, dict], *, roles_dir: str) -> overrides[provider] = apply_topic(overrides.get(provider), topics["services"]) +def _merged_services(services_path: Path, index: int, variant: Mapping) -> Any: + """Return ``variant`` deep-merged onto ``meta/services.yml``, once per variant. + + An entry is reused only while the file keeps its mtime and size and + ``variant`` is the very object it was built from, which + :func:`get_variants` hands out until the variants file changes. The + result is shared by every round that runs the variant, so nothing + downstream may mutate it. + + Args: + services_path: the role's ``meta/services.yml``. + index: the variant ``variant`` belongs to. + variant: the variant's ``services`` block. + + Returns: + The merged map, or ``None`` when the role has no readable + ``meta/services.yml`` mapping. + """ + try: + stat = services_path.stat() + except OSError: + return None + key = (str(services_path), index) + signature = (stat.st_mtime_ns, stat.st_size) + hit = _MERGED_SERVICES.get(key) + if hit is not None and hit[0] == signature and hit[1] is variant: + return hit[2] + try: + base_services = load_yaml_any(services_path) or {} + except (OSError, ValueError, yaml.YAMLError): + return None + if not isinstance(base_services, Mapping): + return None + merged = _deep_merge(dict(base_services), dict(variant)) + _MERGED_SERVICES[key] = (signature, variant, merged) + return merged + + +def _overrides_for_assignment( + active_index: Mapping[str, int], + *, + roles_dir: str, + primary_app_variants: Mapping[str, int], + variants_per_app: Mapping[str, list], +) -> dict[str, dict]: + """Merge each role's assigned variant onto its ``meta/services.yml``. + + Args: + active_index: role -> the variant index the round runs it in. + roles_dir: roles directory the services maps are read from. + primary_app_variants: the round's primary apps; only they keep the + guide host their variant declares. + variants_per_app: parsed variants per role. + + Returns: + ``{role: merged services map}`` for every role with variants and a + ``meta/services.yml``. + """ + overrides: dict[str, dict] = {} + roles_path = Path(roles_dir) + for role_name, variant_list in variants_per_app.items(): + if not variant_list: + continue + index = active_index[role_name] + variant_payload = variant_list[index] + if not isinstance(variant_payload, Mapping): + variant_payload = {} + variant_services = variant_payload.get("services", {}) + if not isinstance(variant_services, Mapping): + continue + merged = _merged_services( + roles_path / role_name / ROLE_FILE_META_SERVICES, index, variant_services + ) + if isinstance(merged, dict): + overrides[role_name] = ( + dict(merged) + if role_name in primary_app_variants and declares_guide(merged) + else guide_off(merged) + ) + _apply_nested_service_maps(overrides, roles_dir=roles_dir) + return overrides + + +def clamped_assignment( + *, + round_index: int, + primary_app_variants: Mapping[str, int], + variants_per_app: Mapping[str, list], +) -> dict[str, int]: + """Return the variant every role starts a round on, before any pin. + + Args: + round_index: the round of the sweep; a role with fewer variants falls + back to variant 0. + primary_app_variants: the already clamped index of every primary app. + variants_per_app: parsed variants per role. + """ + + def _round_index_for(role_name: str, variant_list: list) -> int: + variant_count = max(1, len(variant_list)) + if role_name in primary_app_variants: + idx = primary_app_variants[role_name] + else: + idx = round_index if round_index < variant_count else 0 + return idx if 0 <= idx < len(variant_list) else 0 + + return { + role_name: _round_index_for(role_name, variant_list) + for role_name, variant_list in variants_per_app.items() + if variant_list + } + + +def round_under( + active_index: Mapping[str, int], + *, + roles_dir: str, + primary_app_variants: Mapping[str, int], + variants_per_app: Mapping[str, list], +) -> tuple[ResolvedRound, dict[str, int]]: + """Return what one assignment deploys and the pins in force under it. + + A ``# variant-pin:`` in ``R#i`` is in force only while ``R`` is in the + include running variant ``i``, and no pin moves a primary app. + + Args: + active_index: role -> the variant index to evaluate the round at. + roles_dir: roles directory the services maps and pins are read from. + primary_app_variants: the round's primary apps, in the order the + include resolves them. + variants_per_app: parsed variants per role. + + Returns: + The round under ``active_index``, and ``{role: index}`` for every + non-primary role a pin in force moves. + """ + # Imported per call: legacy_resolver imports this module, and tests patch + # the include resolver on it. + from cli.administration.deploy.development.inventory import legacy_resolver + + overrides = _overrides_for_assignment( + active_index, + roles_dir=roles_dir, + primary_app_variants=primary_app_variants, + variants_per_app=variants_per_app, + ) + include = tuple( + legacy_resolver._resolve_round_include( + primary_apps=list(primary_app_variants), + services_overrides=overrides, + ) + ) + pinned = resolve_pins( + {role: active_index[role] for role in include if role in active_index}, + roles_dir=roles_dir, + variants_per_app=variants_per_app, + ) + forced = { + role: index + for role, index in pinned.items() + if role not in primary_app_variants and role in active_index + } + return ( + ResolvedRound( + active_index=dict(active_index), + services_overrides=overrides, + include=include, + ), + forced, + ) + + +def resolve_round( + *, + roles_dir: str, + round_index: int, + primary_app_variants: Mapping[str, int], + variants_per_app: Mapping[str, list], +) -> ResolvedRound: + """Settle the variant every role runs in one round, and what that deploys. + + Starting from :func:`clamped_assignment`, each step evaluates + :func:`round_under` and moves to the clamped indices plus the pins in + force; the round is the first assignment a step reproduces. + + Args: + roles_dir: roles directory the services maps and pins are read from. + round_index: the round of the sweep. + primary_app_variants: the already clamped index of every primary app, + in the order the include resolves them. + variants_per_app: parsed variants per role. + + Raises: + VariantPinError: the pins never settle on one assignment. + """ + clamped = clamped_assignment( + round_index=round_index, + primary_app_variants=primary_app_variants, + variants_per_app=variants_per_app, + ) + active_index = dict(clamped) + visited: list[dict[str, int]] = [] + while active_index not in visited: + visited.append(active_index) + resolved, forced = round_under( + active_index, + roles_dir=roles_dir, + primary_app_variants=primary_app_variants, + variants_per_app=variants_per_app, + ) + settled = {**clamped, **forced} + if settled == active_index: + return resolved + active_index = settled + raise VariantPinError( + f"round {round_index} with primaries {dict(primary_app_variants)}: " + f"the variant pins cycle through {len(visited)} assignments without " + f"settling on one" + ) + + def active_variants_for_round( *, roles_dir: str, @@ -142,30 +385,17 @@ def active_variants_for_round( variants_per_app: parsed variants per role. Returns: - The clamped round index per role, replaced by the index a pin in - another role's active variant forces. + The assignment :func:`resolve_round` settles on. + + Raises: + VariantPinError: the pins never settle on one assignment. """ - - def _round_index_for(role_name: str, variant_list: list) -> int: - variant_count = max(1, len(variant_list)) - if role_name in primary_app_variants: - idx = primary_app_variants[role_name] - else: - idx = round_index if round_index < variant_count else 0 - return idx if 0 <= idx < len(variant_list) else 0 - - active_index = { - role_name: _round_index_for(role_name, variant_list) - for role_name, variant_list in variants_per_app.items() - if variant_list - } - pinned = resolve_pins( - active_index, roles_dir=roles_dir, variants_per_app=variants_per_app - ) - for role_name, index in pinned.items(): - if role_name not in primary_app_variants and role_name in active_index: - active_index[role_name] = index - return active_index + return resolve_round( + roles_dir=roles_dir, + round_index=round_index, + primary_app_variants=primary_app_variants, + variants_per_app=variants_per_app, + ).active_index def services_overrides_for_round( @@ -178,12 +408,10 @@ def services_overrides_for_round( that results from merging the round's variant payload onto the role's on-disk `meta/services.yml`. - Apps in `primary_app_variants` use the supplied (already clamped) - index. Other roles with their own variants clamp `round_index` to - their own variant count, unless a `# variant-pin:` marker in the - round's active variant of another role pins them (see - :mod:`utils.roles.applications.variant_pins`); a primary app keeps - its supplied index and no pin overrides it. The guide host stays on + Each role runs the variant :func:`resolve_round` settles on: the clamped + round index, the supplied index for a primary app, or the index a + `# variant-pin:` in force forces (see + :mod:`utils.roles.applications.variant_pins`). The guide host stays on only where a primary app's variant declares it; every other role has it off. Roles without variants are absent from the result so the resolver @@ -194,41 +422,9 @@ 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, readonly=True) - overrides: dict[str, dict] = {} - roles_path = Path(roles_dir) - active_index = active_variants_for_round( + return resolve_round( roles_dir=roles_dir, round_index=round_index, primary_app_variants=primary_app_variants, - variants_per_app=variants_per_app, - ) - - for role_name, variant_list in variants_per_app.items(): - if not variant_list: - continue - idx = active_index[role_name] - variant_payload = variant_list[idx] if variant_list else {} - if not isinstance(variant_payload, Mapping): - variant_payload = {} - variant_services = variant_payload.get("services", {}) - if not isinstance(variant_services, Mapping): - continue - services_path = roles_path / role_name / ROLE_FILE_META_SERVICES - if not services_path.exists(): - continue - try: - base_services = load_yaml_any(services_path) or {} - except (OSError, ValueError, yaml.YAMLError): - continue - if not isinstance(base_services, Mapping): - continue - merged = _deep_merge(dict(base_services), dict(variant_services)) - if isinstance(merged, dict): - overrides[role_name] = ( - merged - if role_name in primary_app_variants and declares_guide(merged) - else guide_off(merged) - ) - _apply_nested_service_maps(overrides, roles_dir=roles_dir) - return overrides + variants_per_app=get_variants(roles_dir=roles_dir, readonly=True), + ).services_overrides