diff --git a/README.md b/README.md index 8fcad2a..f90f7d1 100644 --- a/README.md +++ b/README.md @@ -469,6 +469,12 @@ it is what your saved destination credentials are encrypted with. holds across workers and across a restart) and a second one gets `409` while it is held; auto-update skips a stack somebody is already deploying. Locks carry an expiry, so a worker killed mid-deploy does not strand a stack. +- **A stack is never stuck behind its own lock** (0.67.0) — the stack page says + what holds it and since when, and offers to stop it. A running backup is + asked to stop, throws its half-written archive away and starts the containers + it stopped; a lock left behind by a backup that is no longer running is + released on one click; anything else can be force-released. A restart clears + every lock, because after it nothing of ours is running. - **Resilient UI** — a render error shows what broke and offers a way out instead of blanking the window, and clears itself when you navigate away. Routes are code-split, so the entry bundle is 377 kB rather than 841 kB and @@ -599,6 +605,12 @@ it is what your saved destination credentials are encrypted with. cannot be stopped is not backed up. The stack is locked meanwhile, so a deploy, update or scheduled backup cannot interleave (a scheduled backup that finds the stack busy retries after 10 minutes). + - A backup can be stopped while it runs (0.67.0). It is a long operation + that holds the stack — up to twelve hours for a large volume — so the stack + page offers to cancel it: the helper container it streams through is + removed, the archive is thrown away, and the containers it stopped are + started again. A restore is not interruptible (it is halfway through + writing a stack's data back); its lock can only be force-released. - Remote storage is kept out by what it *is*, not only by how it is declared: NFS/CIFS volumes, bind folders on a mounted share (`/mnt/nas/…` via fstab or autofs), bind-type volumes pointing at one, and shares mounted inside the @@ -1040,7 +1052,12 @@ leaked one. that moved into the database: that a busy stack answers 409 without ever reaching Docker, that an expired lock is taken over rather than stranding the stack, that the stats cache serves repeat callers from one sweep, and that the -update cache and rate limiter survive a restart. One of them asserts that +update cache and rate limiter survive a restart. Since 0.67.0 they also cover +the way out of a lock — a backup that is asked to stop, a leftover backup lock +that is released on one click, a deploy lock that takes an explicit force, and +a restore that refuses to be interrupted at all — and +`tests/test_backup_restore.py` that a cancelled backup starts the stack again, +frees the lock and leaves no half-written archive behind. One of them asserts that `update_service` never imports the database — it is pure registry logic, and persistence stays opt-in so the module remains testable without one. diff --git a/backend/main.py b/backend/main.py index 2ad56d5..c34e46a 100644 --- a/backend/main.py +++ b/backend/main.py @@ -92,13 +92,20 @@ async def lifespan(app: FastAPI): except Exception as exc: # noqa: BLE001 - Docker may not be up yet logger.warning("Could not clean up helper containers: %s", exc) # Runtime state that used to live in module dicts and was lost on restart. + # Stack locks mean "an operation is running"; after a restart none is, so + # all of them go — not just the expired ones. A backup holds its lock for + # twelve hours, and one that died with its process used to leave the stack + # un-startable for the rest of them. try: with Session(engine) as session: - stale = stack_lock_service.prune_expired(session) + stale = stack_lock_service.clear_all(session) if stale: - logger.info("Cleared %d stale stack lock(s) from a previous run", stale) + logger.info( + "Cleared %d stack lock(s) left over from a previous run: %s", + len(stale), ", ".join(sorted(stale)), + ) except Exception as exc: # noqa: BLE001 - logger.warning("Could not prune stack locks: %s", exc) + logger.warning("Could not clear stack locks: %s", exc) try: restored = image_status_store.install() logger.info("Restored %d cached image update status(es)", restored) diff --git a/backend/routers/backups.py b/backend/routers/backups.py index 24cb908..d6fad00 100644 --- a/backend/routers/backups.py +++ b/backend/routers/backups.py @@ -65,8 +65,11 @@ def _summary(report: dict) -> str: def _backup_http_error(exc: backup_service.BackupError) -> HTTPException: - # 409 for a stack that is busy or already exists, 400 for the rest. - conflict = isinstance(exc, backup_service.StackBusyError) or "already exists" in str(exc) + # 409 for a stack that is busy, a backup somebody stopped, or a target that + # already exists; 400 for the rest. + conflict = isinstance( + exc, (backup_service.StackBusyError, backup_service.BackupCancelled) + ) or "already exists" in str(exc) return HTTPException(status_code=409 if conflict else 400, detail=str(exc)) diff --git a/backend/routers/stacks.py b/backend/routers/stacks.py index dba2655..c1588d8 100644 --- a/backend/routers/stacks.py +++ b/backend/routers/stacks.py @@ -1,6 +1,8 @@ """Stack CRUD + lifecycle endpoints.""" from __future__ import annotations +import asyncio +import logging import os from dataclasses import asdict @@ -34,12 +36,15 @@ from services import ( icon_service, logo_service, notify_service, + stack_job_service, stack_lock_service, stats_service, update_service, ) from services.convert_service import convert_docker_run +logger = logging.getLogger("stackpilot.stacks") + router = APIRouter(prefix="/api/stacks", tags=["stacks"]) @@ -77,6 +82,34 @@ def _auto_icon(stack: Stack) -> str | None: return f"logo:{slug}" if slug else None +def _lock_view(lock: dict | None, stack_id: str) -> dict | None: + """The lock plus what can be done about it. + + ``cancellable`` is true only while the operation is actually running in + this process and can stop halfway (a backup can, a restore must not). + Everything else can still be force-released — that drops the guard without + stopping anything, which is the last resort for a lock whose operation is + long gone. + """ + if not lock: + return None + job = stack_job_service.get(stack_id) + return { + **lock, + "cancellable": bool(job and job.cancellable and not job.cancelled), + "cancelling": bool(job and job.cancelled), + # A backup or restore always registers itself, so a lock of theirs + # with no job behind it is a leftover: the run is over and releasing + # the row stops nothing. + "stale": _is_orphaned(lock["action"], job), + "phase": job.phase if job else "", + } + + +def _is_orphaned(action: str, job) -> bool: + return job is None and action in stack_job_service.TRACKED + + def _stack_summary( stack: Stack, summaries: dict | None = None, busy: dict[str, str] | None = None ) -> dict: @@ -229,7 +262,8 @@ def get_stack( containers = [] status = "unknown" # An operation in flight outranks whatever the containers currently say. - if stack_lock_service.is_busy(session, stack_id): + lock = stack_lock_service.info(session, stack_id) + if lock: status = "updating" return { "id": stack.id, @@ -238,6 +272,9 @@ def get_stack( "icon": stack.icon, "auto_icon": _auto_icon(stack), "status": status, + # What is holding the stack, so the page can offer to stop it instead + # of only showing a spinner that never ends. + "lock": _lock_view(lock, stack_id), "yaml": compose_service.read_compose(stack_id), # The .env is where credentials live by convention, so it is withheld # from the read-only role — same reasoning as the admin-only file @@ -531,6 +568,113 @@ async def down_stack(stack_id: str, request: Request, session: Session = Depends return await _lifecycle(compose_service.down, "down", stack_id, request, session, user) +# --------------------------------------------------------------------------- # +# The lock: what holds a stack, and getting out from under it +# --------------------------------------------------------------------------- # + +#: How long ``unlock`` waits for a cancelled job to unwind. Cancelling removes +#: the helper container the backup streams through, so the read fails at once; +#: what takes the time afterwards is compose starting the containers the +#: backup stopped, which is what we want to have happened before we answer. +CANCEL_WAIT_SECONDS = 20.0 +_CANCEL_POLL = 0.25 + + +@router.get("/{stack_id}/lock") +def stack_lock( + stack_id: str, + session: Session = Depends(get_session), + _user: User = Depends(get_current_user), +) -> dict: + """What is holding this stack, if anything. + + The stacks list only says "updating"; this says *what*, since when, and + whether it can be stopped. + """ + _get_stack_or_404(session, stack_id) + return {"locked": stack_lock_service.is_busy(session, stack_id), + "lock": _lock_view(stack_lock_service.info(session, stack_id), stack_id)} + + +@router.post("/{stack_id}/unlock") +async def unlock_stack( + stack_id: str, + request: Request, + force: bool = Query( + False, + description="Drop the lock even when the operation could not be stopped.", + ), + session: Session = Depends(get_session), + user: User = Depends(require_admin), +) -> dict: + """Stop whatever holds the stack, so it can be used again. + + A backup is asked to stop and given a moment to unwind — it throws its + half-written archive away, starts the containers it stopped for a + consistent snapshot, and drops the lock itself. That is the clean path and + the one to try first. + + ``force`` is the other one: it deletes the lock row without stopping + anything. For a lock left behind by an operation that is no longer running + that is all that is needed; while something *is* still running it means + the next deploy and that operation can collide, so it stays an explicit + second step. + """ + _get_stack_or_404(session, stack_id) + lock = stack_lock_service.info(session, stack_id) + job = stack_job_service.get(stack_id) + if not lock and not job: + raise HTTPException(status_code=409, detail=f"Stack '{stack_id}' is not locked") + action = lock["action"] if lock else job.action + + cancelled = False + if job is not None and job.cancellable: + stack_job_service.cancel(stack_id, user.username) + cancelled = True + deadline = asyncio.get_running_loop().time() + CANCEL_WAIT_SECONDS + while asyncio.get_running_loop().time() < deadline: + await asyncio.sleep(_CANCEL_POLL) + # The job releases the lock from its own session; ours would + # otherwise answer from its identity map. + session.expire_all() + if not stack_lock_service.is_busy(session, stack_id): + break + + session.expire_all() + released = not stack_lock_service.is_busy(session, stack_id) + forced = False + if not released: + # A lock left behind by a backup that is no longer running guards + # nothing, so it goes without being asked twice. + orphaned = _is_orphaned(action, stack_job_service.get(stack_id)) + if not force and not orphaned: + raise HTTPException( + status_code=409, + detail=( + f"The {action} of '{stack_id}' did not stop in time" + if cancelled + else f"The {action} of '{stack_id}' cannot be stopped from here" + ) + + " — release the lock with force to use the stack anyway.", + ) + stack_lock_service.release(session, stack_id) + released, forced = True, not orphaned + + audit_service.record( + session, user=user.username, action="stack.unlock", target=stack_id, + detail=f"{action}{' cancelled' if cancelled else ''}{' forced' if forced else ''}", + ip=_client_ip(request), + ) + logger.info( + "Lock on '%s' (%s) released by %s%s%s", + stack_id, action, user.username, + " after cancelling the job" if cancelled else "", + " (forced)" if forced else "", + ) + return {"ok": True, "action": action, "cancelled": cancelled, + "released": released, "forced": forced} + + # --------------------------------------------------------------------------- # # logs / export / convert # --------------------------------------------------------------------------- # diff --git a/backend/services/backup_service.py b/backend/services/backup_service.py index b17670f..85ae3f0 100644 --- a/backend/services/backup_service.py +++ b/backend/services/backup_service.py @@ -52,7 +52,12 @@ from sqlmodel import Session from database import engine from docker_client import DockerError, get_long_client, safe_call -from services import compose_service, stack_lock_service, stack_assets_service as assets +from services import ( + compose_service, + stack_job_service, + stack_lock_service, + stack_assets_service as assets, +) logger = logging.getLogger("stackpilot.backup") @@ -76,6 +81,15 @@ class StackBusyError(BackupError): """The stack is locked by another operation (deploy, update, backup).""" +class BackupCancelled(BackupError): + """Somebody stopped this backup from the outside. + + Not a failure: the archive is thrown away, the stack is started again and + the lock goes, exactly as after an error — but nothing is reported as + broken and no "backup failed" notification goes out. + """ + + # --------------------------------------------------------------------------- # # Backup filename convention # --------------------------------------------------------------------------- # @@ -112,14 +126,23 @@ def backup_name_pattern(stack_id: str, prefix: Optional[str] = None) -> re.Patte @contextmanager -def _locked(stack_id: str, action: str): +def _locked(stack_id: str, action: str, owner: str = ""): + """Hold the stack lock, and register the job so it can be stopped. + + The lock alone made a stuck backup fatal: nothing could touch the stack + until its twelve-hour TTL ran out. The registration is what gives the + unlock endpoint something to cancel. + """ with Session(engine) as session: try: - stack_lock_service.acquire(session, stack_id, action, action, ttl=LOCK_TTL) + stack_lock_service.acquire(session, stack_id, action, owner or action, ttl=LOCK_TTL) except stack_lock_service.StackBusy as exc: raise StackBusyError(str(exc)) from exc try: - yield + with stack_job_service.track(stack_id, action, owner or action): + yield + except stack_job_service.JobCancelled as exc: + raise BackupCancelled(str(exc)) from exc finally: try: stack_lock_service.release(session, stack_id) @@ -275,6 +298,10 @@ def _add_tree( """ def add(path: str, rel: str, parent_dev: int) -> None: + # A stack directory with a data folder in it can take as long as a + # volume, so a cancellation lands here too — between two files, with + # the half-written archive thrown away by the caller. + stack_job_service.check() arc = f"{arcroot}/{rel}" if rel else arcroot label = rel or "." try: @@ -387,6 +414,7 @@ async def _create( binds: Optional[list[str]], volumes: Optional[list[str]], ) -> tuple[str, dict]: + stack_job_service.phase("looking at what to capture") selection = await asyncio.to_thread( plan, stack_id, include_volumes, include_binds, binds, volumes ) @@ -429,6 +457,11 @@ async def _create( try: parts: list[tuple[str, str]] = [] for index, bind in enumerate(cap_binds): + # Between two items is where a cancelled backup gives up: never in + # the middle of one, and never after the stack has been stopped + # without the finally below starting it again. + stack_job_service.check() + stack_job_service.phase(f"bind {bind['source']}") part = os.path.join(workdir, f"bind-{index:03d}.tar") try: bind["bytes"] = await asyncio.to_thread( @@ -437,16 +470,22 @@ async def _create( bind["archive"] = f"binds/{index:03d}.tar" parts.append((part, bind["archive"])) except Exception as exc: # noqa: BLE001 + # Cancelling pulls the helper container out from under the + # read, so the failure here *is* the cancellation. + stack_job_service.check() logger.warning("Could not archive bind %s: %s", bind["source"], exc) bind["archive"], bind["error"] = None, str(exc) for index, vol in enumerate(cap_volumes): + stack_job_service.check() + stack_job_service.phase(f"volume {vol['name']}") part = os.path.join(workdir, f"vol-{index:03d}.tar") try: vol["bytes"] = await asyncio.to_thread(assets.export_volume, vol["name"], part) vol["archive"] = f"volumes/{vol['name']}.tar" parts.append((part, vol["archive"])) except Exception as exc: # noqa: BLE001 + stack_job_service.check() logger.warning("Could not archive volume %s: %s", vol["name"], exc) vol["archive"], vol["error"] = None, str(exc) @@ -514,6 +553,8 @@ async def _create( ], } + stack_job_service.check() + stack_job_service.phase("writing the archive") fd, archive_path = tempfile.mkstemp(prefix=f"sp-{stack_id}-", suffix=".tar.gz") os.close(fd) await asyncio.to_thread( diff --git a/backend/services/schedule_service.py b/backend/services/schedule_service.py index 363ec0d..a5f4fe4 100644 --- a/backend/services/schedule_service.py +++ b/backend/services/schedule_service.py @@ -170,6 +170,11 @@ async def _backup_stack( return "ok" except backup_service.StackBusyError as exc: raise _Busy(str(exc)) from exc + except backup_service.BackupCancelled as exc: + # Somebody stopped this backup from the UI. Not a failure of the + # schedule: say so, and don't page anyone with "backup failed". + logger.info("Scheduled backup of %s was cancelled: %s", stack_id, exc) + return f"cancelled: {exc}" except Exception as exc: # noqa: BLE001 logger.warning("Scheduled backup of %s failed: %s", stack_id, exc) return f"error: {exc}" @@ -185,7 +190,11 @@ def _summarise(statuses: dict[str, str]) -> str: bad = {sid: st for sid, st in statuses.items() if st != "ok"} if not bad: return "ok" - worst = "error" if any(not st.startswith("warning") for st in bad.values()) else "warning" + worst = ( + "error" + if any(not st.startswith(("warning", "cancelled")) for st in bad.values()) + else "warning" + ) if len(statuses) == 1: return next(iter(statuses.values())) detail = "; ".join(f"{sid}: {st}" for sid, st in sorted(bad.items())) diff --git a/backend/services/stack_assets_service.py b/backend/services/stack_assets_service.py index 8d7d5b6..fb0c400 100644 --- a/backend/services/stack_assets_service.py +++ b/backend/services/stack_assets_service.py @@ -32,7 +32,7 @@ from docker_client import ( run_container, safe_call, ) -from services import compose_service +from services import compose_service, stack_job_service logger = logging.getLogger("stackpilot.assets") @@ -99,16 +99,22 @@ def ensure_helper_image(client) -> None: def _create_helper(client, volumes: dict): - return safe_call( + container = safe_call( client.containers.create, settings.BACKUP_HELPER_IMAGE, command="true", volumes=volumes, labels={HELPER_LABEL: "1"}, ) + # Streaming a large volume through this container is the part of a backup + # that runs for hours; cancelling one works by removing it (see + # :mod:`services.stack_job_service`), so the job has to know it exists. + stack_job_service.register_helper(container.id) + return container def _remove(container) -> None: + stack_job_service.drop_helper(getattr(container, "id", "")) try: container.remove(force=True) except Exception: # noqa: BLE001 - cleanup is best effort @@ -160,13 +166,15 @@ def _put_archive(container, dest: str, src_file: str, volumes: dict) -> None: def _create_helper_with(client, volumes: dict, command: list[str]): ensure_helper_image(client) - return safe_call( + container = safe_call( client.containers.create, settings.BACKUP_HELPER_IMAGE, command=command, volumes=volumes, labels={HELPER_LABEL: "1"}, ) + stack_job_service.register_helper(container.id) + return container def _unknown() -> dict: diff --git a/backend/services/stack_job_service.py b/backend/services/stack_job_service.py new file mode 100644 index 0000000..2ec5fc7 --- /dev/null +++ b/backend/services/stack_job_service.py @@ -0,0 +1,231 @@ +"""The long-running backup/restore behind a stack lock, and stopping it. + +``stack_lock_service`` answers one question — is this stack busy — and +deliberately knows nothing about who is doing the work. That was enough until +a backup got stuck: the lock a backup takes lives for twelve hours (a large +volume legitimately takes that long), and for as long as it is held the stack +cannot be started, stopped, restarted or deployed. So a backup that hangs — an +unreachable NFS destination, a helper container reading a volume that never +ends — or one whose work died without unwinding takes the whole stack with it +until the TTL runs out. + +This module is the other half of the lock: the operation registers itself +here, and a second request can ask it to stop. + +Cancellation is cooperative, and on purpose: + +* ``cancel`` sets a flag and force-removes the helper containers the job is + streaming through, which is what actually ends the long wait — the stream + breaks, the worker thread returns, and the backup unwinds through its own + ``finally``. That is what restarts the containers the backup stopped and + releases the lock. Killing the task instead would leave both undone. +* Nothing is cancelled from the outside. A restore is not interruptible at all + (it is halfway through writing a stack's data back), so it registers itself + to be *seen*, not stopped. + +When even that does not help — a job whose process is long gone, leaving only +the lock row behind — the escape hatch is releasing the lock itself +(:func:`stack_lock_service.release`), which the unlock endpoint does with +``force``. +""" +from __future__ import annotations + +import logging +import threading +from contextlib import contextmanager +from contextvars import ContextVar +from dataclasses import dataclass, field +from datetime import datetime, timezone +from typing import Optional + +from docker_client import get_long_client, safe_call + +logger = logging.getLogger("stackpilot.stack_job") + +#: Actions that can be stopped halfway without leaving a mess behind. A backup +#: only reads; a restore is mid-way through writing a stack's data back and +#: must be allowed to finish. +CANCELLABLE = {"backup"} + +#: Actions that always register here while they run. A lock held by one of +#: these with no job behind it is therefore a leftover — the run is over, only +#: its row survived — and releasing it stops nothing. Compose operations +#: (start, update, git-deploy) are not in this set: they run as a subprocess +#: that outlives the request, so a lock of theirs may well still mean work. +TRACKED = {"backup", "restore"} + + +class JobCancelled(Exception): + """Raised inside a job that has been asked to stop.""" + + def __init__(self, stack_id: str, action: str, by: str = ""): + self.stack_id = stack_id + self.action = action + self.by = by + who = f" by {by}" if by else "" + super().__init__(f"{action} of '{stack_id}' was cancelled{who}") + + +def _now() -> datetime: + return datetime.now(timezone.utc) + + +@dataclass +class Job: + """One backup or restore, while it runs.""" + + stack_id: str + action: str + owner: str = "" + started_at: datetime = field(default_factory=_now) + #: What it is doing right now ("volume media-data", "writing the archive"), + #: so a stack that looks stuck can be told from one that is just slow. + phase: str = "" + cancelled_by: Optional[str] = None + _stop: threading.Event = field(default_factory=threading.Event) + #: Helper containers this job streams through. Removing them is what ends + #: a read that would otherwise run for hours. + _helpers: set[str] = field(default_factory=set) + + @property + def cancellable(self) -> bool: + return self.action in CANCELLABLE + + @property + def cancelled(self) -> bool: + return self._stop.is_set() + + def to_dict(self) -> dict: + return { + "stack_id": self.stack_id, + "action": self.action, + "owner": self.owner, + "started_at": self.started_at.isoformat(), + "phase": self.phase, + "cancellable": self.cancellable, + "cancelled": self.cancelled, + "cancelled_by": self.cancelled_by, + } + + +_jobs: dict[str, Job] = {} +_guard = threading.Lock() +#: The job of the current task. ``asyncio.to_thread`` copies the context, so +#: the worker threads a backup fans out into see their own job too. +_current: ContextVar[Optional[Job]] = ContextVar("stackpilot_job", default=None) + + +@contextmanager +def track(stack_id: str, action: str, owner: str = ""): + """Register the operation for the duration of the block. + + Always deregisters: a job that is gone from here is one nobody can be + waiting on, so a leftover entry would make a free stack look busy. + """ + job = Job(stack_id=stack_id, action=action, owner=owner) + with _guard: + _jobs[stack_id] = job + token = _current.set(job) + try: + yield job + finally: + _current.reset(token) + with _guard: + if _jobs.get(stack_id) is job: + del _jobs[stack_id] + + +def current() -> Optional[Job]: + return _current.get() + + +def get(stack_id: str) -> Optional[Job]: + with _guard: + return _jobs.get(stack_id) + + +def active() -> list[dict]: + with _guard: + return [job.to_dict() for job in _jobs.values()] + + +def phase(text: str) -> None: + """Record what the current job is doing. No-op outside a job.""" + job = _current.get() + if job is not None: + job.phase = text + + +def cancelled() -> bool: + job = _current.get() + return job is not None and job.cancelled + + +def check() -> None: + """Raise :class:`JobCancelled` if the current job was asked to stop. + + Called at every point a backup can safely give up — between the files and + volumes it archives, not in the middle of one. + """ + job = _current.get() + if job is not None and job.cancelled: + raise JobCancelled(job.stack_id, job.action, job.cancelled_by or "") + + +# --------------------------------------------------------------------------- # +# Helper containers +# --------------------------------------------------------------------------- # + + +def register_helper(container_id: str) -> None: + """Note a helper container the current job reads or writes through.""" + job = _current.get() + if job is not None: + with _guard: + job._helpers.add(container_id) + + +def drop_helper(container_id: str) -> None: + job = _current.get() + if job is not None: + with _guard: + job._helpers.discard(container_id) + + +def _kill_helpers(job: Job) -> int: + with _guard: + ids = list(job._helpers) + if not ids: + return 0 + killed = 0 + client = get_long_client() + for container_id in ids: + try: + container = safe_call(client.containers.get, container_id) + safe_call(container.remove, force=True) + killed += 1 + except Exception as exc: # noqa: BLE001 - already gone is a fine outcome + logger.debug("Helper %s of '%s' could not be removed: %s", container_id[:12], job.stack_id, exc) + return killed + + +def cancel(stack_id: str, by: str = "") -> Optional[Job]: + """Ask the job on ``stack_id`` to stop. Returns it, or None if there is none. + + Returns as soon as the job has been told; the job itself still has to + unwind — restart what it stopped, drop the lock — which is what the caller + waits for afterwards. + """ + job = get(stack_id) + if job is None: + return None + if not job.cancellable: + return job + job.cancelled_by = by or "unknown" + job._stop.set() + killed = _kill_helpers(job) + logger.info( + "Cancelling the %s of '%s' (requested by %s, %d helper container(s) removed)", + job.action, stack_id, job.cancelled_by, killed, + ) + return job diff --git a/backend/services/stack_lock_service.py b/backend/services/stack_lock_service.py index acdf226..66cbf74 100644 --- a/backend/services/stack_lock_service.py +++ b/backend/services/stack_lock_service.py @@ -132,6 +132,45 @@ def active(session: Session) -> dict[str, str]: } +def info(session: Session, stack_id: str) -> Optional[dict]: + """The lock in force on ``stack_id``, as plain data — or None. + + What the UI needs to explain a stack that will not budge: which operation + holds it, since when, and when it would give up on its own. + """ + lock = session.get(StackLock, stack_id) + if lock is None: + return None + now = _now() + expires = _aware(lock.expires_at) or now + if expires <= now: + return None + acquired = _aware(lock.acquired_at) or now + return { + "action": lock.action, + "owner": lock.owner, + "acquired_at": acquired.isoformat(), + "expires_at": expires.isoformat(), + "held_for": int((now - acquired).total_seconds()), + } + + +def clear_all(session: Session) -> list[str]: + """Drop every lock. Returns the stack ids that were locked. + + Called at startup: a lock only means something while the operation holding + it runs, and nothing of ours is in flight yet (StackPilot runs a single + uvicorn worker — see the Dockerfile's CMD). Before this, a backup whose + process died left its stack unusable for the twelve hours of its TTL. + """ + locks = session.exec(select(StackLock)).all() + ids = [lock.stack_id for lock in locks] + for lock in locks: + session.delete(lock) + session.commit() + return ids + + def is_busy(session: Session, stack_id: str) -> bool: lock = session.get(StackLock, stack_id) return lock is not None and (_aware(lock.expires_at) or _now()) > _now() diff --git a/backend/tests/test_backup_restore.py b/backend/tests/test_backup_restore.py index a86d8bd..f2019ee 100644 --- a/backend/tests/test_backup_restore.py +++ b/backend/tests/test_backup_restore.py @@ -277,6 +277,103 @@ def test_failed_volume_is_an_error_and_listed_as_skipped(stack, docker): for s in manifest["skipped"]) +# --------------------------------------------------------------------------- # +# Cancelling a backup +# --------------------------------------------------------------------------- # +# +# A backup holds the stack lock for up to twelve hours and stops the stack's +# containers for a consistent snapshot. One that hangs — an unreachable share, +# a helper container reading a volume that never ends — therefore used to +# leave the stack stopped *and* refusing every start, stop and restart with +# "busy: backup in progress" until the TTL ran out. It can be stopped now, and +# stopping it has to put back exactly what the backup took away. + + +def _cancelling_export(stack_id, docker): + """An export that is cancelled halfway, the way the endpoint does it: + the helper container is removed, so the read fails.""" + + def export_volume(name, dest_file): + from services import stack_job_service + + stack_job_service.cancel(stack_id, "tester") + raise RuntimeError("helper container removed while reading the volume") + + return export_volume + + +def test_cancelling_a_backup_starts_the_stack_again(stack, docker, monkeypatch): + stack_id, _, inv = stack + inv["volumes"].append(_volume(f"{stack_id}_db", stack_id)) + docker.container("app", "running") + monkeypatch.setattr(assets, "export_volume", _cancelling_export(stack_id, docker)) + + with pytest.raises(backup_service.BackupCancelled): + run(backup_service.create_backup_ex(stack_id, "S")) + + assert docker.compose_calls == [["stop", "app"], ["start", "app"]] + assert [c.state for c in docker.containers] == ["running"] + + +def test_a_cancelled_backup_frees_the_lock(stack, docker, monkeypatch): + """The reason to cancel at all: the stack must be usable again.""" + from sqlmodel import Session + + from database import engine + from services import stack_lock_service + + stack_id, _, inv = stack + inv["volumes"].append(_volume(f"{stack_id}_db", stack_id)) + docker.container("app", "running") + monkeypatch.setattr(assets, "export_volume", _cancelling_export(stack_id, docker)) + + with pytest.raises(backup_service.BackupCancelled): + run(backup_service.create_backup_ex(stack_id, "S")) + + with Session(engine) as session: + assert not stack_lock_service.is_busy(session, stack_id) + + +def test_a_cancelled_backup_leaves_no_half_written_archive(stack, docker, monkeypatch): + import glob + import tempfile as tempfile_mod + + stack_id, _, inv = stack + inv["volumes"].append(_volume(f"{stack_id}_db", stack_id)) + docker.container("app", "running") + + def cancel_while_writing(name, dest_file): + from services import stack_job_service + + with tarfile.open(dest_file, "w"): + pass + stack_job_service.cancel(stack_id, "tester") + return os.path.getsize(dest_file) + + monkeypatch.setattr(assets, "export_volume", cancel_while_writing) + + with pytest.raises(backup_service.BackupCancelled): + run(backup_service.create_backup_ex(stack_id, "S")) + + leftovers = glob.glob(os.path.join(tempfile_mod.gettempdir(), f"sp-{stack_id}-*.tar.gz")) + assert leftovers == [] + + +def test_a_backup_nobody_cancelled_is_not_disturbed(stack, docker): + """The check runs on every file and volume; it must only fire when asked.""" + stack_id, _, inv = stack + inv["volumes"].append(_volume(f"{stack_id}_db", stack_id)) + docker.volumes[f"{stack_id}_db"] = {"files": {"a": b"1"}, "labels": {}} + docker.container("app", "running") + + path, report = run(backup_service.create_backup_ex(stack_id, "S")) + try: + assert report["errors"] == [] + assert {v["name"] for v in report["volumes"]} == {f"{stack_id}_db"} + finally: + os.unlink(path) + + def test_file_that_shrinks_while_read_keeps_the_archive_valid(tmp_path, monkeypatch): root = tmp_path / "stack" root.mkdir() diff --git a/backend/tests/test_route_authorization.py b/backend/tests/test_route_authorization.py index e9f2c1b..6ae7310 100644 --- a/backend/tests/test_route_authorization.py +++ b/backend/tests/test_route_authorization.py @@ -89,6 +89,10 @@ USER_READABLE = { # The stack's uploaded icon — an image the user already sees next to the # stack's name in the list. Uploading and resetting it stay admin-only. "GET /api/stacks/{stack_id}/icon", + # Which operation holds the stack and since when — the same "updating" + # the list already shows, with the detail behind it. Stopping it + # (POST .../unlock) is admin-only. + "GET /api/stacks/{stack_id}/lock", "GET /api/stacks/{stack_id}/logs", "GET /api/stacks/{stack_id}/services/{service}/logs", "POST /api/stacks/convert", diff --git a/backend/tests/test_stack_locking.py b/backend/tests/test_stack_locking.py index 35a1456..a1e9181 100644 --- a/backend/tests/test_stack_locking.py +++ b/backend/tests/test_stack_locking.py @@ -204,3 +204,192 @@ def test_an_unlocked_stack_is_not_refused(as_admin, db): session.commit() assert as_admin.post(f"/api/stacks/{stack_id}/start").status_code != 409 + + +# --------------------------------------------------------------------------- # +# Getting out from under a lock +# --------------------------------------------------------------------------- # +# +# The lock made a stuck backup fatal: it is held for twelve hours (a large +# volume legitimately takes that long), and while it is held the stack can be +# neither started, stopped nor restarted — "Stack 'x' is busy: backup in +# progress", with nothing to click. These are the three ways out: the job is +# told to stop, the lock is dropped by hand, or a restart clears what is left +# of a run that is over. + + +def test_clear_all_drops_locks_that_outlived_their_run(svc, lock_session): + """What startup does. + + A lock means "an operation is running". After a restart none is, so an + unexpired one is a leftover — and before this it kept its stack unusable + for the rest of a backup's twelve-hour TTL. + """ + svc.acquire(lock_session, "alpha", "backup") + svc.acquire(lock_session, "beta", "update") + + assert sorted(svc.clear_all(lock_session)) == ["alpha", "beta"] + assert svc.active(lock_session) == {} + + +def test_info_says_what_holds_the_stack(svc, lock_session): + svc.acquire(lock_session, "demo", "backup", "scheduled") + + info = svc.info(lock_session, "demo") + assert info["action"] == "backup" + assert info["owner"] == "scheduled" + assert info["held_for"] >= 0 + assert svc.info(lock_session, "not-locked-at-all") is None + + +def test_info_ignores_an_expired_lock(svc, lock_session): + from models.runtime_state import StackLock + + lock_session.add( + StackLock( + stack_id="dead", + action="backup", + expires_at=datetime.now(timezone.utc) - timedelta(seconds=1), + ) + ) + lock_session.commit() + assert svc.info(lock_session, "dead") is None + + +def test_the_lock_endpoint_says_what_holds_the_stack(as_admin, locked_stack): + body = as_admin.get(f"/api/stacks/{locked_stack}/lock").json() + assert body["locked"] is True + assert body["lock"]["action"] == "update" + assert body["lock"]["cancellable"] is False + # A deploy runs as a subprocess that outlives its request: no job here + # does not mean nothing is running. + assert body["lock"]["stale"] is False + + +def test_a_backup_lock_with_no_backup_behind_it_is_a_leftover(as_admin, locked_stack): + """This is the state a crashed backup leaves: the twelve-hour lock stays, + the backup is long gone, and the stack refuses everything. One click has + to be enough — there is nothing left to stop.""" + from database import engine + from services import stack_lock_service + + with Session(engine) as session: + stack_lock_service.release(session, locked_stack) + stack_lock_service.acquire(session, locked_stack, "backup", "backup") + + assert as_admin.get(f"/api/stacks/{locked_stack}/lock").json()["lock"]["stale"] is True + + body = as_admin.post(f"/api/stacks/{locked_stack}/unlock").json() + assert body == {"ok": True, "action": "backup", "cancelled": False, + "released": True, "forced": False} + assert as_admin.post(f"/api/stacks/{locked_stack}/restart").status_code != 409 + + +def test_unlocking_a_stack_that_nothing_holds_is_refused(as_admin, db): + from models.stack import Stack + from database import engine + from services import compose_service + + stack_id = "not-locked-fixture" + compose_service.write_compose(stack_id, "services:\n a:\n image: alpine\n") + with Session(engine) as session: + if not session.get(Stack, stack_id): + session.add(Stack(id=stack_id, name="not locked")) + session.commit() + + response = as_admin.post(f"/api/stacks/{stack_id}/unlock") + assert response.status_code == 409 + assert "not locked" in response.json()["detail"] + + +def test_a_lock_nobody_can_stop_needs_force(as_admin, locked_stack): + """The deploy behind this lock is not something we can interrupt, so the + plain call refuses and says what the second step is — rather than claiming + to have stopped something.""" + response = as_admin.post(f"/api/stacks/{locked_stack}/unlock") + assert response.status_code == 409 + assert "force" in response.json()["detail"] + + forced = as_admin.post(f"/api/stacks/{locked_stack}/unlock?force=true") + assert forced.status_code == 200 + assert forced.json() == { + "ok": True, "action": "update", "cancelled": False, + "released": True, "forced": True, + } + assert as_admin.get(f"/api/stacks/{locked_stack}/lock").json()["locked"] is False + + +def test_the_stack_can_be_used_again_after_the_lock_is_released(as_admin, locked_stack): + """The whole point: the actions that were refused go through afterwards.""" + assert as_admin.post(f"/api/stacks/{locked_stack}/restart").status_code == 409 + as_admin.post(f"/api/stacks/{locked_stack}/unlock?force=true") + assert as_admin.post(f"/api/stacks/{locked_stack}/restart").status_code != 409 + + +def test_releasing_a_lock_is_written_to_the_audit_log(as_admin, locked_stack): + as_admin.post(f"/api/stacks/{locked_stack}/unlock?force=true") + rows = as_admin.get(f"/api/audit?stack_id={locked_stack}&limit=20").json() + assert any(r["action"] == "stack.unlock" and r["target"] == locked_stack for r in rows) + + +# --------------------------------------------------------------------------- # +# The job behind the lock +# --------------------------------------------------------------------------- # + + +@pytest.fixture +def jobs(): + from services import stack_job_service + + return stack_job_service + + +def test_a_backup_can_be_cancelled_and_a_restore_cannot(jobs): + """A backup only reads, so dropping it costs nothing. A restore is halfway + through writing a stack's data back; interrupting it would leave the stack + in a state no one asked for, so it registers itself to be seen, not + stopped.""" + with jobs.track("demo", "backup"): + assert jobs.cancel("demo", "alice").cancelled is True + + with jobs.track("demo", "restore"): + assert jobs.cancel("demo", "alice").cancelled is False + + +def test_cancelling_a_stack_nobody_is_working_on_says_so(jobs): + assert jobs.cancel("nothing-running") is None + + +def test_the_job_is_gone_once_its_operation_ends(jobs): + """A leftover entry would make a free stack look like it is still busy.""" + with pytest.raises(RuntimeError): + with jobs.track("demo", "backup"): + raise RuntimeError("backup blew up") + assert jobs.get("demo") is None + + +def test_check_only_raises_inside_a_cancelled_job(jobs): + jobs.check() # no job at all: the backup code calls this unconditionally + with jobs.track("demo", "backup"): + jobs.check() + jobs.cancel("demo", "alice") + with pytest.raises(jobs.JobCancelled): + jobs.check() + + +def test_a_cancelled_job_that_will_not_stop_can_still_be_forced( + as_admin, locked_stack, monkeypatch, jobs +): + """The order the UI offers: ask the backup to stop, and only when it does + not, drop the lock without it.""" + import routers.stacks as stacks_router + + monkeypatch.setattr(stacks_router, "CANCEL_WAIT_SECONDS", 0.5) + with jobs.track(locked_stack, "backup", "scheduled") as job: + response = as_admin.post(f"/api/stacks/{locked_stack}/unlock") + assert response.status_code == 409 + assert "did not stop in time" in response.json()["detail"] + assert job.cancelled is True, "it was still asked to stop" + + forced = as_admin.post(f"/api/stacks/{locked_stack}/unlock?force=true").json() + assert forced["cancelled"] is True and forced["forced"] is True diff --git a/backend/version.py b/backend/version.py index 6b057c8..1f3b5c5 100644 --- a/backend/version.py +++ b/backend/version.py @@ -1,3 +1,3 @@ """Single source of truth for the StackPilot release version.""" -APP_VERSION = "0.66.3" +APP_VERSION = "0.67.0" diff --git a/frontend/package.json b/frontend/package.json index 58bdbfe..deca6d3 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "stackpilot-frontend", "private": true, - "version": "0.66.3", + "version": "0.67.0", "type": "module", "scripts": { "dev": "vite", diff --git a/frontend/src/api/stacks.ts b/frontend/src/api/stacks.ts index 67f3287..1b53599 100644 --- a/frontend/src/api/stacks.ts +++ b/frontend/src/api/stacks.ts @@ -1,5 +1,12 @@ import api from "./client"; -import type { StackDetail, StackStats, StackSummary, StackUpdateInfo } from "@/types"; +import type { + StackDetail, + StackLock, + StackStats, + StackSummary, + StackUnlockResult, + StackUpdateInfo, +} from "@/types"; export const stacksApi = { list: () => api.get("/api/stacks").then((r) => r.data), @@ -31,6 +38,20 @@ export const stacksApi = { clone: (id: string, name: string) => api.post(`/api/stacks/${id}/clone`, { name }).then((r) => r.data), + /** What holds the stack right now, if anything. */ + lock: (id: string) => + api + .get<{ locked: boolean; lock: StackLock | null }>(`/api/stacks/${id}/lock`) + .then((r) => r.data), + /** Stop whatever holds the stack. A running backup is asked to stop and + * given a moment to put the stack back the way it found it; `force` drops + * the lock without stopping anything, which is the last resort for one + * left behind by an operation that is no longer running. */ + unlock: (id: string, force = false) => + api + .post(`/api/stacks/${id}/unlock?force=${force}`) + .then((r) => r.data), + start: (id: string) => api.post(`/api/stacks/${id}/start`).then((r) => r.data), stop: (id: string) => api.post(`/api/stacks/${id}/stop`).then((r) => r.data), restart: (id: string) => api.post(`/api/stacks/${id}/restart`).then((r) => r.data), diff --git a/frontend/src/components/stacks/StackLockBanner.tsx b/frontend/src/components/stacks/StackLockBanner.tsx new file mode 100644 index 0000000..da2d89a --- /dev/null +++ b/frontend/src/components/stacks/StackLockBanner.tsx @@ -0,0 +1,139 @@ +import { useState } from "react"; +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { Loader2, Lock, Unlock } from "lucide-react"; +import { toast } from "sonner"; +import { Button } from "@/components/ui"; +import { ConfirmDialog } from "@/components/ui/ConfirmDialog"; +import { stacksApi } from "@/api/stacks"; +import { apiErrorMessage } from "@/api/client"; +import type { StackLock } from "@/types"; + +/** "14 minutes", "3 hours" — how long this has been going on. */ +function since(seconds: number): string { + if (seconds < 90) return `${Math.max(seconds, 0)} seconds`; + const minutes = Math.round(seconds / 60); + if (minutes < 90) return `${minutes} minutes`; + return `${Math.round(minutes / 60)} hours`; +} + +const LABEL: Record = { + backup: "A backup", + restore: "A restore", + update: "An update", + start: "A deploy", + "git-deploy": "A Git deploy", + "auto-update": "An automatic update", +}; + +function describe(lock: StackLock): string { + return LABEL[lock.action] ?? `A ${lock.action}`; +} + +/** + * Shown while an operation holds the stack — and, more to the point, offers a + * way out of it. + * + * One operation runs per stack at a time, and a backup may hold that lock for + * twelve hours (a large volume legitimately takes that long). So a backup that + * dies without unwinding leaves every start, stop and restart answering + * "Stack 'x' is busy: backup in progress" with nothing to click. This is the + * button that was missing: stop the backup (it throws its half-written archive + * away and starts the containers it stopped), or — when there is nothing left + * running to stop — drop the lock. + */ +export function StackLockBanner({ + stackId, + lock, +}: { + stackId: string; + lock: StackLock; +}) { + const qc = useQueryClient(); + const [confirmForce, setConfirmForce] = useState(false); + + const unlock = useMutation({ + mutationFn: (force: boolean) => stacksApi.unlock(stackId, force), + onSuccess: (result) => { + toast.success( + result.cancelled + ? `${describe(lock)} of ${stackId} was stopped` + : `The lock on ${stackId} was released` + ); + setConfirmForce(false); + qc.invalidateQueries({ queryKey: ["stack", stackId] }); + qc.invalidateQueries({ queryKey: ["stacks"] }); + }, + onError: (err) => { + toast.error(apiErrorMessage(err)); + // It would not stop: offer the blunt instrument rather than leaving the + // user with an error and no next step. + setConfirmForce(true); + }, + }); + + const running = unlock.isPending; + const canStop = lock.cancellable && !lock.cancelling; + + return ( +
+
+ {lock.stale ? ( + + ) : ( + + )} + + {lock.stale ? ( + <> + {describe(lock)} left this stack locked, but it is no longer + running. Until the lock goes, start, stop and restart are refused. + + ) : ( + <> + {describe(lock)} is running — started {since(lock.held_for)} ago + {lock.phase ? ` (${lock.phase})` : ""}. The stack cannot be + started, stopped or deployed while it does. + {lock.cancelling && " Stopping it…"} + + )} + + {canStop && ( + + )} + +
+ + {confirmForce && ( + unlock.mutate(true)} + onCancel={() => setConfirmForce(false)} + /> + )} +
+ ); +} diff --git a/frontend/src/hooks/useStackActions.ts b/frontend/src/hooks/useStackActions.ts index 7e9fbf3..f29cd18 100644 --- a/frontend/src/hooks/useStackActions.ts +++ b/frontend/src/hooks/useStackActions.ts @@ -23,6 +23,17 @@ export type StackActionStatus = { * or until the same stack is acted on again. */ const SUCCESS_CLEAR_MS = 5000; +/** + * "Stack 'x' is busy: backup in progress" is a dead end on its own — the user + * gets it on every button and nothing here says that the operation holding the + * stack can be stopped. The stack's own page has that button, so point at it. + */ +function withLockHint(message: string): string { + return /\bis busy\b/i.test(message) + ? `${message}. Open the stack to stop it or release the lock.` + : message; +} + /** * Runs stack lifecycle actions and tracks their progress. * @@ -84,7 +95,7 @@ export function useStackActions() { toast.success(`${label} ${id} ✓`, { id: t }); succeed(id, label); } catch (err) { - const message = apiErrorMessage(err); + const message = withLockHint(apiErrorMessage(err)); toast.error(message, { id: t }); fail(id, label, message); } finally { @@ -102,7 +113,7 @@ export function useStackActions() { toast.success(`${label} ${id} ✓`, { id: t }); succeed(id, label); } catch (err) { - const message = err instanceof Error ? err.message : String(err); + const message = withLockHint(err instanceof Error ? err.message : String(err)); toast.error(`${label} ${id} failed: ${message}`, { id: t }); fail(id, label, message); } finally { diff --git a/frontend/src/pages/StackDetail.tsx b/frontend/src/pages/StackDetail.tsx index 6c02ee5..eb93ad0 100644 --- a/frontend/src/pages/StackDetail.tsx +++ b/frontend/src/pages/StackDetail.tsx @@ -18,6 +18,7 @@ import { ConfirmDialog } from "@/components/ui/ConfirmDialog"; import { LogViewer } from "@/components/stacks/LogViewer"; import { ContainerCard } from "@/components/stacks/ContainerCard"; import { ActionStatusList } from "@/components/stacks/ActionStatusBanner"; +import { StackLockBanner } from "@/components/stacks/StackLockBanner"; import { AutoUpdatePanel } from "@/components/stacks/AutoUpdatePanel"; import { SecretsPanel } from "@/components/stacks/SecretsPanel"; import { GitPanel } from "@/components/stacks/GitPanel"; @@ -43,8 +44,10 @@ export function StackDetail() { const { data, isLoading } = useQuery({ queryKey: ["stack", id], queryFn: () => stacksApi.get(id), - // Container state arrives over /ws/events; this is the fallback. - refetchInterval: 30000, + // Container state arrives over /ws/events; this is the fallback. A stack + // held by an operation is watched closely instead — the banner's "stop + // the backup" has to turn back into the normal page once it worked. + refetchInterval: (query) => (query.state.data?.lock ? 3000 : 30000), }); if (isLoading || !data) return ; @@ -99,6 +102,8 @@ export function StackDetail() { )} + {data.lock && isAdmin && } + {/* Tabs */} diff --git a/frontend/src/types/index.ts b/frontend/src/types/index.ts index fe5dd4d..3f1db9f 100644 --- a/frontend/src/types/index.ts +++ b/frontend/src/types/index.ts @@ -48,6 +48,35 @@ export interface ContainerInfo { created?: string | null; } +/** The operation holding a stack, from `GET /api/stacks/{id}/lock` (and + * inlined in the stack detail). One compose/backup operation runs per stack + * at a time; while the lock is held, start/stop/restart/deploy are refused. */ +export interface StackLock { + /** "backup", "restore", "update", "start", "git-deploy", … */ + action: string; + owner: string; + acquired_at: string; + expires_at: string; + held_for: number; + /** The operation is running here and can be stopped halfway (a backup can, + * a restore must not). */ + cancellable: boolean; + cancelling: boolean; + /** Nothing is running behind this lock any more — it is a leftover and can + * be released without stopping anything. */ + stale: boolean; + /** What the operation is doing right now ("volume media-data"). */ + phase: string; +} + +export interface StackUnlockResult { + ok: boolean; + action: string; + cancelled: boolean; + released: boolean; + forced: boolean; +} + export interface StackDetail { id: string; name: string; @@ -55,6 +84,8 @@ export interface StackDetail { icon?: string | null; auto_icon?: string | null; status: StackStatus; + /** Set while an operation holds the stack; null otherwise. */ + lock?: StackLock | null; yaml: string; env: string; containers: ContainerInfo[];