diff --git a/.github/scripts/tests/test_log2ram_setup.py b/.github/scripts/tests/test_log2ram_setup.py index 40b0af1e..864dfa4f 100644 --- a/.github/scripts/tests/test_log2ram_setup.py +++ b/.github/scripts/tests/test_log2ram_setup.py @@ -5,6 +5,7 @@ and interval the host already has.""" import os from pathlib import Path import shutil +import shlex import subprocess import tempfile import unittest @@ -25,10 +26,176 @@ class KeepOnDiskTests(unittest.TestCase): (self.log / "journal/abc/system.journal").write_text("entries") (self.log / "pveproxy").mkdir() (self.log / "pveproxy/access.log").write_text("GET /") + self.config = self.root / "etc/log2ram.conf" + self.config.parent.mkdir() + self.config.write_text("SIZE=128M\nJOURNALD_AWARE=true\nNOTIFICATION=true\n") + + def run_setup(self, edit_failure=False): + # Extract only the shared setup function, never an administrative caller. + body = SETUP.read_text().split("pmx_log2ram_setup() {", 1)[1].split("\n}\n", 1)[0] + body = body.replace("/etc/", str(self.root / "etc") + "/") + for directory in ("cron.d", "tmpfiles.d"): + (self.root / "etc" / directory).mkdir(exist_ok=True) + stubs = r''' +translate() { printf '%s' "$1"; } +msg_info() { :; } +msg_ok() { :; } +pmx_install_pkg() { :; } +_pmx_log2ram_install_release() { :; } +_pmx_log2ram_pbs_logrotate() { :; } +_pmx_log2ram_remove_legacy() { :; } +pmx_edit_file() { local file="$1"; shift; sed -i "$@" "$file"; } +pmx_append_file() { cat >> "$1"; } +pmx_write_file() { cat > "$1"; } +pmx_remove_file() { rm -f "$1"; } +pmx_apply_setting() { :; } +systemctl() { printf '%s\n' "$*" >> "$SYSTEMCTL_TRACE"; } +''' + if edit_failure: + # A failed configuration writer must be handled even by root runners. + stubs += "\npmx_edit_file() { return 1; }\n" + script = (stubs + "\npmx_log2ram_setup() {" + body + "\n}\n" + + "pmx_log2ram_setup 256 3 false\n") + env = dict(os.environ, SYSTEMCTL_TRACE=str(self.root / "systemctl.trace"), + PMX_L2R_TAG="1.7.2", PMX_L2R_SCRIPTS=str(KEEP.parent), + PMX_L2R_REVERT_UNIT=str(self.root / "etc/revert.service")) + return subprocess.run(["bash", "-c", script], env=env, capture_output=True, text=True, timeout=10) + + def test_setup_keeps_journal_aware_syncing_until_boot(self): + self.config.write_text("SIZE=128M\nJOURNALD_AWARE=false\nNOTIFICATION=true\n") + result = self.run_setup() + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("JOURNALD_AWARE=true\n", self.config.read_text()) + self.assertIn("SIZE=256M\n", self.config.read_text()) + self.assertFalse((self.log / "journal").is_symlink()) + self.assertEqual((self.root / "systemctl.trace").read_text(), "daemon-reload\n") + + def test_boot_disables_journal_aware_syncing_after_relocation(self): + self.assertEqual(self.run_setup().returncode, 0) + result = self.run_script() + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(os.readlink(self.log / "journal"), "/var/hdd.journal") + self.assertEqual((self.root / "var/hdd.journal/abc/system.journal").read_text(), "entries") + self.assertIn("JOURNALD_AWARE=false\n", self.config.read_text()) + self.assertIn("SIZE=256M\n", self.config.read_text()) + # Reapplying the repair to an already-relocated installation is safe. + self.assertEqual(self.run_setup().returncode, 0) + self.assertEqual(self.run_script().returncode, 0) + self.assertIn("JOURNALD_AWARE=false\n", self.config.read_text()) + self.assertEqual(sorted(p.name for p in (self.root / "var/hdd.journal").iterdir()), ["abc"]) + + def test_boot_migration_failure_is_not_ignored_by_startup(self): + self.assertEqual(self.run_setup().returncode, 0) + # A non-directory destination causes a real copy failure, without mounts. + conflict = self.root / "var/hdd.journal" + conflict.write_text("blocked") + unit = self.root / "etc/systemd/system/log2ram.service.d/proxmenux.conf" + command = next(line.split("=", 1)[1] for line in unit.read_text().splitlines() + if line.startswith("ExecStartPre=")) + result = subprocess.run(["bash", *shlex.split(command.lstrip("-"))], + env=dict(os.environ, PMX_L2R_ROOT=str(self.root)), + capture_output=True, text=True, timeout=10) + self.assertNotEqual(result.returncode, 0, result.stderr) + self.assertIn("JOURNALD_AWARE=true\n", self.config.read_text()) + self.assertFalse(command.startswith("-"), "systemd must not ignore the helper's failure") + conflict.unlink() + self.assertEqual(self.run_script().returncode, 0) + self.assertIn("JOURNALD_AWARE=false\n", self.config.read_text()) + + def test_setup_adds_journal_aware_setting_when_missing(self): + self.config.write_text("SIZE=128M\n#JOURNALD_AWARE=false\n") + result = self.run_setup() + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("\nJOURNALD_AWARE=true\n", self.config.read_text()) + self.assertEqual(self.run_script().returncode, 0) + self.assertIn("\nJOURNALD_AWARE=false\n", self.config.read_text()) + + def check_missing_setting_without_final_newline(self, content): + self.config.write_text(content) + result = self.run_setup() + self.assertEqual(result.returncode, 0, result.stderr) + for expected in ("true", "false"): + with self.subTest(journal_aware=expected): + # Read shell semantics in a fresh Bash, not a substring match. + sourced = subprocess.run( + ["bash", "--noprofile", "--norc", "-c", + 'unset JOURNALD_AWARE NOTIFICATION; source "$1"; ' + 'printf "%s\\n%s\\n" "${JOURNALD_AWARE-UNSET}" "${NOTIFICATION-UNSET}"', + "fixture", str(self.config)], + capture_output=True, text=True, timeout=10) + self.assertEqual(sourced.returncode, 0, sourced.stderr) + self.assertEqual(sourced.stdout, expected + "\nfalse\n") + + if expected == "true": + self.assertFalse((self.log / "journal").is_symlink()) + self.assertEqual((self.log / "journal/abc/system.journal").read_text(), "entries") + result = self.run_script() + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(os.readlink(self.log / "journal"), "/var/hdd.journal") + self.assertEqual((self.root / "var/hdd.journal/abc/system.journal").read_text(), "entries") + + def test_missing_setting_after_comment_without_final_newline(self): + self.check_missing_setting_without_final_newline( + "SIZE=128M\nNOTIFICATION=false\n#JOURNALD_AWARE=false") + + def test_missing_setting_after_assignment_without_final_newline(self): + self.check_missing_setting_without_final_newline("SIZE=128M\nNOTIFICATION=false") + + def test_setup_reports_configuration_edit_failure(self): + self.config.write_text("SIZE=128M\nJOURNALD_AWARE=false\nNOTIFICATION=false\n") + result = self.run_setup(edit_failure=True) + self.assertNotEqual(result.returncode, 0, result.stderr) + self.assertIn("JOURNALD_AWARE=false\n", self.config.read_text()) + + def test_an_existing_link_with_an_unusable_disk_target_fails(self): + self.assertEqual(self.run_script().returncode, 0) + self.config.write_text("JOURNALD_AWARE=true\n") + disk = self.root / "var/hdd.journal" + shutil.rmtree(disk) + disk.write_text("blocked") + result = self.run_script() + self.assertNotEqual(result.returncode, 0, result.stderr) + self.assertEqual(self.config.read_text(), "JOURNALD_AWARE=true\n") + + def test_an_unexpected_journal_link_does_not_complete_migration(self): + shutil.rmtree(self.log / "journal") + (self.log / "journal").symlink_to("pveproxy") + result = self.run_script() + self.assertNotEqual(result.returncode, 0, result.stderr) + self.assertEqual(os.readlink(self.log / "journal"), "pveproxy") + self.assertIn("JOURNALD_AWARE=true\n", self.config.read_text()) + + def test_a_missing_journal_with_an_unusable_disk_target_fails(self): + shutil.rmtree(self.log / "journal") + (self.root / "var/hdd.journal").write_text("blocked") + result = self.run_script() + self.assertNotEqual(result.returncode, 0, result.stderr) + self.assertIn("JOURNALD_AWARE=true\n", self.config.read_text()) + self.assertFalse((self.log / "journal").is_symlink()) + + def test_failed_source_removal_does_not_complete_migration(self): + (self.root / "var/hdd.journal").mkdir() + result = subprocess.run( + ["bash", "-c", 'rm() { return 1; }; export -f rm; exec bash "$1"', "fixture", str(KEEP)], + env=dict(os.environ, PMX_L2R_ROOT=str(self.root)), capture_output=True, text=True, timeout=10) + self.assertNotEqual(result.returncode, 0, result.stderr) + self.assertIn("JOURNALD_AWARE=true\n", self.config.read_text()) + self.assertTrue((self.log / "journal/abc/system.journal").is_file()) + + def test_boot_configuration_failure_is_reported_and_can_be_retried(self): + self.config.unlink() + self.config.mkdir() + result = self.run_script() + self.assertNotEqual(result.returncode, 0, result.stderr) + self.assertEqual(os.readlink(self.log / "journal"), "/var/hdd.journal") + self.config.rmdir() + self.config.write_text("JOURNALD_AWARE=true\n") + self.assertEqual(self.run_script().returncode, 0) + self.assertEqual(self.config.read_text(), "JOURNALD_AWARE=false\n") def run_script(self, revert=False): env = dict(os.environ, PMX_L2R_ROOT=str(self.root), PMX_L2R_REVERT="1" if revert else "0") - return subprocess.run(["bash", str(KEEP)], env=env, capture_output=True, text=True) + return subprocess.run(["bash", str(KEEP)], env=env, capture_output=True, text=True, timeout=10) def test_the_journal_moves_to_disk_behind_a_link(self): self.assertEqual(self.run_script().returncode, 0) @@ -78,6 +245,7 @@ class KeepOnDiskTests(unittest.TestCase): def test_removing_log2ram_puts_the_folders_back(self): (self.log / "proxmox-backup").mkdir() self.run_script() + self.config.unlink() # Revert must not require or rewrite Log2RAM settings. self.assertEqual(self.run_script(revert=True).returncode, 0) for name in ("journal", "proxmox-backup"): self.assertFalse((self.log / name).is_symlink(), name) @@ -99,7 +267,7 @@ class KeptSettingsTests(unittest.TestCase): library = Path(folder) / "setup.sh" library.write_text(script) result = subprocess.run(["bash", "-c", f'source "{library}"; {function}'], - capture_output=True, text=True) + capture_output=True, text=True, timeout=10) return result.stdout.strip() def test_the_size_comes_from_the_installed_configuration(self): @@ -114,6 +282,24 @@ class KeptSettingsTests(unittest.TestCase): cron = "MAILTO=\"\"\n0 */3 * * * root /usr/local/bin/log2ram write >/dev/null 2>&1\n" self.assertEqual(self.read("pmx_log2ram_current_hours", {"/etc/cron.d/log2ram": cron}), "3") + def test_both_modes_offer_a_repair_to_version_2_0_installs(self): + for filename, function, updater in ( + ("auto_post_install.sh", "install_log2ram_auto", "_update_existing_log2ram_auto"), + ("customizable_post_install.sh", "configure_log2ram", "_update_existing_log2ram_custom"), + ): + with self.subTest(mode=filename), tempfile.TemporaryDirectory() as folder: + config = Path(folder) / "log2ram.conf" + config.write_text("JOURNALD_AWARE=false\n") + source = (ROOT / "scripts/post_install" / filename).read_text() + body = source.split(function + "() {", 1)[1].split("\n}\n", 1)[0] + body = body.replace("/etc/log2ram.conf", str(config)) + script = ("pmx_journal_context() { :; }\n" + + updater + "() { printf '%s\\n' \"$1\"; }\n" + + function + "() {" + body + "\n}\n" + function) + result = subprocess.run(["bash", "-c", script], capture_output=True, text=True, timeout=10) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), "2.1") + def test_the_release_is_pinned_to_a_verified_commit(self): text = SETUP.read_text() self.assertIn('PMX_L2R_TAG="1.7.2"', text) diff --git a/scripts/global/log2ram_keep_on_disk.sh b/scripts/global/log2ram_keep_on_disk.sh index bbffe243..3e183e74 100755 --- a/scripts/global/log2ram_keep_on_disk.sh +++ b/scripts/global/log2ram_keep_on_disk.sh @@ -30,20 +30,21 @@ keep_on_disk() { local place="$LOG_DIR/$name" disk="$ROOT/var/hdd.$name" if [ -L "$place" ]; then - [ -d "$disk" ] || mkdir -p "$disk" + [ "$(readlink "$place")" = "/var/hdd.$name" ] || return 1 + [ -d "$disk" ] || mkdir -p "$disk" || return 1 return 0 fi # The journal is always kept on disk; any other folder only where it exists. if [ ! -d "$place" ]; then [ "$name" = journal ] || return 0 - mkdir -p "$disk" + mkdir -p "$disk" || return 1 chown root:systemd-journal "$disk" 2>/dev/null || true chmod 2755 "$disk" elif [ ! -e "$disk" ]; then mv "$place" "$disk" || return 1 else cp -a "$place/." "$disk/" || return 1 - rm -rf "${place:?}" + rm -rf "${place:?}" || return 1 fi ln -s "/var/hdd.$name" "$place" } @@ -70,4 +71,8 @@ for name in $KEPT_ON_DISK; do keep_on_disk "$name" || status=1 fi done +# Only a completed boot migration makes it safe to skip journal rotation. +if [ "$status" = 0 ] && [ "${PMX_L2R_REVERT:-0}" != 1 ]; then + sed -i 's/^JOURNALD_AWARE=.*/JOURNALD_AWARE=false/' "$ROOT/etc/log2ram.conf" || status=1 +fi exit "$status" diff --git a/scripts/global/log2ram_setup.sh b/scripts/global/log2ram_setup.sh index 02f5abff..fdb7a485 100644 --- a/scripts/global/log2ram_setup.sh +++ b/scripts/global/log2ram_setup.sh @@ -112,19 +112,22 @@ pmx_log2ram_setup() { _pmx_log2ram_install_release || return 1 msg_ok "$(translate "Log2RAM installed:") ${PMX_L2R_TAG}" - # The journal is not in RAM, so Log2RAM does not rotate it before a sync; - # and Proxmox has no mail command for its error notice. + # Keep journal-aware syncing through shutdown: the running journal may + # still be in RAM. The boot helper disables it only after relocation. + # Proxmox has no mail command for the error notice. pmx_edit_file /etc/log2ram.conf \ -e "s/^SIZE=.*/SIZE=${size_mb}M/" \ - -e "s/^JOURNALD_AWARE=.*/JOURNALD_AWARE=false/" \ - -e "s/^#\?NOTIFICATION=.*/NOTIFICATION=false/" + -e "s/^JOURNALD_AWARE=.*/JOURNALD_AWARE=true/" \ + -e "s/^#\?NOTIFICATION=.*/NOTIFICATION=false/" || return 1 + grep -q '^JOURNALD_AWARE=' /etc/log2ram.conf || \ + printf '\nJOURNALD_AWARE=true\n' | pmx_append_file /etc/log2ram.conf || return 1 grep -q '^NOTIFICATION=' /etc/log2ram.conf || printf 'NOTIFICATION=false\n' | pmx_append_file /etc/log2ram.conf mkdir -p /etc/systemd/system/log2ram.service.d /etc/systemd/system/log2ram-daily.timer.d pmx_write_file /etc/systemd/system/log2ram.service.d/proxmenux.conf <