From 0e46a99c5089ecd941fc2a4294c54edeaa5bd387 Mon Sep 17 00:00:00 2001 From: Ismael Leon Date: Thu, 20 Aug 2026 20:22:52 -0600 Subject: [PATCH] Make the systemd units declarative update.sh reloaded the code but never reinstalled the units, so a pull that changed how the bot is run - its sandboxing, resource caps, the updater schedule - left the old configuration in place while the repo said otherwise. Silent drift, and it bit during the last deploy: dropping EnvironmentFile from the unit needed setup.sh re-run by hand. Copying the definition into update.sh would have created two sources of truth, so it moves to install-units.sh instead and both scripts call it. Writing two files and reloading is cheap enough to do on every update, which makes the running configuration match the commit by construction. Two guards, both verified non-vacuous: - No deploy script may declare EnvironmentFile. It now scans every script rather than one by name - the previous version pointed at setup.sh and would have silently stopped checking anything when the unit moved out of it. - The service unit must be defined exactly once. Confirmed by injecting a duplicate definition and watching it fail. --- deploy/install-units.sh | 101 ++++++++++++++++++++++++++++++++++++++++ deploy/setup.sh | 86 ++-------------------------------- deploy/update.sh | 5 ++ tests/test_config.py | 45 +++++++++++++----- 4 files changed, 144 insertions(+), 93 deletions(-) create mode 100644 deploy/install-units.sh diff --git a/deploy/install-units.sh b/deploy/install-units.sh new file mode 100644 index 0000000..8e8fcc1 --- /dev/null +++ b/deploy/install-units.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +# +# Write the systemd units and reload. The single source of truth for how this +# bot runs — both setup.sh (first install) and update.sh (every update) call it, +# so the running configuration always matches what is committed. +# +# Idempotent and cheap: two small files plus a daemon-reload. It does NOT start +# or restart anything; the caller decides that. +# +# Usage: +# bash deploy/install-units.sh +# +set -euo pipefail + +APP_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +VENV_DIR="$APP_DIR/.venv" +SERVICE_NAME="loopify-bot" +UPDATER_NAME="loopify-ytdlp-update" +# Whoever owns the checkout is who the bot runs as. +APP_USER="$(stat -c '%U' "$APP_DIR")" + +echo "==> Installing systemd units (user: $APP_USER, dir: $APP_DIR)" + +sudo tee "/etc/systemd/system/${SERVICE_NAME}.service" >/dev/null </dev/null </dev/null < Installing systemd service '$SERVICE_NAME'..." -sudo tee "/etc/systemd/system/${SERVICE_NAME}.service" >/dev/null < Installing yt-dlp auto-update timer..." -chmod +x "$APP_DIR/deploy/update-ytdlp.sh" - -sudo tee "/etc/systemd/system/${SERVICE_NAME%-bot}-ytdlp-update.service" >/dev/null </dev/null < Done. Manage the bot with:" diff --git a/deploy/update.sh b/deploy/update.sh index 4bbda80..fe997e1 100644 --- a/deploy/update.sh +++ b/deploy/update.sh @@ -64,6 +64,11 @@ else "$VENV_DIR/bin/pip" install --quiet --upgrade -r requirements.txt fi +# Reinstall the units every time. A pull can change how the bot is *run* — its +# sandboxing, resource caps, the updater schedule — and restarting alone would +# silently keep the old configuration while the repo says otherwise. +bash "$APP_DIR/deploy/install-units.sh" + echo "==> Restarting $SERVICE_NAME..." sudo systemctl restart "$SERVICE_NAME" diff --git a/tests/test_config.py b/tests/test_config.py index 376c315..a8d1763 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -148,17 +148,40 @@ def test_dotenv_is_anchored_to_the_project_root(): assert os.path.isfile(os.path.join(config.PROJECT_ROOT, "config.py")) -def test_the_systemd_unit_does_not_also_parse_env(): +def _deploy_scripts() -> list: + import glob + import os + found = glob.glob(os.path.join(config.PROJECT_ROOT, "deploy", "*.sh")) + assert found, "no deploy scripts found — this guard would pass vacuously" + return found + + +def test_no_systemd_unit_also_parses_env(): """ Two parsers over one file disagree on quoting and fail silently. Only - python-dotenv reads .env; the unit must not declare EnvironmentFile. + python-dotenv reads .env; no unit may declare EnvironmentFile. + + Scans every deploy script rather than one by name, so moving the unit + definition between files cannot quietly turn this guard off. """ - import os - setup = os.path.join(config.PROJECT_ROOT, "deploy", "setup.sh") - with open(setup, encoding="utf-8") as handle: - body = handle.read() - active = [ - line for line in body.splitlines() - if "EnvironmentFile" in line and not line.lstrip().startswith("#") - ] - assert active == [] + offenders = [] + for path in _deploy_scripts(): + with open(path, encoding="utf-8") as handle: + for number, line in enumerate(handle, 1): + if "EnvironmentFile" in line and not line.lstrip().startswith("#"): + offenders.append(f"{path}:{number}") + assert offenders == [] + + +def test_the_bot_unit_is_defined_exactly_once(): + """ + setup.sh and update.sh both install units. If either grew its own copy of + the definition they would drift, and the deployed config would depend on + which script ran last. + """ + definers = [] + for path in _deploy_scripts(): + with open(path, encoding="utf-8") as handle: + if "Description=LoopifyBot Discord Music Bot" in handle.read(): + definers.append(path) + assert len(definers) == 1, f"the service unit is defined in {len(definers)} places: {definers}"