mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-25 18:40:06 -04:00
A library on the internal disk gets a database
"Run repair without external drive" kept the library on the internal disk, but the API's unmounted-media guard refuses to create or use a database beneath /media/wrolpi when it is not a mount. After onboarding without a drive the API restarted forever with no database, and downloads were blocked as "media not mounted". The Controller now records the choice as drives.library_on_internal_disk in controller.yaml -- the only source of it. The database guard and the media_mounted check both read that marker, so require_media_mounted keeps its meaning and needs no change. Onboarding a drive later clears the marker, so it is never copied onto the drive. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
559bfebc56
commit
3acb7439a4
8 changed files with 119 additions and 10 deletions
|
|
@ -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": [
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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'}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue