diff --git a/agents.md b/agents.md index f867d73..ed59b8e 100644 --- a/agents.md +++ b/agents.md @@ -25,7 +25,7 @@ shard_core/ service/ → Business logic app_installation/ App install/uninstall/reinstall + background worker queue app_lifecycle.py Two-tier idle control: RUNNING -> PAUSED -> STOPPED, PSI-driven LRU demotion, wake-on-request - app_tools.py Docker CLI wrapper functions (start/stop/pause/unpause/down) + app_tools.py Docker CLI wrappers (pause/unpause/stop/down); start_app is the idempotent revive primitive that decides unpause vs up from real container state memory_pressure.py PSI parsing (/host/pressure/memory), cgroup v2 memory.reclaim page-out pause_metrics.py In-memory pause-tier telemetry accumulators (transitions, latencies, PSI snapshots) pairing.py Terminal pairing (JWT creation, code generation) diff --git a/shard_core/data_model/backend/shard_model.py b/shard_core/data_model/backend/shard_model.py index 705257d..2750413 100644 --- a/shard_core/data_model/backend/shard_model.py +++ b/shard_core/data_model/backend/shard_model.py @@ -4,7 +4,7 @@ from enum import StrEnum, auto from typing import List -from pydantic import BaseModel +from pydantic import BaseModel, field_validator from .permission_model import PermissionHolder from .subscription_model import SubscriptionStatus @@ -102,6 +102,8 @@ class ShardBase(BaseModel): price_cents: int | None = None pending_vm_size: VmSize | None = None pending_price_cents: int | None = None + config_overrides_desired: dict[str, str] = {} + config_overrides_applied: dict[str, str] = {} @property def short_id(self) -> str: @@ -226,6 +228,29 @@ class PairingCodeResponse(BaseModel): valid_until: datetime +class ConfigOverrideKey(StrEnum): + """Allowlist of override keys an operator may set. Typing the request key as + this enum is the single source of truth: it rejects unknown keys with 422 and + surfaces the allowed set in the OpenAPI schema, so the frontend dropdown is + generated from it. Adding an override key = one entry here. Each value is a + valid env-var name by construction, so no separate format validation is + needed.""" + + PAUSE_ENABLED = "PAUSE_ENABLED" + + +class ConfigOverrideRequest(BaseModel): + key: ConfigOverrideKey + value: str + + @field_validator("value") + @classmethod + def _value_has_no_newline(cls, v: str) -> str: + if "\n" in v or "\r" in v: + raise ValueError("value must not contain newlines") + return v + + class CoreUpdateRequest(BaseModel): target_version: str diff --git a/shard_core/service/app_installation/worker.py b/shard_core/service/app_installation/worker.py index 257ea14..ae329c9 100644 --- a/shard_core/service/app_installation/worker.py +++ b/shard_core/service/app_installation/worker.py @@ -12,6 +12,7 @@ from shard_core.database import installed_apps as db_installed_apps from shard_core.data_model.app_meta import Status from shard_core.service.app_tools import ( + app_op_lock, get_installed_apps_path, docker_create_app_containers, docker_stop_app, @@ -123,31 +124,36 @@ async def _install_app_from_existing_zip(app_name: str): async def _uninstall_app(app_name: str): - try: - installed_app = await get_app_from_db(app_name) - if installed_app.status == Status.PAUSED: - # unfreeze while the status still says PAUSED — once it flips to - # UNINSTALLING nothing knows the containers are frozen, and a - # frozen stack can be neither stopped nor removed - await docker_unpause_app(app_name) - await update_app_status(app_name, Status.UNINSTALLING) - except KeyError: - log.warning(f"during uninstallation of {app_name}: app not found in database") - - try: - await docker_stop_app(app_name) - await docker_shutdown_app(app_name) - except Exception as e: - log.error(f"Error while shutting down app {app_name}: {e!r}") - - log.debug(f"deleting app data for {app_name}") - shutil.rmtree(Path(get_installed_apps_path() / app_name), ignore_errors=True) - log.debug(f"removing app {app_name} from database") - async with db_conn() as conn: - await db_installed_apps.remove(conn, app_name) - await write_traefik_dyn_config() - await signals.on_apps_update.send_async() - log.info(f"uninstalled {app_name}") + # Held across the whole teardown so a concurrent wake/control-tick revive + # (start_app) can't recreate a container between our stop and row removal. + async with app_op_lock(app_name): + try: + installed_app = await get_app_from_db(app_name) + if installed_app.status == Status.PAUSED: + # unfreeze while the status still says PAUSED — once it flips to + # UNINSTALLING nothing knows the containers are frozen, and a + # frozen stack can be neither stopped nor removed + await docker_unpause_app(app_name) + await update_app_status(app_name, Status.UNINSTALLING) + except KeyError: + log.warning( + f"during uninstallation of {app_name}: app not found in database" + ) + + try: + await docker_stop_app(app_name) + await docker_shutdown_app(app_name) + except Exception as e: + log.error(f"Error while shutting down app {app_name}: {e!r}") + + log.debug(f"deleting app data for {app_name}") + shutil.rmtree(Path(get_installed_apps_path() / app_name), ignore_errors=True) + log.debug(f"removing app {app_name} from database") + async with db_conn() as conn: + await db_installed_apps.remove(conn, app_name) + await write_traefik_dyn_config() + await signals.on_apps_update.send_async() + log.info(f"uninstalled {app_name}") async def _reinstall_app(app_name: str): diff --git a/shard_core/service/app_lifecycle.py b/shard_core/service/app_lifecycle.py index 79e690e..3b652aa 100644 --- a/shard_core/service/app_lifecycle.py +++ b/shard_core/service/app_lifecycle.py @@ -8,10 +8,9 @@ from shard_core.data_model.app_meta import InstalledApp, Status from shard_core.service import disk, memory_pressure, pause_metrics from shard_core.service.app_tools import ( - docker_start_app, + start_app, docker_stop_app, docker_pause_app, - docker_unpause_app, get_app_metadata, size_is_compatible, ) @@ -35,16 +34,12 @@ async def ensure_app_is_running(app: InstalledApp): if await size_is_compatible(app_meta.minimum_portal_size): global last_access_dict last_access_dict[app.name] = time.time() - # PAUSED wakes via unpause (ms to ~2s), everything else via the legacy - # start path. Calling docker_unpause_app directly also dodges the - # global 5s throttle on docker_start_app, which would silently drop - # the wake. This dispatch stays active even with pause_enabled=false, - # so already-paused apps still wake after a backout. - if app.status == Status.PAUSED: - coro = docker_unpause_app(app.name) - else: - coro = docker_start_app(app.name) - task = asyncio.create_task(coro, name=f"ensure {app.name} is running") + # One idempotent revive primitive decides unpause vs up from the real + # container state, so a paused stack unfreezes and an out-of-band exit + # (crash, OOM, core-upgrade converge) still starts instead of 502-looping. + task = asyncio.create_task( + start_app(app.name), name=f"ensure {app.name} is running" + ) background_tasks.add(task) task.add_done_callback(background_tasks.discard) @@ -84,7 +79,7 @@ async def _control_app_time(app: InstalledApp, pause_enabled: bool): if app.status != Status.RUNNING and await size_is_compatible( app_meta.minimum_portal_size ): - await docker_start_app(app.name) + await start_app(app.name) return idle = time.time() - last_access_dict.get(app.name, 0.0) diff --git a/shard_core/service/app_tools.py b/shard_core/service/app_tools.py index b7fdc8c..1b18203 100644 --- a/shard_core/service/app_tools.py +++ b/shard_core/service/app_tools.py @@ -2,7 +2,9 @@ import json import logging import time +from collections import defaultdict from pathlib import Path +from typing import Literal import shard_core.data_model.profile from shard_core.database.connection import db_conn @@ -31,41 +33,135 @@ async def docker_create_app_containers(name: str): await subprocess(*_app_compose(name), "up", "--no-start") -@throttle(5) -async def docker_start_app(name: str): - async with db_conn() as conn: - app = await db_installed_apps.get_by_name(conn, name) - app_status = app["status"] if app else None +ContainerState = Literal["running", "paused", "exited", "missing"] - if app_status == Status.PAUSED: - # a paused app wakes by unfreezing, not by compose up - await docker_unpause_app(name) - return +# Statuses a lifecycle revive may act on. Transitional/terminal ones +# (INSTALLING, *_QUEUED, UNINSTALLING, REINSTALLING, ERROR) must never be +# started, or the control tick recreates containers mid-uninstall/reinstall. +_REVIVABLE_STATUS = (Status.STOPPED, Status.RUNNING, Status.DOWN, Status.PAUSED) - if app_status in [Status.STOPPED, Status.RUNNING, Status.DOWN]: - log.debug(f"starting app {name=}") - try: +# Per-app op lock serializing revive against teardown: start_app and the +# uninstall worker both take it, so a wake-on-access or control-tick revive can +# never recreate a container the uninstall worker just removed (issue #185). +_app_op_locks: dict[str, asyncio.Lock] = defaultdict(asyncio.Lock) + + +def app_op_lock(name: str) -> asyncio.Lock: + return _app_op_locks[name] + + +async def get_app_container_state(name: str) -> ContainerState: + """Real docker state of an app's containers: running | paused | exited | missing. + + Reads the daemon rather than the stored app status, so a caller can revive an + app whose containers changed state out-of-band (crash, OOM, core-upgrade + converge stop) while the database still says PAUSED or RUNNING. + """ + compose = _app_compose(name) + try: + ids_out = await subprocess(*compose, "ps", "-a", "-q") + ids = [line.strip() for line in ids_out.splitlines() if line.strip()] + if not ids: + return "missing" + states_out = await subprocess( + "docker", "inspect", "--format", "{{.State.Status}}", *ids + ) + except SubprocessError: + return "missing" + states = [line.strip() for line in states_out.splitlines() if line.strip()] + if any(s == "paused" for s in states): + return "paused" + if states and all(s == "running" for s in states): + return "running" + return "exited" + + +async def _compose_up(name: str): + try: + await subprocess(*_app_compose(name), "up", "-d") + except SubprocessError as e: + if "network" in str(e) and "not found" in str(e): + log.warning( + f"stale network reference for app {name=}, recreating containers" + ) + await subprocess(*_app_compose(name), "down") + await subprocess(*_app_compose(name), "up", "-d") + elif "Conflict" in str(e) and "already in use" in str(e): + log.warning(f"stale containers for app {name=}, removing and recreating") + await subprocess(*_app_compose(name), "down") await subprocess(*_app_compose(name), "up", "-d") - except SubprocessError as e: - if "network" in str(e) and "not found" in str(e): + else: + raise + + +async def _do_unpause(name: str): + unpause_started = time.monotonic() + await subprocess(*_app_compose(name), "unpause") + pause_metrics.record_unpause_latency((time.monotonic() - unpause_started) * 1000) + pause_metrics.record_app_transition(name, Status.PAUSED, Status.RUNNING) + + +async def _mark_running(name: str): + async with db_conn() as conn: + await db_installed_apps.update_status(conn, name, Status.RUNNING) + await signals.on_apps_update.send_async() + + +@throttle(5) +async def start_app(name: str) -> None: + """Bring a revivable app up, deciding from the real container state. + + The single revive primitive for wake-on-access and the always-on control + tick. It acts only on a revivable app (see _REVIVABLE_STATUS) and dispatches + on the real container state, not the stored status, so an app that exited + out-of-band while the database still says PAUSED still starts (via up) rather + than erroring on unpause. Idempotent: an already-running stack is a no-op. + + Holds the per-app op lock so a revive can never race the uninstall worker's + teardown and recreate a container it just removed (issue #185). + """ + async with app_op_lock(name): + async with db_conn() as conn: + app = await db_installed_apps.get_by_name(conn, name) + db_status = app["status"] if app else None + if db_status not in _REVIVABLE_STATUS: + log.debug(f"app {name=} has {db_status=}, skipping start") + return + + state = await get_app_container_state(name) + + if state == "running": + if db_status != Status.RUNNING: log.warning( - f"stale network reference for app {name=}, recreating containers" + f"app {name=} container is running but db says {db_status=}; reconciling" ) - await subprocess(*_app_compose(name), "down") - await subprocess(*_app_compose(name), "up", "-d") - elif "Conflict" in str(e) and "already in use" in str(e): + await _mark_running(name) + return + + if state == "paused": + if db_status != Status.PAUSED: log.warning( - f"stale containers for app {name=}, removing and recreating" + f"app {name=} container is paused but db says {db_status=}; unpausing" ) - await subprocess(*_app_compose(name), "down") - await subprocess(*_app_compose(name), "up", "-d") - else: - raise - async with db_conn() as conn: - await db_installed_apps.update_status(conn, name, Status.RUNNING) - await signals.on_apps_update.send_async() - else: - log.debug(f"app {name=} has status {app_status}, skipping start") + log.debug(f"unpausing app {name=}") + try: + await _do_unpause(name) + except SubprocessError: + # a partially-paused stack (some containers already exited) can't + # be revived by unpause — fall back to a plain start + log.warning(f"unpause failed for {name=}, starting instead") + await _compose_up(name) + await _mark_running(name) + return + + # exited / created / missing + if db_status in (Status.RUNNING, Status.PAUSED): + log.warning( + f"app {name=} container is {state} but db says {db_status=}; starting it" + ) + log.debug(f"starting app {name=}") + await _compose_up(name) + await _mark_running(name) async def docker_pause_app(name: str): @@ -93,15 +189,8 @@ async def docker_unpause_app(name: str): app_status = app["status"] if app else None if app_status == Status.PAUSED: log.debug(f"unpausing app {name=}") - unpause_started = time.monotonic() - await subprocess(*_app_compose(name), "unpause") - pause_metrics.record_unpause_latency( - (time.monotonic() - unpause_started) * 1000 - ) - pause_metrics.record_app_transition(name, Status.PAUSED, Status.RUNNING) - async with db_conn() as conn: - await db_installed_apps.update_status(conn, name, Status.RUNNING) - await signals.on_apps_update.send_async() + await _do_unpause(name) + await _mark_running(name) else: log.debug(f"app {name=} has {app_status=}, skipping unpause") diff --git a/shard_core/util/misc.py b/shard_core/util/misc.py index 5253758..a3207aa 100644 --- a/shard_core/util/misc.py +++ b/shard_core/util/misc.py @@ -3,23 +3,32 @@ def throttle(min_duration: float): + """Throttle calls per distinct positional-args key. + + A call whose positional args match one made within the last min_duration + seconds is dropped (returns None); different args throttle independently, so + throttling one app's operation never drops another app's call. Keyed on + positional args only — callers that vary a throttled arg by keyword collapse + to one key. One entry is retained per distinct args tuple. + """ + def decorator_throttle(func): - last_call = None + last_call: dict[tuple, float] = {} if inspect.iscoroutinefunction(func): async def wrapper_throttle(*args, **kwargs): - nonlocal last_call - if last_call is None or last_call + min_duration < time.time(): - last_call = time.time() + prev = last_call.get(args) + if prev is None or prev + min_duration < time.time(): + last_call[args] = time.time() return await func(*args, **kwargs) else: def wrapper_throttle(*args, **kwargs): - nonlocal last_call - if last_call is None or last_call + min_duration < time.time(): - last_call = time.time() + prev = last_call.get(args) + if prev is None or prev + min_duration < time.time(): + last_call[args] = time.time() return func(*args, **kwargs) return wrapper_throttle diff --git a/tests/test_app_compose_pinning.py b/tests/test_app_compose_pinning.py index 8b2793b..ecb17dc 100644 --- a/tests/test_app_compose_pinning.py +++ b/tests/test_app_compose_pinning.py @@ -5,6 +5,7 @@ project "core" — stopping shard_core itself (issue #160). """ +import asyncio from pathlib import Path from unittest.mock import AsyncMock, patch @@ -17,6 +18,7 @@ from shard_core.util.subprocess import ( ComposeFileNotFound, ComposeProjectNotAllowed, + SubprocessError, app_compose_command, normalize_project_name, ) @@ -95,11 +97,11 @@ def subprocess_mock(): @pytest.fixture(autouse=True) def reset_start_throttle(): - """docker_start_app's @throttle(5) is global across apps and tests — a start - triggered by an earlier test would silently drop our call.""" - wrapper = app_tools.docker_start_app + """start_app's @throttle(5) is per-app but persists across tests — a start + of the same app in an earlier test would silently drop our call.""" + wrapper = app_tools.start_app cell = wrapper.__closure__[wrapper.__code__.co_freevars.index("last_call")] - cell.cell_contents = None + cell.cell_contents.clear() async def _insert_app(name: str, status: Status): @@ -113,7 +115,7 @@ async def _insert_app(name: str, status: Status): "operation, status", [ (app_tools.docker_create_app_containers, Status.STOPPED), - (app_tools.docker_start_app, Status.STOPPED), + (app_tools.start_app, Status.STOPPED), (app_tools.docker_pause_app, Status.RUNNING), (app_tools.docker_unpause_app, Status.PAUSED), (app_tools.docker_stop_app, Status.RUNNING), @@ -148,3 +150,229 @@ async def test_app_operation_with_compose_file_pins_the_app_project( assert "-f" in command and command[command.index("-f") + 1] == str( app_dir / "docker-compose.yml" ) + + +async def _status(name: str) -> str | None: + async with db_conn() as conn: + app = await db_installed_apps.get_by_name(conn, name) + return app["status"] if app else None + + +def _issued_commands(subprocess_mock) -> list[tuple]: + return [call.args for call in subprocess_mock.await_args_list] + + +@pytest.mark.parametrize( + "ps_ids, inspect_states, expected", + [ + ("cid1\ncid2\n", "running\nrunning\n", "running"), + ("cid1\ncid2\n", "running\npaused\n", "paused"), + ("cid1\ncid2\n", "paused\nexited\n", "paused"), + ("cid1\ncid2\n", "running\nexited\n", "exited"), + ("cid1\n", "created\n", "exited"), + ("", None, "missing"), + ], +) +async def test_get_app_container_state_categorizes_real_state( + tmp_path, subprocess_mock, ps_ids, inspect_states, expected +): + _app_dir(tmp_path, "app1") + if inspect_states is None: + subprocess_mock.side_effect = [ps_ids] + else: + subprocess_mock.side_effect = [ps_ids, inspect_states] + + with settings_override({"path_root": str(tmp_path)}): + state = await app_tools.get_app_container_state("app1") + + assert state == expected + + +async def test_get_app_container_state_missing_when_ps_fails(tmp_path, subprocess_mock): + _app_dir(tmp_path, "app1") + subprocess_mock.side_effect = SubprocessError("no such project") + + with settings_override({"path_root": str(tmp_path)}): + state = await app_tools.get_app_container_state("app1") + + assert state == "missing" + + +async def test_start_app_revives_exited_app_even_when_db_says_paused( + db, tmp_path, subprocess_mock +): + """The #185 bug: db says PAUSED but the container exited (crash / OOM / + core-upgrade converge). The old wake ran `unpause` and crashed; revive must + `up -d` instead.""" + _app_dir(tmp_path, "exited_app") + await _insert_app("exited_app", Status.PAUSED) + + with ( + settings_override({"path_root": str(tmp_path)}), + patch.object( + app_tools, "get_app_container_state", new=AsyncMock(return_value="exited") + ), + ): + await app_tools.start_app("exited_app") + + commands = _issued_commands(subprocess_mock) + assert any(c[-2:] == ("up", "-d") for c in commands) + assert not any("unpause" in c for c in commands) + assert await _status("exited_app") == Status.RUNNING + + +async def test_start_app_unpauses_a_genuinely_paused_app(db, tmp_path, subprocess_mock): + _app_dir(tmp_path, "paused_app") + await _insert_app("paused_app", Status.PAUSED) + + with ( + settings_override({"path_root": str(tmp_path)}), + patch.object( + app_tools, "get_app_container_state", new=AsyncMock(return_value="paused") + ), + ): + await app_tools.start_app("paused_app") + + commands = _issued_commands(subprocess_mock) + assert any(c[-1] == "unpause" for c in commands) + assert not any(c[-2:] == ("up", "-d") for c in commands) + assert await _status("paused_app") == Status.RUNNING + + +async def test_start_app_running_container_is_a_noop_but_reconciles_status( + db, tmp_path, subprocess_mock +): + _app_dir(tmp_path, "running_app") + await _insert_app("running_app", Status.PAUSED) + + with ( + settings_override({"path_root": str(tmp_path)}), + patch.object( + app_tools, "get_app_container_state", new=AsyncMock(return_value="running") + ), + ): + await app_tools.start_app("running_app") + + subprocess_mock.assert_not_called() + assert await _status("running_app") == Status.RUNNING + + +async def test_start_app_starts_a_missing_stack(db, tmp_path, subprocess_mock): + _app_dir(tmp_path, "gone_app") + await _insert_app("gone_app", Status.DOWN) + + with ( + settings_override({"path_root": str(tmp_path)}), + patch.object( + app_tools, "get_app_container_state", new=AsyncMock(return_value="missing") + ), + ): + await app_tools.start_app("gone_app") + + commands = _issued_commands(subprocess_mock) + assert any(c[-2:] == ("up", "-d") for c in commands) + assert await _status("gone_app") == Status.RUNNING + + +@pytest.mark.parametrize( + "status", [Status.ERROR, Status.UNINSTALLING, Status.INSTALLING] +) +async def test_start_app_skips_non_revivable_status( + db, tmp_path, subprocess_mock, status +): + """A revive must never start an app that is being uninstalled/reinstalled or + is in ERROR — otherwise the always-on control tick recreates its containers + mid-teardown.""" + _app_dir(tmp_path, "term_app") + await _insert_app("term_app", status) + + with settings_override({"path_root": str(tmp_path)}): + await app_tools.start_app("term_app") + + subprocess_mock.assert_not_called() + assert await _status("term_app") == status + + +async def test_start_app_falls_back_to_up_when_unpause_fails( + db, tmp_path, subprocess_mock +): + """A partially-paused stack (some containers already exited) can't be revived + by unpause — start_app must fall back to `up -d` instead of crashing.""" + _app_dir(tmp_path, "mixed_app") + await _insert_app("mixed_app", Status.PAUSED) + subprocess_mock.side_effect = [ + SubprocessError("Container mixed_app is not paused"), # unpause + "", # up -d fallback + ] + + with ( + settings_override({"path_root": str(tmp_path)}), + patch.object( + app_tools, "get_app_container_state", new=AsyncMock(return_value="paused") + ), + ): + await app_tools.start_app("mixed_app") + + commands = _issued_commands(subprocess_mock) + assert commands[0][-1] == "unpause" + assert commands[-1][-2:] == ("up", "-d") + assert await _status("mixed_app") == Status.RUNNING + + +@pytest.mark.parametrize( + "error_text", ["network portal not found", "Conflict already in use"] +) +async def test_start_app_recovers_stale_containers( + db, tmp_path, subprocess_mock, error_text +): + _app_dir(tmp_path, "stale_app") + await _insert_app("stale_app", Status.DOWN) + subprocess_mock.side_effect = [ + SubprocessError(error_text), # first up -d + "", # down + "", # up -d retry + ] + + with ( + settings_override({"path_root": str(tmp_path)}), + patch.object( + app_tools, "get_app_container_state", new=AsyncMock(return_value="exited") + ), + ): + await app_tools.start_app("stale_app") + + commands = _issued_commands(subprocess_mock) + assert commands[0][-2:] == ("up", "-d") + assert commands[1][-1] == "down" + assert commands[2][-2:] == ("up", "-d") + assert await _status("stale_app") == Status.RUNNING + + +async def test_start_app_is_serialized_by_the_per_app_op_lock( + db, tmp_path, subprocess_mock +): + """start_app and an uninstall teardown must not interleave: both hold the + per-app op lock, so a revive can't recreate a container after the uninstall + worker removed it (issue #185 follow-up). While the lock is held, start_app + blocks and issues no compose command.""" + _app_dir(tmp_path, "locked_app") + await _insert_app("locked_app", Status.RUNNING) + + lock = app_tools.app_op_lock("locked_app") + assert app_tools.app_op_lock("locked_app") is lock # one lock per app name + + with ( + settings_override({"path_root": str(tmp_path)}), + patch.object( + app_tools, "get_app_container_state", new=AsyncMock(return_value="missing") + ), + ): + await lock.acquire() + try: + task = asyncio.create_task(app_tools.start_app("locked_app")) + await asyncio.sleep(0.05) + assert not task.done() # blocked on the lock, no compose up yet + subprocess_mock.assert_not_called() + finally: + lock.release() + await asyncio.wait_for(task, timeout=1) diff --git a/tests/test_app_lifecycle.py b/tests/test_app_lifecycle.py index 45a7a92..18bcb32 100644 --- a/tests/test_app_lifecycle.py +++ b/tests/test_app_lifecycle.py @@ -2,9 +2,26 @@ from fastapi import status from shard_core.data_model.app_meta import InstalledApp, Status +from shard_core.service.app_tools import get_app_container_state from tests.util import retry_async, wait_until_app_installed +async def test_get_app_container_state_reads_real_created_container( + requests_mock, api_client +): + """Pins get_app_container_state against real docker output: a freshly + installed app has created-but-not-started containers, which the revive path + must classify as needing a start (not 'running').""" + app_name = "quick_stop" + + response = await api_client.post(f"protected/apps/{app_name}") + assert response.status_code == status.HTTP_201_CREATED + await wait_until_app_installed(api_client, app_name) + + assert docker.from_env().containers.get(app_name).status == "created" + assert await get_app_container_state(app_name) == "exited" + + async def test_app_starts_and_stops(requests_mock, api_client): docker_client = docker.from_env() app_name = "quick_stop" diff --git a/tests/test_app_lifecycle_pause.py b/tests/test_app_lifecycle_pause.py index 23692fa..e411ff4 100644 --- a/tests/test_app_lifecycle_pause.py +++ b/tests/test_app_lifecycle_pause.py @@ -5,6 +5,7 @@ (that lives in the integration tests). """ +import asyncio import time from unittest.mock import AsyncMock, patch @@ -36,15 +37,14 @@ def _meta(lifecycle: Lifecycle) -> AppMeta: @pytest.fixture def docker_mocks(): with ( - patch.object(app_lifecycle, "docker_start_app", new=AsyncMock()) as start, + patch.object(app_lifecycle, "start_app", new=AsyncMock()) as start, patch.object(app_lifecycle, "docker_stop_app", new=AsyncMock()) as stop, patch.object(app_lifecycle, "docker_pause_app", new=AsyncMock()) as pause, - patch.object(app_lifecycle, "docker_unpause_app", new=AsyncMock()) as unpause, patch.object( app_lifecycle, "size_is_compatible", new=AsyncMock(return_value=True) ), ): - yield {"start": start, "stop": stop, "pause": pause, "unpause": unpause} + yield {"start": start, "stop": stop, "pause": pause} @pytest.fixture(autouse=True) @@ -63,6 +63,30 @@ def _app(name: str, status: Status, idle: float) -> InstalledApp: # tests/config.toml: default_idle_for_pause=5, default_idle_for_stop=12 +@pytest.mark.parametrize( + "status", [Status.PAUSED, Status.STOPPED, Status.DOWN, Status.RUNNING] +) +async def test_wake_routes_every_status_through_start_app(docker_mocks, status): + # Previously a PAUSED app woke via docker_unpause_app, which 502-looped when + # the container had actually exited (#185). Every status now goes through the + # one real-state revive primitive. + app = _app("a", status, idle=0) + with ( + settings_override(PAUSE_ON), + patch.object( + app_lifecycle, "get_app_metadata", return_value=_meta(Lifecycle()) + ), + patch.object( + app_lifecycle.disk, + "current_disk_usage", + app_lifecycle.disk.DiskUsage(total_gb=10, free_gb=9, disk_space_low=False), + ), + ): + await app_lifecycle.ensure_app_is_running(app) + await asyncio.sleep(0) + docker_mocks["start"].assert_awaited_once_with("a") + + async def test_running_app_pauses_after_t1(docker_mocks): app = _app("a", Status.RUNNING, idle=7) with ( diff --git a/tests/test_throttle.py b/tests/test_throttle.py index df1e9e8..57fbdbc 100644 --- a/tests/test_throttle.py +++ b/tests/test_throttle.py @@ -28,3 +28,14 @@ def call_me(): assert call_me() == "called" time.sleep(0.2) assert call_me() == "called" + + +def test_throttle_is_per_argument(): + @throttle(0.1) + def call_me(key): + return "called" + + # throttling one key must not drop a call for a different key + assert call_me("a") == "called" + assert call_me("a") is None + assert call_me("b") == "called"