diff --git a/controller/defaults.py b/controller/defaults.py index 616e7542..530b8529 100644 --- a/controller/defaults.py +++ b/controller/defaults.py @@ -14,6 +14,9 @@ DEFAULT_CONFIG = { "supported_filesystems": ["ext4", "btrfs", "vfat", "exfat"], "auto_mount": True, "mounts": [], # Populated from controller.yaml after drive mount + # True when the user chose "Run repair without external drive"; the + # API then accepts an unmounted /media/wrolpi as the library. + "library_on_internal_disk": False, }, "managed_services": [ diff --git a/controller/lib/onboarding.py b/controller/lib/onboarding.py index be82bf7c..5b4a3657 100644 --- a/controller/lib/onboarding.py +++ b/controller/lib/onboarding.py @@ -176,13 +176,15 @@ def onboard_without_drive() -> dict: Set up WROLPi with the library on the internal disk, then start repair. controller.yaml marks the primary drive as set up; without it the wizard - would be shown again after repair. + would be shown again after repair. Its drives.library_on_internal_disk + tells the API that /media/wrolpi is deliberately not a mount. Returns: dict with success, mounts, and repair_started """ config_dir = get_media_directory() / "config" config_dir.mkdir(parents=True, exist_ok=True) + update_config("drives.library_on_internal_disk", True) save_config() repair_result = start_script("repair") @@ -282,8 +284,11 @@ def commit_onboarding( if not fstab_result.get("success"): logger.warning("Failed to add primary fstab entry: %s", fstab_result.get("error")) - # Step 5: Reload config from drive and ensure controller.yaml exists + # Step 5: Reload config from drive and ensure controller.yaml exists. + # The library now lives on this drive, even if this process earlier + # onboarded without one. reload_config_from_drive() + update_config("drives.library_on_internal_disk", False) try: save_config() except RuntimeError: diff --git a/controller/test/test_onboarding.py b/controller/test/test_onboarding.py index 745932a0..93136ad1 100644 --- a/controller/test/test_onboarding.py +++ b/controller/test/test_onboarding.py @@ -3,12 +3,14 @@ Unit tests for onboarding orchestration logic. """ import contextlib +import copy from pathlib import Path from unittest import mock import pytest import yaml +from controller.defaults import DEFAULT_CONFIG from controller.lib.onboarding import ( TEMP_MOUNT_PATH, cancel_probe, @@ -361,12 +363,13 @@ class TestCommitOnboarding: mock.patch("controller.lib.onboarding.start_script", return_value={"success": True}): commit_onboarding("/dev/sda1", "ext4") - mock_update.assert_called_once_with("drives.mounts", [{ + mounts_updates = [c for c in mock_update.call_args_list if c.args[0] == "drives.mounts"] + assert mounts_updates == [mock.call("drives.mounts", [{ "device": "UUID=abc-123", "mount_point": str(media_dir), "fstype": "ext4", "options": "defaults", - }]) + }])] def test_does_not_overwrite_existing_mounts(self, mock_docker_mode, tmp_path): """Should not modify drives.mounts when config already has mounts.""" @@ -384,7 +387,7 @@ class TestCommitOnboarding: mock.patch("controller.lib.onboarding.start_script", return_value={"success": True}): commit_onboarding("/dev/sda1", "ext4") - mock_update.assert_not_called() + assert not [c for c in mock_update.call_args_list if c.args[0] == "drives.mounts"] def test_mount_failure(self, mock_docker_mode, tmp_path): """Should return error when primary mount fails.""" @@ -563,13 +566,39 @@ class TestOnboardWithoutDrive: with mock.patch("controller.lib.onboarding.get_media_directory", return_value=media_dir), \ mock.patch("controller.lib.config.CONFIG_PATH_ON_DRIVE", config_path), \ + mock.patch("controller.lib.config._runtime_config", copy.deepcopy(DEFAULT_CONFIG)), \ mock.patch("controller.lib.onboarding.start_script", return_value={"success": True}) as start: result = onboard_without_drive() assert result == {"success": True, "mounts": [], "repair_started": True} - assert config_path.is_file() + # The API reads this marker; without it, it refuses to create a database on an unmounted /media/wrolpi. + assert yaml.safe_load(config_path.read_text()) == {"drives": {"library_on_internal_disk": True}} start.assert_called_once_with("repair") + def test_onboarding_a_drive_later_does_not_copy_the_marker(self, mock_docker_mode, tmp_path): + """The marker describes the internal disk; a drive onboarded afterward (same Controller process) must + not inherit it, or the API would accept an unmounted drive's directory as the library.""" + media_dir = tmp_path / "wrolpi" + media_dir.mkdir() + config_path = media_dir / "config" / "controller.yaml" + + with mock.patch("controller.lib.onboarding.get_media_directory", return_value=media_dir), \ + mock.patch("controller.lib.config.CONFIG_PATH_ON_DRIVE", config_path), \ + mock.patch("controller.lib.config._runtime_config", copy.deepcopy(DEFAULT_CONFIG)), \ + mock.patch("controller.lib.onboarding.start_script", return_value={"success": True}): + onboard_without_drive() + # The drive is mounted over the internal disk's directory; it has no controller.yaml yet. + config_path.unlink() + with mock.patch("controller.lib.onboarding._cleanup_temp_mount"), \ + mock.patch("controller.lib.onboarding.mount_drive", return_value={"success": True}), \ + mock.patch("controller.lib.onboarding.add_fstab_entry", return_value={"success": False}): + # The internal disk's library is shadowed by the drive; the user confirmed. + result = commit_onboarding("/dev/sda1", "ext4", force_shadowed=True) + + assert result["success"] is True + drive_config = yaml.safe_load(config_path.read_text()) or {} + assert drive_config.get("drives", {}).get("library_on_internal_disk") is not True + def test_repair_failure_is_reported(self, mock_docker_mode, tmp_path): media_dir = tmp_path / "wrolpi" media_dir.mkdir() diff --git a/wrolpi/common.py b/wrolpi/common.py index e722d4a4..feb4917f 100644 --- a/wrolpi/common.py +++ b/wrolpi/common.py @@ -473,7 +473,24 @@ def get_media_directory() -> Path: return MEDIA_DIRECTORY -DB_CONFIG_FILE_NAMES = {'tags.yaml', 'channels.yaml', 'domains.yaml', 'download_manager.yaml', +def library_on_internal_disk(media_directory: Path = None) -> bool: + """True when the user chose to keep the library on the internal disk (the Controller's "Run repair without + external drive"), so /media/wrolpi need not be a mount. + + The Controller's config/controller.yaml is the only source of this choice. It is read from the media + directory itself: when a drive fails to mount, the root filesystem beneath has no marker (unless the + library really lives there).""" + media_directory = media_directory or get_media_directory() + try: + with (media_directory / 'config' / 'controller.yaml').open() as fh: + config = yaml.safe_load(fh) + except (OSError, yaml.YAMLError): + return False + drives = config.get('drives') if isinstance(config, dict) else None + return isinstance(drives, dict) and drives.get('library_on_internal_disk') is True + + +DB_CONFIG_FILE_NAMES ={'tags.yaml', 'channels.yaml', 'domains.yaml', 'download_manager.yaml', 'playlists.yaml', 'repos.yaml'} diff --git a/wrolpi/db_bootstrap.py b/wrolpi/db_bootstrap.py index 076d0e1f..056a5c2e 100644 --- a/wrolpi/db_bootstrap.py +++ b/wrolpi/db_bootstrap.py @@ -89,6 +89,10 @@ def media_directory_is_unmounted_production(media_directory: Path = None) -> boo if not str(media_directory).startswith('/media/'): # A custom MEDIA_DIRECTORY (dev boxes) may legitimately not be a mount. return False + from wrolpi.common import library_on_internal_disk + if library_on_internal_disk(media_directory): + # The user chose to keep the library on the internal disk; there is no drive to wait for. + return False try: return not media_directory.is_mount() except OSError: diff --git a/wrolpi/flags.py b/wrolpi/flags.py index 049a288f..139970a7 100644 --- a/wrolpi/flags.py +++ b/wrolpi/flags.py @@ -7,7 +7,7 @@ from pathlib import Path from sqlalchemy import Column, Boolean, Integer -from wrolpi.common import logger, Base, get_media_directory, get_wrolpi_config +from wrolpi.common import logger, Base, get_media_directory, get_wrolpi_config, library_on_internal_disk from wrolpi.vars import DOCKERIZED __all__ = [ @@ -210,9 +210,10 @@ def _destinations_have_mounted_storage() -> bool: silently write to the SD card on a Raspberry Pi. Docker mode is exempt: the media directory is always a bind mount and - the user cannot remount drives from inside the container. + the user cannot remount drives from inside the container. So is a + library the user chose to keep on the internal disk. """ - if DOCKERIZED: + if DOCKERIZED or library_on_internal_disk(): return True media_dir = get_media_directory().resolve() diff --git a/wrolpi/test/test_db_bootstrap.py b/wrolpi/test/test_db_bootstrap.py index fad811ba..4eb18b07 100644 --- a/wrolpi/test/test_db_bootstrap.py +++ b/wrolpi/test/test_db_bootstrap.py @@ -410,6 +410,8 @@ def test_unmounted_production_guard(test_directory): fake_media = mock.MagicMock(spec=pathlib.Path) fake_media.__str__ = lambda self: '/media/wrolpi' + # Files beneath it (config/controller.yaml) come from the real, marker-less test directory. + fake_media.__truediv__ = lambda self, other: test_directory / other fake_media.is_mount.return_value = False fake_media.is_dir.return_value = True @@ -431,3 +433,43 @@ def test_unmounted_production_guard(test_directory): assert db_bootstrap.ensure_db() is True # create a real DB in the test directory first with mock.patch.object(db_bootstrap, 'media_directory_is_unmounted_production', return_value=True): assert db_bootstrap.ensure_db() is False + + +def test_unmounted_production_guard_library_on_internal_disk(test_directory): + """A library the user chose to keep on the internal disk ("Run repair without external drive") is not an + unmounted drive: the Controller's marker in config/controller.yaml lifts the guard.""" + import pathlib + + config_dir = test_directory / 'config' + config_dir.mkdir(exist_ok=True) + fake_media = mock.MagicMock(spec=pathlib.Path) + fake_media.__str__ = lambda self: '/media/wrolpi' + fake_media.__truediv__ = lambda self, other: test_directory / other + fake_media.is_mount.return_value = False + + with mock.patch.object(db_bootstrap, 'PYTEST', False), \ + mock.patch.object(db_bootstrap, 'DOCKERIZED', False), \ + mock.patch.object(db_bootstrap.sys, 'platform', 'linux'): + # A controller.yaml without the marker (a drive that failed to mount) stays guarded. + (config_dir / 'controller.yaml').write_text('{}\n') + assert db_bootstrap.media_directory_is_unmounted_production(fake_media) is True + + (config_dir / 'controller.yaml').write_text('drives:\n library_on_internal_disk: true\n') + assert db_bootstrap.media_directory_is_unmounted_production(fake_media) is False + + +@pytest.mark.parametrize('contents,expected', [ + (None, False), + ('{}\n', False), + ('drives:\n library_on_internal_disk: false\n', False), + ('drives:\n library_on_internal_disk: true\n', True), + ('drives: [not, a, dict]\n', False), + ('drives: {library_on_internal_disk: true\n', False), # Malformed YAML. +]) +def test_library_on_internal_disk(test_directory, contents, expected): + from wrolpi.common import library_on_internal_disk + + if contents is not None: + (test_directory / 'config').mkdir(exist_ok=True) + (test_directory / 'config' / 'controller.yaml').write_text(contents) + assert library_on_internal_disk() is expected diff --git a/wrolpi/test/test_flags.py b/wrolpi/test/test_flags.py index 33a52c74..faaf2233 100644 --- a/wrolpi/test/test_flags.py +++ b/wrolpi/test/test_flags.py @@ -85,6 +85,14 @@ def test_destinations_have_mounted_storage_nothing_mounted(test_directory, test_ assert flags._destinations_have_mounted_storage() is False +def test_destinations_have_mounted_storage_library_on_internal_disk(test_directory, test_wrolpi_config): + """Nothing is mounted, but the user chose to keep the library on the internal disk, so downloads may use it.""" + (test_directory / 'config').mkdir(exist_ok=True) + (test_directory / 'config' / 'controller.yaml').write_text('drives:\n library_on_internal_disk: true\n') + with _patch_mounts(set(), test_directory): + assert flags._destinations_have_mounted_storage() is True + + def test_destinations_have_mounted_storage_partial_coverage(async_client, test_directory, test_wrolpi_config): """If videos is on a drive but archive/zims/map are not, return False.