diff --git a/cli/k8s_runner.py b/cli/k8s_runner.py index a202230..1c55a5b 100644 --- a/cli/k8s_runner.py +++ b/cli/k8s_runner.py @@ -7,13 +7,20 @@ import re import subprocess # nosec B404 import tempfile +import time +from collections.abc import Callable from pathlib import Path from typing import IO, Any import yaml -from .state import parse_k8s_workloads_json -from .up_runner import _validate_command, run_streamed +from .state import group_services_by_health, parse_k8s_workloads_json +from .up_runner import ( + DEFAULT_POLL_INTERVAL_SECONDS, + _poll_until_settled, + _validate_command, + run_streamed, +) # RFC 1123 DNS label: lowercase alphanumeric and `-`, not leading/trailing # with `-`, 1-63 chars. Both Kubernetes namespaces and Helm release names @@ -90,16 +97,37 @@ def helm_up( timeout: float, detach: bool, log_file: IO[str], + poll_interval: float = DEFAULT_POLL_INTERVAL_SECONDS, + use_color: bool = False, + sleep_fn: Callable[[float], None] = time.sleep, + now_fn: Callable[[], float] = time.monotonic, + redraw_fn: Callable[[str], None] | None = None, ) -> int: - """Install or upgrade a rendered chart without persisting secret values.""" + """ + Install or upgrade a rendered chart without persisting secret values. + + `timeout` is one overall budget shared across the whole call, the way + Compose's `--timeout` already is: it bounds `helm upgrade --install` + itself, then whatever remains bounds the readiness poll below, rather + than being reused in full for each step (which could let total + wall-clock time exceed `timeout` several times over on a + multi-workload release). + + Readiness is decided by polling `get_k8s_state()` / + `group_services_by_health()` — the exact same functions `cds state + --target helm` uses — instead of a separate `helm --wait` + + `kubectl rollout status`/`wait` implementation, so "ready" here and + "healthy" in `cds state` can never disagree. + """ namespace = _validate_k8s_name(namespace, "namespace") release = _validate_k8s_name(release, "release") kube_context = _validate_kube_context(kube_context) timeout = _validate_timeout(timeout) + deadline = now_fn() + timeout secret_values = _secret_values(plan) secret_path = _write_secret_values(secret_values) context_args = ["--kube-context", kube_context] if kube_context else [] - timeout_arg = f"{timeout}s" + apply_timeout_arg = f"{timeout}s" command = [ "helm", *context_args, @@ -113,10 +141,8 @@ def helm_up( "--values", str(secret_path), "--timeout", - timeout_arg, + apply_timeout_arg, ] - if not detach: - command.append("--wait") try: result = run_streamed(command, log_file, timeout=timeout + 30) @@ -126,31 +152,76 @@ def helm_up( return result workloads = get_k8s_workloads(namespace, release, kube_context) - for workload in workloads: - kind = str(workload.get("kind", "")).lower() - name = str((workload.get("metadata") or {}).get("name", "")) - if not kind or not name: - continue - if kind == "job": - wait_command = _kubectl_command(kube_context, namespace) + [ - "wait", - "--for=condition=complete", - f"job/{name}", - f"--timeout={timeout_arg}", - ] - else: - wait_command = _kubectl_command(kube_context, namespace) + [ - "rollout", - "status", - f"{kind}/{name}", - f"--timeout={timeout_arg}", - ] - result = run_streamed(wait_command, log_file, timeout=timeout + 30) - if result != 0: - return result + remaining = max(0.0, deadline - now_fn()) + settled, grouped = poll_k8s_state_until_settled( + namespace, + release, + kube_context, + expected_service_count=len(workloads), + poll_interval=poll_interval, + timeout=remaining, + use_color=use_color, + sleep_fn=sleep_fn, + now_fn=now_fn, + redraw_fn=redraw_fn, + ) + if not settled: + unhealthy = [name for bucket in ("UNHEALTHY", "UNHEALTHY EXIT") for name in grouped.get(bucket, [])] + log_file.write( + f"helm release {release} did not settle within {timeout:.0f}s" + + (f"; unhealthy: {', '.join(unhealthy)}" if unhealthy else "") + + "\n" + ) + log_file.flush() + return 1 return 0 +def poll_k8s_state_until_settled( + namespace: str, + release: str, + kube_context: str | None, + *, + expected_service_count: int | None = None, + poll_interval: float = DEFAULT_POLL_INTERVAL_SECONDS, + timeout: float = 180.0, + use_color: bool = False, + sleep_fn: Callable[[float], None] = time.sleep, + now_fn: Callable[[], float] = time.monotonic, + fetch_fn: Callable[[], list[dict[str, Any]]] | None = None, + redraw_fn: Callable[[str], None] | None = None, +) -> tuple[bool, dict[str, list[str]]]: + """ + Polls `get_k8s_state()` every `poll_interval` seconds, grouping with + `group_services_by_health()` — the same pair `cds state --target + helm` calls — until every workload settles into a terminal bucket or + `timeout` seconds elapse. Shares its settle/failure rule with + Compose's `poll_state_until_settled()` via `_poll_until_settled()`. + + `fetch_fn` is injectable for tests; defaults to a real + `get_k8s_state(namespace, release, kube_context)` call. + """ + if fetch_fn is None: + def fetch_fn() -> list[dict[str, Any]]: + return get_k8s_state(namespace, release, kube_context) + + def fetch_grouped() -> dict[str, list[str]]: + return group_services_by_health(fetch_fn()) + + return _poll_until_settled( + fetch_grouped, + expected_service_count=expected_service_count, + poll_interval=poll_interval, + timeout=timeout, + use_color=use_color, + sleep_fn=sleep_fn, + now_fn=now_fn, + redraw_fn=redraw_fn, + up_done_fn=None, + on_up_finished=None, + ) + + def helm_down( *, namespace: str, diff --git a/cli/main.py b/cli/main.py index 6836705..c1b7f1c 100644 --- a/cli/main.py +++ b/cli/main.py @@ -33,7 +33,7 @@ from .k8s_runner import get_k8s_state, helm_down, helm_up from .k8s_security import scan_k8s_security from .loader import save_generated_profile -from .overlay import resolve_extends, resolve_profile +from .overlay import _merge_profile_docs, resolve_extends, resolve_profile from .planner import build_plan from .preflight import preflight_passed, run_preflight from .renderer import render_compose @@ -97,19 +97,73 @@ def print_diagnostics(diagnostics) -> None: print(f"{prefix} {d.format()}\n") -def _k8s_runtime_defaults(profile_path: str) -> tuple[str, str]: - """Read non-secret Helm defaults without planning or loading environment values.""" +def _helm_identity( + metadata: dict[str, Any] | None, + runtime: dict[str, Any] | None, + profile_path: str, +) -> tuple[str, str]: + """ + Shared release/namespace fallback rule: release from metadata.name (or + the profile's directory name), namespace from runtime.namespace (or + "cds-local"). `metadata`/`runtime` can come either from a fully built + plan (`up`) or from a best-effort document resolution (`down`/`state`) + as long as both use this same rule, so a stack brought up with a given + --environment is torn down/inspected against the same release and + namespace. + """ fallback_release = Path(profile_path).parent.name or "cds" + release = str((metadata or {}).get("name") or fallback_release) + namespace = str((runtime or {}).get("namespace") or "cds-local") + return release, namespace + + +def _resolve_profile_document_best_effort( + profile_path: str, environment: str | None = None +) -> dict[str, Any]: + """ + Resolves a profile's `extends` chain and, if given, an `--environment` + overlay, WITHOUT running the full validate_loaded_profile() gate that + resolve_profile() applies. This is deliberate: down/state must be able + to resolve the release/namespace a stack was brought up under even if + the profile has since been edited into an invalid state (e.g. an + unrelated module config error) -- falling back to generic defaults in + that case would target the wrong release/namespace instead of tearing + down (or reporting on) the one that's actually running. + """ + document, provenance, diagnostics = resolve_extends(profile_path) + if document is None or any(d.level == "error" for d in diagnostics): + return {} + + if not environment: + return document + + profile_file = Path(profile_path) + overlay_file = profile_file.parent / "environments" / f"{environment}.yaml" + if not overlay_file.is_file(): + return document + try: - document = yaml.safe_load(Path(profile_path).read_text(encoding="utf-8")) or {} + overlay = yaml.safe_load(overlay_file.read_text(encoding="utf-8")) or {} except (OSError, UnicodeDecodeError, yaml.YAMLError): - return fallback_release, "cds-local" - release = str((document.get("metadata") or {}).get("name") or fallback_release) - namespace = str( - (((document.get("spec") or {}).get("runtime") or {}).get("namespace")) - or "cds-local" + return document + + merged, merge_diagnostics = _merge_profile_docs( + document, overlay, str(profile_file), str(overlay_file), provenance ) - return release, namespace + if merge_diagnostics: + return document + return merged + + +def _k8s_runtime_defaults(profile_path: str, environment: str | None = None) -> tuple[str, str]: + """ + Read non-secret Helm defaults without planning, using the same + release/namespace rule the built plan uses (`_helm_identity`), so + `down`/`state` target the same release an `up --environment ...` run + created. + """ + document = _resolve_profile_document_best_effort(profile_path, environment) + return _helm_identity(document.get("metadata"), (document.get("spec") or {}).get("runtime"), profile_path) def profile_completer(prefix, parsed_args, **kwargs): @@ -1123,6 +1177,7 @@ def main() -> int: down_parser = subparsers.add_parser("down", help="Stop or uninstall a running profile") _add_profile_arg(down_parser) + _add_environment_arg(down_parser) down_parser.add_argument( "--target", choices=["compose", "helm"], default="compose", help="Runtime target." ) @@ -1174,6 +1229,7 @@ def main() -> int: help="Show running service status grouped by health", ) _add_profile_arg(state_parser) + _add_environment_arg(state_parser) state_parser.add_argument( "--target", choices=["compose", "helm"], default="compose", help="Runtime target." ) @@ -1348,7 +1404,7 @@ def main() -> int: args = parser.parse_args() environment_explicit = hasattr(args, "environment") - if args.command in {"validate", "plan", "render", "up", "test", "preflight", "init", "security"}: + if args.command in {"validate", "plan", "render", "up", "test", "preflight", "init", "security", "down", "state"}: args.environment = getattr(args, "environment", None) or load_saved_environment() if args.command in {"render", "up", "test"}: args.image_source = getattr(args, "image_source", None) or load_saved_image_source() @@ -1618,8 +1674,16 @@ def main() -> int: print("Cannot start stack because Helm rendering failed.") return code or 1 - namespace = args.namespace or plan.get("runtime", {}).get("namespace") or "cds-local" - release = args.release or plan.get("metadata", {}).get("name") or "cds" + plan_release, plan_namespace = _helm_identity( + plan.get("metadata"), plan.get("runtime"), profile_path + ) + namespace = args.namespace or plan_namespace + release = args.release or plan_release + if args.no_build: + print( + "NOTE --no-build has no effect with --target helm: " + "the Helm target does not build local images yet." + ) log_path = ( Path(args.log_file) if args.log_file else default_log_path(Path(profile_path).parent.name) ).resolve() @@ -1635,6 +1699,7 @@ def main() -> int: timeout=args.timeout, detach=args.detach, log_file=log_file, + use_color=(not args.no_color) and sys.stdout.isatty(), ) except (FileNotFoundError, OSError, RuntimeError, ValueError) as exc: print(f"ERROR {exc}") @@ -1837,7 +1902,7 @@ def _begin_log_tail(_up_exit_code: int) -> None: print(f"ERROR {exc}") return 1 project_root = resolve_project_root(profile_path) - profile_release, profile_namespace = _k8s_runtime_defaults(profile_path) + profile_release, profile_namespace = _k8s_runtime_defaults(profile_path, args.environment) log_path = ( Path(args.log_file) if args.log_file else default_log_path(f"down-{Path(profile_path).parent.name}") ).resolve() @@ -2038,7 +2103,7 @@ def _begin_log_tail(_up_exit_code: int) -> None: return 1 if args.target == "helm": - profile_release, profile_namespace = _k8s_runtime_defaults(profile_path) + profile_release, profile_namespace = _k8s_runtime_defaults(profile_path, args.environment) try: services = get_k8s_state( args.namespace or profile_namespace, diff --git a/cli/up_runner.py b/cli/up_runner.py index b75a672..74e1555 100644 --- a/cli/up_runner.py +++ b/cli/up_runner.py @@ -205,8 +205,8 @@ def _default_redraw(text: str) -> None: sys.stdout.flush() -def poll_state_until_settled( - compose_path: str, +def _poll_until_settled( + fetch_grouped: Callable[[], dict[str, list[str]]], *, expected_service_count: int | None = None, poll_interval: float = DEFAULT_POLL_INTERVAL_SECONDS, @@ -214,55 +214,37 @@ def poll_state_until_settled( use_color: bool = False, sleep_fn: Callable[[float], None] = time.sleep, now_fn: Callable[[], float] = time.monotonic, - ps_fn: Callable[[], subprocess.CompletedProcess] | None = None, redraw_fn: Callable[[str], None] | None = None, up_done_fn: Callable[[], int | None] | None = None, on_up_finished: Callable[[int], None] | None = None, ) -> tuple[bool, dict[str, list[str]]]: """ - Polls `docker compose ps -a --format json` every `poll_interval` - seconds, redrawing the grouped `cds state` view each time, until - every service `docker compose ps` reports is in a terminal bucket - (HEALTHY, RUNNING, HEALTHY EXIT, UNHEALTHY EXIT, or UNHEALTHY) or - `timeout` seconds elapse. + Shared readiness-polling core used by both `poll_state_until_settled` + (Compose) and `poll_k8s_state_until_settled` (Helm). Calls + `fetch_grouped()` every `poll_interval` seconds, redrawing the grouped + `cds state` view each time, until every reported service is in a + terminal bucket (HEALTHY, RUNNING, HEALTHY EXIT, UNHEALTHY EXIT, or + UNHEALTHY) or `timeout` seconds elapse. Returns `(settled, grouped)`. `settled` is False if the loop timed out, or if any service ended in UNHEALTHY / UNHEALTHY EXIT. - `ps_fn`, `sleep_fn`, `now_fn`, and `redraw_fn` are injectable so this - can be unit tested with a fake clock and canned `ps` output instead - of real Docker calls and real sleeping. + `sleep_fn`, `now_fn`, and `redraw_fn` are injectable so this can be + unit tested with a fake clock instead of real sleeping. `up_done_fn`, if given, is polled once per iteration and must return - `None` while `docker compose up` is still running, or its exit code - once it has finished. This lets the caller run `up` in the - background while this loop redraws the live view immediately, - without either process blocking the other: - - - If `up` exits non-zero, this returns `(False, grouped)` right - away instead of burning through the full `timeout`, since - services that `up` never started will never settle. - - The `timeout` clock only starts once `up` finishes successfully, - so a stack whose healthchecks legitimately outlast `timeout` - isn't penalized for time `up` itself spent blocked on - healthcheck-gated `depends_on` dependencies. Omitting `up_done_fn` - preserves the old behavior of starting the clock immediately. + `None` while the underlying apply command is still running, or its + exit code once it has finished. See `poll_state_until_settled` for + the full rationale. `on_up_finished`, if given, is called exactly once, the first time - `up_done_fn` reports a successful result (exit code 0), so callers can - defer setup (e.g. starting a log tail) until `up` is done rather - than running it concurrently with `up`'s own output. + `up_done_fn` reports a successful result (exit code 0). """ - if ps_fn is None: - def ps_fn() -> subprocess.CompletedProcess: - ps_cmd = ["docker", "compose", "-f", compose_path, "ps", "-a", "--format", "json"] - return subprocess.run(ps_cmd, capture_output=True, text=True) # nosec B603 # noqa: S603 - if redraw_fn is None: redraw_fn = _default_redraw if up_done_fn is None: - # No background `up` process to track: behave as if it had + # No background apply process to track: behave as if it had # already finished successfully, so the timeout clock starts # immediately (matches the pre-existing behavior). def up_done_fn() -> int | None: @@ -272,9 +254,7 @@ def up_done_fn() -> int | None: up_finished_seen = False grouped: dict[str, list[str]] = {} while True: - ps_result = ps_fn() - services = parse_compose_ps_json(ps_result.stdout) if ps_result.returncode == 0 else [] - grouped = group_services_by_health(services) + grouped = fetch_grouped() redraw_fn(format_state_output(grouped, use_color=use_color)) if _is_settled(grouped, expected_service_count): @@ -298,3 +278,75 @@ def up_done_fn() -> int | None: return False, grouped sleep_fn(poll_interval) + + +def poll_state_until_settled( + compose_path: str, + *, + expected_service_count: int | None = None, + poll_interval: float = DEFAULT_POLL_INTERVAL_SECONDS, + timeout: float = DEFAULT_TIMEOUT_SECONDS, + use_color: bool = False, + sleep_fn: Callable[[float], None] = time.sleep, + now_fn: Callable[[], float] = time.monotonic, + ps_fn: Callable[[], subprocess.CompletedProcess] | None = None, + redraw_fn: Callable[[str], None] | None = None, + up_done_fn: Callable[[], int | None] | None = None, + on_up_finished: Callable[[int], None] | None = None, +) -> tuple[bool, dict[str, list[str]]]: + """ + Polls `docker compose ps -a --format json` every `poll_interval` + seconds, redrawing the grouped `cds state` view each time, until + every service `docker compose ps` reports is in a terminal bucket + (HEALTHY, RUNNING, HEALTHY EXIT, UNHEALTHY EXIT, or UNHEALTHY) or + `timeout` seconds elapse. + + Returns `(settled, grouped)`. `settled` is False if the loop timed + out, or if any service ended in UNHEALTHY / UNHEALTHY EXIT. + + `ps_fn`, `sleep_fn`, `now_fn`, and `redraw_fn` are injectable so this + can be unit tested with a fake clock and canned `ps` output instead + of real Docker calls and real sleeping. + + `up_done_fn`, if given, is polled once per iteration and must return + `None` while `docker compose up` is still running, or its exit code + once it has finished. This lets the caller run `up` in the + background while this loop redraws the live view immediately, + without either process blocking the other: + + - If `up` exits non-zero, this returns `(False, grouped)` right + away instead of burning through the full `timeout`, since + services that `up` never started will never settle. + - The `timeout` clock only starts once `up` finishes successfully, + so a stack whose healthchecks legitimately outlast `timeout` + isn't penalized for time `up` itself spent blocked on + healthcheck-gated `depends_on` dependencies. Omitting `up_done_fn` + preserves the old behavior of starting the clock immediately. + + `on_up_finished`, if given, is called exactly once, the first time + `up_done_fn` reports a successful result (exit code 0), so callers can + defer setup (e.g. starting a log tail) until `up` is done rather + than running it concurrently with `up`'s own output. + """ + if ps_fn is None: + def ps_fn() -> subprocess.CompletedProcess: + ps_cmd = ["docker", "compose", "-f", compose_path, "ps", "-a", "--format", "json"] + return subprocess.run(ps_cmd, capture_output=True, text=True) # nosec B603 # noqa: S603 + + def fetch_grouped() -> dict[str, list[str]]: + ps_result = ps_fn() + services = parse_compose_ps_json(ps_result.stdout) if ps_result.returncode == 0 else [] + return group_services_by_health(services) + + return _poll_until_settled( + fetch_grouped, + expected_service_count=expected_service_count, + poll_interval=poll_interval, + timeout=timeout, + use_color=use_color, + sleep_fn=sleep_fn, + now_fn=now_fn, + redraw_fn=redraw_fn, + up_done_fn=up_done_fn, + on_up_finished=on_up_finished, + ) diff --git a/tests/test_k8s_runner.py b/tests/test_k8s_runner.py index b1ee19c..4ede90c 100644 --- a/tests/test_k8s_runner.py +++ b/tests/test_k8s_runner.py @@ -78,7 +78,67 @@ def test_helm_up_removes_secret_file(self, mock_run, _mock_workloads) -> None: values_path = Path(command[command.index("--values") + 1]) self.assertFalse(values_path.exists()) self.assertNotIn("sentinel", " ".join(command)) - self.assertIn("--wait", command) + # Readiness is no longer decided by Helm's own --wait: it's decided by + # polling get_k8s_state()/group_services_by_health() below (the same + # pair `cds state` uses), so the apply step itself doesn't block on + # workload rollout. + self.assertNotIn("--wait", command) + + @patch("cli.k8s_runner.get_k8s_state", return_value=[]) + @patch("cli.k8s_runner.get_k8s_workloads", return_value=[]) + @patch("cli.k8s_runner.run_streamed", return_value=0) + def test_helm_up_reports_failure_when_workload_never_settles( + self, mock_run, mock_workloads, _mock_state + ) -> None: + plan: dict = {"secrets": {}} + mock_workloads.return_value = [ + {"kind": "Deployment", "metadata": {"name": "cds-web"}, "spec": {"replicas": 1}} + ] + + result = helm_up( + plan, + Path("chart"), + namespace="test", + release="cds", + kube_context=None, + timeout=0, + detach=False, + log_file=io.StringIO(), + sleep_fn=lambda _seconds: None, + ) + + self.assertEqual(result, 1) + self.assertEqual(mock_run.call_count, 1) + + @patch("cli.k8s_runner.get_k8s_state") + @patch("cli.k8s_runner.get_k8s_workloads") + @patch("cli.k8s_runner.run_streamed", return_value=0) + def test_helm_up_ready_state_matches_cds_state_health_grouping( + self, _mock_run, mock_workloads, mock_state + ) -> None: + # A release with one healthy Deployment settles as ready via the same + # get_k8s_state()/group_services_by_health() pair `cds state` uses. + workload = { + "kind": "Deployment", + "metadata": {"name": "cds-web"}, + "spec": {"replicas": 1}, + "status": {"readyReplicas": 1}, + } + mock_workloads.return_value = [workload] + mock_state.return_value = [{"Service": "cds-web", "Health": "HEALTHY", "State": "running"}] + + result = helm_up( + {"secrets": {}}, + Path("chart"), + namespace="test", + release="cds", + kube_context=None, + timeout=30, + detach=False, + log_file=io.StringIO(), + ) + + self.assertEqual(result, 0) @patch("cli.k8s_runner.get_k8s_workloads") @patch("cli.k8s_runner.run_streamed", return_value=0) diff --git a/tests/test_main.py b/tests/test_main.py index 5a01a7d..67b7024 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -13,6 +13,7 @@ from cli.image_updates import collect_module_images from cli.main import ( _collect_profile_env_vars, + _k8s_runtime_defaults, _resolve_profile_root, _run_image_verification, generate_profile, @@ -2251,6 +2252,68 @@ def test_up_helm_resolves_a_relative_chart_dir( self.assertTrue(Path(rendered_chart_dir).is_absolute()) self.assertEqual(Path(rendered_chart_dir), Path(tmpdir).resolve() / "relative-chart") + @patch("cli.main.default_log_path") + @patch("cli.main.helm_up", return_value=0) + @patch("cli.main._render_helm_chart", return_value=(0, [])) + @patch("cli.main.build_plan") + @patch("cli.main.validate_profile", return_value=[]) + def test_up_helm_wires_no_color_through_to_helm_up( + self, _mock_validate, mock_plan, _mock_render, mock_helm_up, mock_log_path + ): + plan = {"metadata": {"name": "demo"}, "runtime": {"namespace": "demo-ns"}, "modules": []} + mock_plan.return_value = (plan, []) + with tempfile.TemporaryDirectory() as tmpdir: + mock_log_path.return_value = Path(tmpdir) / "up.log" + with patch.dict( + os.environ, {"CDS_PROFILE_PATH": str(self.profiles_root)}, clear=False + ), patch.object( + sys, + "argv", + [ + "cds", + "up", + "local-dagster-postgres-superset", + "--target", + "helm", + "--no-color", + ], + ): + result = main() + + self.assertEqual(result, 0) + self.assertFalse(mock_helm_up.call_args.kwargs["use_color"]) + + @patch("cli.main.default_log_path") + @patch("cli.main.helm_up", return_value=0) + @patch("cli.main._render_helm_chart", return_value=(0, [])) + @patch("cli.main.build_plan") + @patch("cli.main.validate_profile", return_value=[]) + def test_up_helm_warns_that_no_build_has_no_effect( + self, _mock_validate, mock_plan, _mock_render, _mock_helm_up, mock_log_path + ): + plan = {"metadata": {"name": "demo"}, "runtime": {"namespace": "demo-ns"}, "modules": []} + mock_plan.return_value = (plan, []) + with tempfile.TemporaryDirectory() as tmpdir: + mock_log_path.return_value = Path(tmpdir) / "up.log" + with patch.dict( + os.environ, {"CDS_PROFILE_PATH": str(self.profiles_root)}, clear=False + ), patch.object( + sys, + "argv", + [ + "cds", + "up", + "local-dagster-postgres-superset", + "--target", + "helm", + "--no-build", + ], + ), contextlib.redirect_stdout(io.StringIO()) as stdout: + result = main() + + self.assertEqual(result, 0) + self.assertIn("--no-build has no effect with --target helm", stdout.getvalue()) + @patch("cli.main.default_log_path") @patch("cli.main.helm_down", return_value=0) def test_down_helm_retains_pvcs_by_default(self, mock_helm_down, mock_log_path): @@ -2276,6 +2339,65 @@ def test_down_helm_retains_pvcs_by_default(self, mock_helm_down, mock_log_path): self.assertEqual(result, 0) self.assertFalse(mock_helm_down.call_args.kwargs["delete_pvcs"]) + @patch("cli.main.default_log_path") + @patch("cli.main.helm_down", return_value=0) + def test_down_helm_accepts_environment_and_derives_matching_release( + self, mock_helm_down, mock_log_path + ): + # Regression test for #689: `down --environment prod` must derive + # the same release/namespace an `up --environment prod` run would + # have used, even though `down` never builds a full plan. + with tempfile.TemporaryDirectory() as tmpdir: + profile_dir = Path(tmpdir) / "profiles" / "overlay-demo" + (profile_dir / "environments").mkdir(parents=True) + (profile_dir / "profile.yaml").write_text( + """\ +apiVersion: cds/v1alpha1 +kind: Profile +metadata: + name: overlay-demo + environment: local +spec: + runtime: + type: docker-compose + namespace: base-ns + modules: [] +""", + encoding="utf-8", + ) + (profile_dir / "environments" / "prod.yaml").write_text( + """\ +metadata: + name: overlay-demo-prod +spec: + runtime: + namespace: prod-ns +""", + encoding="utf-8", + ) + + mock_log_path.return_value = Path(tmpdir) / "down.log" + with patch.dict( + os.environ, {"CDS_PROFILE_PATH": str(profile_dir.parent)}, clear=False + ), patch.object( + sys, + "argv", + [ + "cds", + "down", + "overlay-demo", + "--target", + "helm", + "--environment", + "prod", + ], + ): + result = main() + + self.assertEqual(result, 0) + self.assertEqual(mock_helm_down.call_args.kwargs["release"], "overlay-demo-prod") + self.assertEqual(mock_helm_down.call_args.kwargs["namespace"], "prod-ns") + class CollectModuleImagesTest(unittest.TestCase): @@ -2843,5 +2965,101 @@ def test_unexpected_error_during_plan_render_reports_e095(self, mock_resolve_pro self.assertIn("E095", stderr.getvalue()) +class K8sRuntimeDefaultsTest(unittest.TestCase): + """Directly exercises `_k8s_runtime_defaults`, the release/namespace + resolver shared by `cds down --target helm` and `cds state --target + helm`. See #689: this must derive the same identity `cds up` would + have used for the same profile/--environment combination, without + requiring the profile to currently pass full validation.""" + + def _write_profile(self, profile_dir: Path, name: str = "demo", namespace: str | None = None) -> None: + (profile_dir).mkdir(parents=True, exist_ok=True) + runtime_lines = " type: docker-compose\n" + if namespace: + runtime_lines += f" namespace: {namespace}\n" + (profile_dir / "profile.yaml").write_text( + f"""\ +apiVersion: cds/v1alpha1 +kind: Profile +metadata: + name: {name} + environment: local +spec: + runtime: +{runtime_lines} modules: [] +""", + encoding="utf-8", + ) + + def test_reads_base_profile_without_an_environment(self): + with tempfile.TemporaryDirectory() as tmpdir: + profile_dir = Path(tmpdir) / "demo" + self._write_profile(profile_dir, name="demo", namespace="demo-ns") + + release, namespace = _k8s_runtime_defaults(str(profile_dir / "profile.yaml")) + + self.assertEqual(release, "demo") + self.assertEqual(namespace, "demo-ns") + + def test_environment_overlay_changes_release_and_namespace_like_the_plan_would(self): + with tempfile.TemporaryDirectory() as tmpdir: + profile_dir = Path(tmpdir) / "demo" + self._write_profile(profile_dir, name="demo", namespace="demo-ns") + (profile_dir / "environments").mkdir() + (profile_dir / "environments" / "prod.yaml").write_text( + """\ +metadata: + name: demo-prod +spec: + runtime: + namespace: demo-prod-ns +""", + encoding="utf-8", + ) + + release, namespace = _k8s_runtime_defaults( + str(profile_dir / "profile.yaml"), environment="prod" + ) + + self.assertEqual(release, "demo-prod") + self.assertEqual(namespace, "demo-prod-ns") + + def test_falls_back_to_directory_name_when_module_validation_fails(self): + # A profile referencing a nonexistent module still has a valid + # metadata.name/runtime.namespace shape; best-effort resolution + # must still surface those instead of falling back to generic + # cds-local/directory-name defaults, since the release/namespace + # a stack is actually running under doesn't depend on whether the + # profile currently validates cleanly. + with tempfile.TemporaryDirectory() as tmpdir: + profile_dir = Path(tmpdir) / "demo" + profile_dir.mkdir() + (profile_dir / "profile.yaml").write_text( + """\ +apiVersion: cds/v1alpha1 +kind: Profile +metadata: + name: demo + environment: local +spec: + runtime: + type: docker-compose + namespace: demo-ns + modules: + - id: missing + source: does/not-exist + version: "0.1.0" + enabled: true + config: {} +""", + encoding="utf-8", + ) + + release, namespace = _k8s_runtime_defaults(str(profile_dir / "profile.yaml")) + + self.assertEqual(release, "demo") + self.assertEqual(namespace, "demo-ns") + + if __name__ == "__main__": unittest.main()