build: harden controller v2 runtime
This commit is contained in:
@@ -2,6 +2,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import math
|
||||
import os
|
||||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
@@ -135,6 +136,8 @@ def _number(value: Any, name: str, *, minimum: float = 0.0) -> float:
|
||||
result = float(value)
|
||||
except (TypeError, ValueError, OverflowError) as exc:
|
||||
raise ConfigError(f"{name} must be a number") from exc
|
||||
if not math.isfinite(result):
|
||||
raise ConfigError(f"{name} must be finite")
|
||||
if result < minimum:
|
||||
raise ConfigError(f"{name} must be >= {minimum}")
|
||||
return result
|
||||
@@ -240,6 +243,10 @@ def load_config(
|
||||
raise ConfigError("storage.database_name must be a filename")
|
||||
if Path(storage.legacy_yaml_name).name != storage.legacy_yaml_name:
|
||||
raise ConfigError("storage.legacy_yaml_name must be a filename")
|
||||
if not storage.media_base.is_absolute():
|
||||
raise ConfigError("storage.media_base must be an absolute path")
|
||||
if not storage.data_base.is_absolute():
|
||||
raise ConfigError("storage.data_base must be an absolute path")
|
||||
|
||||
cookie_raw = _env_value(env, "YTPLAYER_COOKIE_PATH", downloads_raw.get("cookie_path", "/run/secrets/yt-dlp-cookies.txt"))
|
||||
downloads = DownloadConfig(
|
||||
@@ -252,6 +259,8 @@ def load_config(
|
||||
)
|
||||
if downloads.cookie_required and downloads.cookie_path is None:
|
||||
raise ConfigError("downloads.cookie_path is required when cookie_required is true")
|
||||
if downloads.cookie_path is not None and not downloads.cookie_path.is_absolute():
|
||||
raise ConfigError("downloads.cookie_path must be an absolute path")
|
||||
|
||||
unknown_behavior = _text(router_raw.get("when_state_unknown", "attempt"), "router.when_state_unknown")
|
||||
if unknown_behavior not in {"attempt", "continue", "reject"}:
|
||||
|
||||
+118
-48
@@ -7,9 +7,12 @@ import os
|
||||
import signal
|
||||
import shutil
|
||||
import subprocess
|
||||
import tempfile
|
||||
import threading
|
||||
import time
|
||||
import uuid
|
||||
from collections.abc import Iterator
|
||||
from contextlib import contextmanager
|
||||
from concurrent.futures import Future, ThreadPoolExecutor
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
@@ -171,37 +174,78 @@ class DownloadManager:
|
||||
except BaseException:
|
||||
LOG.exception("download event callback failed")
|
||||
|
||||
def _cookie_arguments(self) -> list[str]:
|
||||
path = self.config.downloads.cookie_path
|
||||
if path is None:
|
||||
return []
|
||||
if path.is_file():
|
||||
return ["--cookies", str(path)]
|
||||
def _working_cookie_copy(self) -> Path | None:
|
||||
source = self.config.downloads.cookie_path
|
||||
if source is None:
|
||||
return None
|
||||
if source.is_file():
|
||||
descriptor, temporary_name = tempfile.mkstemp(
|
||||
prefix="ytplayer-cookies-",
|
||||
suffix=".txt",
|
||||
)
|
||||
temporary_path = Path(temporary_name)
|
||||
try:
|
||||
with os.fdopen(descriptor, "wb") as destination:
|
||||
with source.open("rb") as source_file:
|
||||
shutil.copyfileobj(source_file, destination)
|
||||
destination.flush()
|
||||
os.fsync(destination.fileno())
|
||||
temporary_path.chmod(0o600)
|
||||
return temporary_path
|
||||
except BaseException:
|
||||
temporary_path.unlink(missing_ok=True)
|
||||
raise
|
||||
if self.config.downloads.cookie_required:
|
||||
raise FileNotFoundError(f"required yt-dlp cookie file is missing: {path}")
|
||||
LOG.warning("yt-dlp cookie file is unavailable; continuing without it: %s", path)
|
||||
return []
|
||||
raise FileNotFoundError(
|
||||
f"required yt-dlp cookie file is missing: {source}"
|
||||
)
|
||||
LOG.warning(
|
||||
"yt-dlp cookie file is unavailable; continuing without it: %s",
|
||||
source,
|
||||
)
|
||||
return None
|
||||
|
||||
def _base_yt_dlp_arguments(self) -> list[str]:
|
||||
return [self.config.downloads.yt_dlp_binary, "--ignore-config"] + self._cookie_arguments()
|
||||
@contextmanager
|
||||
def _yt_dlp_arguments(self) -> Iterator[list[str]]:
|
||||
working_cookie = self._working_cookie_copy()
|
||||
arguments = [
|
||||
self.config.downloads.yt_dlp_binary,
|
||||
"--ignore-config",
|
||||
"--no-js-runtimes",
|
||||
"--js-runtimes",
|
||||
"node",
|
||||
"--cache-dir",
|
||||
"/tmp/yt-dlp-cache",
|
||||
]
|
||||
if working_cookie is not None:
|
||||
arguments.extend(("--cookies", str(working_cookie)))
|
||||
try:
|
||||
yield arguments
|
||||
finally:
|
||||
if working_cookie is not None:
|
||||
working_cookie.unlink(missing_ok=True)
|
||||
|
||||
def _get_title(self, url: str) -> tuple[str, str | None]:
|
||||
arguments = self._base_yt_dlp_arguments() + [
|
||||
"--flat-playlist",
|
||||
"--print",
|
||||
"%(playlist_title)s|||%(title)s|||%(id)s",
|
||||
"--playlist-end",
|
||||
"1",
|
||||
url,
|
||||
]
|
||||
try:
|
||||
result = self._run_command(
|
||||
arguments,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=min(120.0, self.config.downloads.process_timeout_seconds),
|
||||
)
|
||||
with self._yt_dlp_arguments() as base_arguments:
|
||||
arguments = base_arguments + [
|
||||
"--flat-playlist",
|
||||
"--print",
|
||||
"%(playlist_title)s|||%(title)s|||%(id)s",
|
||||
"--playlist-end",
|
||||
"1",
|
||||
url,
|
||||
]
|
||||
result = self._run_command(
|
||||
arguments,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=min(
|
||||
120.0,
|
||||
self.config.downloads.process_timeout_seconds,
|
||||
),
|
||||
)
|
||||
line = (result.stdout or "").strip().splitlines()[0]
|
||||
playlist_title, _, remainder = line.partition("|||")
|
||||
video_title, _, yt_id = remainder.partition("|||")
|
||||
@@ -241,28 +285,29 @@ class DownloadManager:
|
||||
if slot_directory.is_symlink():
|
||||
raise RuntimeError(f"refusing to download through symlink: {slot_directory}")
|
||||
title, yt_id = self._get_title(url)
|
||||
arguments = self._base_yt_dlp_arguments() + [
|
||||
"--yes-playlist",
|
||||
"--no-overwrites",
|
||||
"--no-progress",
|
||||
"-o",
|
||||
str(
|
||||
slot_directory
|
||||
/ "%(playlist_index)03d - %(title)s [%(id)s].%(ext)s"
|
||||
),
|
||||
"-f",
|
||||
(
|
||||
f"bv*[height<={self.config.downloads.max_height}]+ba/"
|
||||
f"b[height<={self.config.downloads.max_height}]/b"
|
||||
),
|
||||
url,
|
||||
]
|
||||
self._run_command(
|
||||
arguments,
|
||||
check=True,
|
||||
text=True,
|
||||
timeout=self.config.downloads.process_timeout_seconds,
|
||||
)
|
||||
with self._yt_dlp_arguments() as base_arguments:
|
||||
arguments = base_arguments + [
|
||||
"--yes-playlist",
|
||||
"--no-overwrites",
|
||||
"--no-progress",
|
||||
"-o",
|
||||
str(
|
||||
slot_directory
|
||||
/ "%(playlist_index)03d - %(title)s [%(id)s].%(ext)s"
|
||||
),
|
||||
"-f",
|
||||
(
|
||||
f"bv*[height<={self.config.downloads.max_height}]+ba/"
|
||||
f"b[height<={self.config.downloads.max_height}]/b"
|
||||
),
|
||||
url,
|
||||
]
|
||||
self._run_command(
|
||||
arguments,
|
||||
check=True,
|
||||
text=True,
|
||||
timeout=self.config.downloads.process_timeout_seconds,
|
||||
)
|
||||
media_files = sorted(
|
||||
path
|
||||
for path in slot_directory.iterdir()
|
||||
@@ -467,6 +512,15 @@ class DownloadManager:
|
||||
recovered: list[int] = []
|
||||
trash_root = self.config.storage.media_base / ".controller-trash"
|
||||
for token in self.store.pending_deletions():
|
||||
if any(
|
||||
guard.slot == token.slot
|
||||
for guard in self.store.list_live_guards()
|
||||
):
|
||||
LOG.warning(
|
||||
"deferring interrupted deletion for protected slot %s",
|
||||
token.slot,
|
||||
)
|
||||
continue
|
||||
if trash_root.is_dir():
|
||||
prefix = f"slot{token.slot}.generation{token.generation}."
|
||||
for candidate in trash_root.iterdir():
|
||||
@@ -479,6 +533,22 @@ class DownloadManager:
|
||||
recovered.append(token.slot)
|
||||
return recovered
|
||||
|
||||
def schedule_pending_deletion_recovery(self) -> bool:
|
||||
"""Run recoverable trash cleanup without delaying service startup."""
|
||||
|
||||
with self._lock:
|
||||
if self._stopping:
|
||||
return False
|
||||
try:
|
||||
future = self._executor.submit(self.recover_pending_deletions)
|
||||
except BaseException:
|
||||
LOG.exception("could not schedule interrupted deletion recovery")
|
||||
return False
|
||||
with self._lock:
|
||||
self._futures.add(future)
|
||||
future.add_done_callback(self._forget_future)
|
||||
return True
|
||||
|
||||
def shutdown(self, *, wait: bool = True) -> None:
|
||||
with self._lock:
|
||||
self._stopping = True
|
||||
|
||||
@@ -141,7 +141,9 @@ class MqttAdapter:
|
||||
self.config.mqtt.port,
|
||||
keepalive=self.config.mqtt.keepalive_seconds,
|
||||
)
|
||||
if result != mqtt.MQTT_ERR_SUCCESS:
|
||||
# Paho 2.x returns None after configuring an asynchronous connect;
|
||||
# older/test clients may return MQTT_ERR_SUCCESS.
|
||||
if result not in (None, mqtt.MQTT_ERR_SUCCESS):
|
||||
raise RuntimeError(f"failed to configure MQTT connection: {result}")
|
||||
with self._lifecycle_lock:
|
||||
if (
|
||||
|
||||
+73
-4
@@ -97,6 +97,9 @@ class ControllerService:
|
||||
self._next_expiry = 0.0
|
||||
self._next_reconciliation = 0.0
|
||||
self._processed_packets: dict[tuple[int, str, int], float] = {}
|
||||
self._player_evidence_ready = False
|
||||
self._operator_cleanup_override = False
|
||||
self._recovery_scheduled = False
|
||||
|
||||
@property
|
||||
def started(self) -> bool:
|
||||
@@ -144,7 +147,11 @@ class ControllerService:
|
||||
self.store,
|
||||
on_event=self._queue_download_event,
|
||||
)
|
||||
self.downloads.recover_pending_deletions()
|
||||
# Cleanup is deliberately deferred until fresh Player evidence
|
||||
# (or an explicit administrative reconciliation) makes it safe.
|
||||
self._player_evidence_ready = False
|
||||
self._operator_cleanup_override = False
|
||||
self._recovery_scheduled = False
|
||||
self._stop_event.clear()
|
||||
self._next_expiry = (
|
||||
self._monotonic()
|
||||
@@ -271,6 +278,8 @@ class ControllerService:
|
||||
degraded: list[str] = []
|
||||
if started and not mqtt_connected:
|
||||
degraded.append("mqtt_disconnected")
|
||||
if started and not self._cleanup_allowed():
|
||||
degraded.append("awaiting_player_evidence")
|
||||
if (
|
||||
started
|
||||
and router_observation is not None
|
||||
@@ -375,6 +384,11 @@ class ControllerService:
|
||||
def _handle_call(self, action: str, payload: Any) -> Any:
|
||||
if action == "enqueue_download":
|
||||
assert self.downloads is not None
|
||||
if not self._cleanup_allowed():
|
||||
raise ControllerUnavailableError(
|
||||
"downloads are deferred until fresh Player evidence or "
|
||||
"administrative playback reconciliation"
|
||||
)
|
||||
reservation = self.downloads.enqueue(
|
||||
payload["url"], payload.get("submitter"), payload.get("note")
|
||||
)
|
||||
@@ -402,12 +416,16 @@ class ControllerService:
|
||||
handled_successfully = True
|
||||
return
|
||||
if event.kind == "connected":
|
||||
self._player_evidence_ready = False
|
||||
self._operator_cleanup_override = False
|
||||
self._announce_discovery()
|
||||
self._sync_slot_states(force=True)
|
||||
self._announce_reconciliation_reviews()
|
||||
handled_successfully = True
|
||||
return
|
||||
if event.kind == "disconnected":
|
||||
self._player_evidence_ready = False
|
||||
self._operator_cleanup_override = False
|
||||
assert self.playback is not None
|
||||
assert self.router is not None
|
||||
playback_effect = self.playback.mark_transport_uncertain(
|
||||
@@ -426,12 +444,27 @@ class ControllerService:
|
||||
payload = self._decode_json(event.payload, "Player status")
|
||||
if payload is not None:
|
||||
assert self.playback is not None
|
||||
before_generation = self.store.player_observation().generation
|
||||
self._handle_playback_effects(self.playback.handle_status(payload))
|
||||
self._refresh_player_evidence(before_generation)
|
||||
elif event.topic == self.config.topics.player_telemetry:
|
||||
payload = self._decode_json(event.payload, "Player telemetry")
|
||||
if payload is not None:
|
||||
assert self.playback is not None
|
||||
before_generation = self.store.player_observation().generation
|
||||
self._handle_playback_effects(self.playback.handle_telemetry(payload))
|
||||
self._refresh_player_evidence(before_generation)
|
||||
elif event.topic == self.config.topics.player_state:
|
||||
try:
|
||||
flat_state = event.payload.decode("utf-8").strip()
|
||||
except UnicodeDecodeError:
|
||||
flat_state = ""
|
||||
# Retained flat idle is sufficient only as a quiescence gate;
|
||||
# it never mutates detailed playback state or clears guards.
|
||||
self._player_evidence_ready = flat_state == "idle"
|
||||
self._operator_cleanup_override = False
|
||||
if self._player_evidence_ready:
|
||||
self._schedule_deletion_recovery()
|
||||
elif event.topic == self.config.topics.controller_command:
|
||||
if event.retain:
|
||||
self._publish_event(
|
||||
@@ -478,8 +511,8 @@ class ControllerService:
|
||||
else:
|
||||
decision = self.router.observe_mode(mode)
|
||||
self._handle_router_decision(decision)
|
||||
# Player flat state is intentionally only a Companion/cold-start
|
||||
# hint. It is never reduced as authoritative playback truth.
|
||||
# Flat state is only a cold-start cleanup gate. Detailed playback
|
||||
# truth continues to come exclusively from status and telemetry.
|
||||
handled_successfully = True
|
||||
finally:
|
||||
if handled_successfully:
|
||||
@@ -828,6 +861,14 @@ class ControllerService:
|
||||
|
||||
def _request_delete(self, payload: Mapping[str, Any]) -> dict[str, Any]:
|
||||
assert self.downloads is not None
|
||||
if (
|
||||
not self._cleanup_allowed()
|
||||
and payload.get("confirm_player_unknown") is not True
|
||||
):
|
||||
raise SlotProtectedError(
|
||||
"delete requires fresh Player evidence or "
|
||||
"confirm_player_unknown=true"
|
||||
)
|
||||
slot = self._parse_slot(payload.get("slot"))
|
||||
expected_url = payload.get("url")
|
||||
if expected_url is not None and not isinstance(expected_url, str):
|
||||
@@ -886,6 +927,14 @@ class ControllerService:
|
||||
)
|
||||
if not cleared:
|
||||
raise StaleOperationError("reconciliation review_id is stale")
|
||||
observation = self.store.player_observation()
|
||||
if (
|
||||
not self.store.list_live_guards()
|
||||
and observation.active_slot is None
|
||||
and observation.active_path is None
|
||||
):
|
||||
self._operator_cleanup_override = True
|
||||
self._schedule_deletion_recovery()
|
||||
assert self.router is not None
|
||||
self.router.abandon(
|
||||
self._session_lease_id,
|
||||
@@ -1118,6 +1167,26 @@ class ControllerService:
|
||||
if topic is not None:
|
||||
self.mqtt.publish_json(topic, payload, qos=1, retain=False)
|
||||
|
||||
def _cleanup_allowed(self) -> bool:
|
||||
return self._player_evidence_ready or self._operator_cleanup_override
|
||||
|
||||
def _schedule_deletion_recovery(self) -> None:
|
||||
if self._recovery_scheduled or not self._cleanup_allowed():
|
||||
return
|
||||
assert self.downloads is not None
|
||||
self._recovery_scheduled = (
|
||||
self.downloads.schedule_pending_deletion_recovery()
|
||||
)
|
||||
|
||||
def _refresh_player_evidence(self, before_generation: int) -> None:
|
||||
observation = self.store.player_observation()
|
||||
if observation.generation == before_generation:
|
||||
return
|
||||
self._operator_cleanup_override = False
|
||||
self._player_evidence_ready = observation.availability == "online"
|
||||
if self._player_evidence_ready and observation.phase == "idle":
|
||||
self._schedule_deletion_recovery()
|
||||
|
||||
def _run_timers(self) -> None:
|
||||
assert self.playback is not None
|
||||
assert self.router is not None
|
||||
@@ -1133,7 +1202,7 @@ class ControllerService:
|
||||
if now >= self._next_reconciliation:
|
||||
self._handle_playback_effects(self.playback.mark_due_reconciliation())
|
||||
self._next_reconciliation = now + 1.0
|
||||
if now >= self._next_expiry:
|
||||
if now >= self._next_expiry and self._cleanup_allowed():
|
||||
for slot in self.downloads.expire_due_slots():
|
||||
self._announce_slot(slot, force=True)
|
||||
self._next_expiry = now + self.config.storage.expiry_interval_seconds
|
||||
|
||||
Reference in New Issue
Block a user