diff --git a/media_preview_generator/web/app.py b/media_preview_generator/web/app.py index 252599ee..c114018c 100644 --- a/media_preview_generator/web/app.py +++ b/media_preview_generator/web/app.py @@ -549,6 +549,15 @@ def create_app(config_dir: str | None = None) -> Flask: if config_dir is None: config_dir = os.environ.get("CONFIG_DIR", "/config") + # Preflight the config directory FIRST so a read-only / wrong-owner + # /config surfaces as one clear log line at boot (issue #278) instead of + # a flood of per-item "attempt to write a readonly database" failures. + # The dashboard reads the same probe via /api/system/config-health to + # render a banner; scan + schedule creation refuse to start when it fails. + from .config_health import log_config_health + + log_config_health(config_dir) + # Create Flask app app = Flask(__name__, template_folder="templates", static_folder="static") diff --git a/media_preview_generator/web/config_health.py b/media_preview_generator/web/config_health.py new file mode 100644 index 00000000..f685a66e --- /dev/null +++ b/media_preview_generator/web/config_health.py @@ -0,0 +1,236 @@ +"""Config-directory health probe. + +At startup (and on demand from the dashboard) we check that the mounted +config directory (``/config`` by default) is actually usable: writable by +the process user, not a read-only mount, on a filesystem where SQLite's WAL +locking is reliable, and not out of space. + +A read-only ``/config`` is the single most common "nothing works" support +report (issue #278): every write to ``jobs.db`` / ``scheduler.db`` / +``settings.json`` fails with SQLite's opaque ``attempt to write a readonly +database`` and the UI looks frozen while the log floods with one persist +failure per item. Surfacing ONE clear, actionable message — is it an +ownership mismatch, an explicit ``:ro`` mount, a full disk, or a flaky +network share? — beats that failure mode entirely. +""" + +import os +import uuid + +from loguru import logger + +# Warn when free space on the config filesystem drops below this. SQLite +# needs headroom for the WAL + rollback journal; under a few MB, writes +# start failing with "database or disk is full" — a different error with +# the same "nothing saves" symptom. +_LOW_SPACE_THRESHOLD_BYTES = 50 * 1024 * 1024 # 50 MB + +# Filesystem types where SQLite's POSIX file-locking / WAL is unreliable. +# Pointing /config at an NFS/SMB share is a common NAS setup that produces +# intermittent "database is locked" errors — we warn, never block, because +# it often works well enough that a hard failure would be wrong. +_NETWORK_FS_TYPES = {"nfs", "nfs4", "cifs", "smbfs", "smb3", "ncpfs", "9p"} + + +def _proc_mounts() -> list[tuple[str, str, set[str]]]: + """Parse ``/proc/mounts`` into ``(mountpoint, fstype, options)`` tuples. + + Returns an empty list on any platform without ``/proc/mounts`` (macOS, + Windows) so callers degrade to "no mount info" rather than raising. + """ + entries: list[tuple[str, str, set[str]]] = [] + try: + with open("/proc/mounts", encoding="utf-8") as fh: + for line in fh: + parts = line.split() + if len(parts) < 4: + continue + # /proc/mounts octal-escapes spaces in the mountpoint. + mountpoint = parts[1].replace("\\040", " ") + entries.append((mountpoint, parts[2], set(parts[3].split(",")))) + except OSError: + return [] + return entries + + +def _mount_for_path(path: str) -> tuple[str, set[str]] | None: + """Return ``(fstype, options)`` of the mount that contains ``path``. + + Picks the longest matching mountpoint (most specific) so a bind-mount + at ``/config`` wins over the root filesystem mounted at ``/``. Returns + ``None`` when no mount info is available. + """ + real = os.path.realpath(path) + best: tuple[int, str, set[str]] | None = None + for mountpoint, fstype, options in _proc_mounts(): + mp = os.path.realpath(mountpoint) + if real == mp or real.startswith(mp.rstrip("/") + "/"): + score = len(mp) + if best is None or score > best[0]: + best = (score, fstype, options) + if best is None: + return None + return best[1], best[2] + + +def probe_config_health(config_dir: str) -> dict: + """Probe ``config_dir`` for the conditions that silently break persistence. + + The authoritative check is an actual create+delete of a probe file — it + catches every reason a write can fail (ownership, ``:ro`` mount, full + disk, ACLs) in one go. The mount/space lookups only exist to turn that + boolean into a *specific* fix ("remove :ro" vs "chown" vs "free space"). + + Args: + config_dir: The configuration directory (e.g. ``/config``). + + Returns: + A wire-friendly dict the dashboard renders directly: + + * ``writable`` (bool) — the load-bearing field; ``False`` means + settings, schedules, and job history cannot be saved. + * ``status`` — ``"ok"`` | ``"not_writable"`` | ``"read_only_mount"``. + * ``detail`` / ``hint`` — human message + the exact host-side fix. + * ``warnings`` — list of non-fatal advisories (network fs, low space), + each ``{"kind", "message"}``. + * diagnostics: ``process_user``, ``dir_owner``, ``dir_mode``, + ``read_only_mount``, ``network_fs``, ``free_bytes``, ``low_space``. + """ + process_user = f"{os.getuid()}:{os.getgid()}" + result: dict = { + "path": config_dir, + "writable": True, + "status": "ok", + "read_only_mount": False, + "network_fs": None, + "free_bytes": None, + "low_space": False, + "process_user": process_user, + "dir_owner": None, + "dir_mode": None, + "detail": "", + "hint": "", + "warnings": [], + } + + # Best-effort: create the directory so a first-run probe of a + # not-yet-created /config still reports a meaningful writability result + # (a parent that's read-only makes this fail, which is itself the answer). + try: + os.makedirs(config_dir, exist_ok=True) + except OSError: + pass + + try: + st = os.stat(config_dir) + result["dir_owner"] = f"{st.st_uid}:{st.st_gid}" + result["dir_mode"] = oct(st.st_mode & 0o777) + except OSError: + pass + + mount = _mount_for_path(config_dir) + if mount is not None: + fstype, options = mount + if fstype in _NETWORK_FS_TYPES or fstype.startswith("fuse"): + result["network_fs"] = fstype + if "ro" in options: + result["read_only_mount"] = True + + try: + stv = os.statvfs(config_dir) + free = stv.f_bavail * stv.f_frsize + result["free_bytes"] = free + result["low_space"] = free < _LOW_SPACE_THRESHOLD_BYTES + except OSError: + pass + + # Authoritative writability check — create a probe file. The name is + # unique per attempt: pid alone is NOT enough because the deployment runs + # gunicorn gthread with one worker (one PID), so concurrent probes (every + # open tab polls /config-health, and create_job/create_schedule probe too) + # would race on a shared path — a losing racer's cleanup unlink would raise + # and spuriously report the dir unwritable (false 503 / red banner). + probe_path = os.path.join(config_dir, f".config-write-probe-{os.getpid()}-{uuid.uuid4().hex}") + write_ok = False + try: + fd = os.open(probe_path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + os.close(fd) + write_ok = True + except OSError as exc: + result["writable"] = False + result["error_type"] = type(exc).__name__ + result["error_message"] = str(exc) + if result["read_only_mount"]: + result["status"] = "read_only_mount" + result["detail"] = f"Config folder {config_dir} is mounted read-only." + result["hint"] = ( + f"Remove the ':ro' flag from the volume mapped to {config_dir} " + "(or set the dataset/share to read-write), then restart the container." + ) + else: + result["status"] = "not_writable" + owner = result["dir_owner"] or "another user" + result["detail"] = f"Config folder {config_dir} isn't writable by this container." + result["hint"] = ( + f"The app runs as {process_user} but {config_dir} is owned by {owner}. " + f"On the host run `chown -R {process_user} ` " + "(or set PUID/PGID to match the owner), then restart the container." + ) + + # Best-effort cleanup — a probe that WROTE already proved writability, so a + # cleanup hiccup must never flip the verdict back to unwritable. + if write_ok: + try: + os.unlink(probe_path) + except OSError: + logger.debug("Could not remove config write-probe {}", probe_path) + + # Non-fatal advisories only make sense when writes actually work; a + # read-only mount already dominates the message. + if result["writable"]: + if result["network_fs"]: + result["warnings"].append( + { + "kind": "network_fs", + "message": ( + f"Config folder {config_dir} is on a '{result['network_fs']}' network " + "filesystem. SQLite's file locking is unreliable over network shares and " + "can cause intermittent 'database is locked' errors — a local disk/volume " + "for the config folder is strongly recommended." + ), + } + ) + if result["low_space"] and result["free_bytes"] is not None: + free_mb = result["free_bytes"] // (1024 * 1024) + result["warnings"].append( + { + "kind": "low_space", + "message": ( + f"Only {free_mb} MB free on the config filesystem. Settings, schedules, and " + "job history will start failing to save when it fills up — free up space." + ), + } + ) + + return result + + +def log_config_health(config_dir: str) -> dict: + """Probe ``config_dir`` and log ONE actionable message on any problem. + + Called once at startup so a broken mount is visible in ``docker logs`` + immediately — instead of being discovered as a flood of per-item + "attempt to write a readonly database" persist failures. Returns the + probe result so the caller can reuse it. + """ + health = probe_config_health(config_dir) + if not health["writable"]: + logger.error( + "{} {} Until this is fixed, settings, schedules, and job history cannot be " + "saved and the dashboard will appear frozen while jobs run.", + health["detail"], + health["hint"], + ) + for warning in health["warnings"]: + logger.warning(warning["message"]) + return health diff --git a/media_preview_generator/web/routes/api_jobs.py b/media_preview_generator/web/routes/api_jobs.py index 69cbe067..1dff6308 100644 --- a/media_preview_generator/web/routes/api_jobs.py +++ b/media_preview_generator/web/routes/api_jobs.py @@ -6,7 +6,7 @@ from concurrent.futures import ThreadPoolExecutor from datetime import datetime -from flask import jsonify, request, session +from flask import current_app, jsonify, request, session from loguru import logger from ..auth import ( @@ -602,6 +602,28 @@ def _reconstruct_retry_reason(child_job) -> dict | None: ) +def _config_unwritable_response(): + """Return a 503 (payload, status) tuple when /config isn't writable, else None. + + Starting a scan on a read-only /config lets FFmpeg burn CPU while every + job-state persist to jobs.db fails and the UI looks frozen (issue #278). + Refuse up front with the actionable fix instead. Returns ``None`` when + the config dir is writable so callers can proceed. + """ + from ..config_health import probe_config_health + + config_dir = current_app.config.get("CONFIG_DIR", "/config") + health = probe_config_health(config_dir) + if health["writable"]: + return None + logger.error( + "Refusing to start a job — {} {}", + health["detail"], + health["hint"], + ) + return jsonify({"error": health["detail"], "hint": health["hint"], "config_health": health}), 503 + + @api.route("/jobs", methods=["POST"]) @api_token_required def create_job(): @@ -612,6 +634,10 @@ def create_job(): * ``library_names: list[str]`` — back-compat from older clients. * ``library_id: str`` — back-compat single-library shape. """ + blocked = _config_unwritable_response() + if blocked is not None: + return blocked + data = request.get_json() or {} library_ids = list(data.get("library_ids") or []) @@ -700,6 +726,10 @@ def create_manual_job(): 201 with job dict on success, 400 on validation failure. """ + blocked = _config_unwritable_response() + if blocked is not None: + return blocked + data = request.get_json() or {} raw_paths = data.get("file_paths") or [] force_regenerate = _param_to_bool(data.get("force_regenerate"), False) diff --git a/media_preview_generator/web/routes/api_schedules.py b/media_preview_generator/web/routes/api_schedules.py index 4b8db464..f1a42b3d 100644 --- a/media_preview_generator/web/routes/api_schedules.py +++ b/media_preview_generator/web/routes/api_schedules.py @@ -1,6 +1,6 @@ """Schedule management API routes.""" -from flask import jsonify, request +from flask import current_app, jsonify, request from loguru import logger from ..auth import api_token_required @@ -105,6 +105,29 @@ def create_schedule(): ) return jsonify({"error": "Invalid schedule parameters"}), 400 except Exception as e: + # A read-only /config makes APScheduler's INSERT into scheduler.db + # fail with "attempt to write a readonly database" — nothing to do + # with cron syntax. Detect it and give the actionable permissions fix + # instead of the misleading duplicate/cron hint (issue #278). + if "readonly database" in str(e).lower() or "unable to open database" in str(e).lower(): + from ..config_health import probe_config_health + + health = probe_config_health(current_app.config.get("CONFIG_DIR", "/config")) + # The re-probe may transiently disagree with the DB write that just + # failed (race, or config made writable in the interim) — never emit + # a blank 503; fall back to the DB error's own message. + detail = health["detail"] or "The config folder isn't writable, so the schedule can't be saved." + hint = health["hint"] or ( + "Make the config folder writable by the container (check PUID/PGID and that the " + "mount isn't read-only), then try again." + ) + logger.error( + "Could not save the new schedule {!r} — the config folder isn't writable. {} {}", + data.get("name", ""), + detail, + hint, + ) + return jsonify({"error": detail, "hint": hint, "config_health": health}), 503 logger.exception( "Could not save the new schedule {!r} ({}: {}). " "Most often this is a malformed cron expression or a clash with an existing schedule — " diff --git a/media_preview_generator/web/routes/api_system.py b/media_preview_generator/web/routes/api_system.py index 0bf5ac5e..539e6a5a 100644 --- a/media_preview_generator/web/routes/api_system.py +++ b/media_preview_generator/web/routes/api_system.py @@ -7,7 +7,7 @@ import threading import time -from flask import jsonify, request +from flask import current_app, jsonify, request from loguru import logger from ...logging_config import LEVEL_ORDER, get_app_log_path @@ -221,6 +221,48 @@ def get_system_status(): return jsonify({"error": "Failed to retrieve system status"}), 500 +def _current_media_mount_issues() -> list[dict[str, str]]: + """Live re-probe of configured media mounts for the dashboard banner. + + Cheap ``os.stat``/``listdir`` checks over each server's ``local_prefix`` + so a stale bind-mount (empty underlay) or an unmounted share is visible + the same way a read-only /config is — instead of surfacing job-by-job as + "missing on disk" failures. Recomputed per poll so the banner clears once + the share mounts, without a restart. + """ + from ...config.paths import detect_unhealthy_media_mounts + from ..app import _get_media_servers_from_settings + + config_dir = current_app.config.get("CONFIG_DIR", "/config") + mappings: list[dict] = [] + for server in _get_media_servers_from_settings(config_dir) or []: + if isinstance(server, dict): + mappings.extend(server.get("path_mappings") or []) + return detect_unhealthy_media_mounts(mappings) + + +@api.route("/system/config-health") +@setup_or_auth_required +def get_config_health(): + """Report config-dir writability + media-mount health for the UI banner. + + Kept intentionally cheap (no GPU detection) so every page can poll it to + render the "config folder isn't writable" / "media mount missing" banner. + Available during setup too, since a read-only /config blocks the wizard. + """ + from ..config_health import probe_config_health + + config_dir = current_app.config.get("CONFIG_DIR", "/config") + payload = {"config": probe_config_health(config_dir)} + try: + payload["media_mount_issues"] = _current_media_mount_issues() + except Exception: + # A settings-read failure must never break the health banner itself. + logger.debug("Could not probe media-mount health for the config-health banner", exc_info=True) + payload["media_mount_issues"] = [] + return jsonify(payload) + + _media_server_status_cache: dict = {"result": None, "fetched_at": 0.0} _media_server_status_lock = threading.Lock() _MEDIA_SERVER_STATUS_TTL = 30 # seconds diff --git a/media_preview_generator/web/static/js/app.js b/media_preview_generator/web/static/js/app.js index c03c523b..6ebca10a 100644 --- a/media_preview_generator/web/static/js/app.js +++ b/media_preview_generator/web/static/js/app.js @@ -3950,8 +3950,81 @@ document.addEventListener('DOMContentLoaded', () => { // For elements added dynamically after page load, call // window._initBootstrapTooltips(scope) — see below. _initBootstrapTooltips(document); + + // Config-health banner (issue #278) — runs on every page so a + // read-only /config or a missing media mount is visible even on the + // Settings/Schedules pages, not just the dashboard. Polled so it + // clears once the user fixes permissions (no restart needed). + if (document.getElementById('configHealthBanner')) { + checkConfigHealth(); + setInterval(checkConfigHealth, 60000); + } }); +/** + * Fetch config-dir + media-mount health and render the global banner. + * Red (danger) when /config isn't writable — the app can't save anything; + * yellow (warning) for non-fatal advisories (network fs, low space, + * missing media mount). Clears when everything is healthy. + */ +async function checkConfigHealth() { + const banner = document.getElementById('configHealthBanner'); + if (!banner) return; + // Raw fetch, NOT apiGet: apiGet redirects to /login on 401, which would let + // this passive background probe hijack navigation (e.g. bounce the setup + // wizard to login when the config-health endpoint is auth-gated). A health + // check must never move the user — silently bail on any non-OK response. + let data; + try { + const resp = await fetch('/api/system/config-health'); + if (!resp.ok) return; + data = await resp.json(); + } catch (e) { + // Network hiccup — leave the last banner state, never blank the page. + return; + } + const cfg = data.config || {}; + const cards = []; + + if (cfg.writable === false) { + cards.push(` + `); + } + (cfg.warnings || []).forEach((w) => { + cards.push(` + `); + }); + (data.media_mount_issues || []).forEach((m) => { + const msg = m.issue === 'empty' + ? `Media mount "${m.path}" is mounted but EMPTY inside the container (stale bind-mount / unmounted share). Every preview job for files on this disk will fail as "missing on disk".` + : `Media mount "${m.path}" doesn't exist inside the container — check the volume mount and the path mapping under Settings → Media Servers.`; + cards.push(` + `); + }); + + if (cards.length === 0) { + banner.classList.add('d-none'); + banner.innerHTML = ''; + return; + } + banner.innerHTML = cards.join(''); + banner.classList.remove('d-none'); +} +window.checkConfigHealth = checkConfigHealth; + /** * Initialise Bootstrap tooltips on every `[data-bs-toggle="tooltip"]` * element under ``scope`` (defaults to the whole document). Safe to diff --git a/media_preview_generator/web/templates/base.html b/media_preview_generator/web/templates/base.html index e53d176e..7b5e4cef 100644 --- a/media_preview_generator/web/templates/base.html +++ b/media_preview_generator/web/templates/base.html @@ -294,6 +294,13 @@