From 7e00904dceb43a06e34448e8f4fa9c810b71ccf1 Mon Sep 17 00:00:00 2001 From: VAIO73 <50487331+Vaso73@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:59:46 +0000 Subject: [PATCH 1/4] fix(oci): align JDownloader and Arr suite web links --- .github/workflows/test-oci-registries.yml | 4 + oci/catalog/apps/jdownloader.json | 2 +- oci/catalog/apps/suite-arr.json | 10 ++- oci/catalog/curated/jdownloader.json | 2 +- oci/catalog/curated/suite-arr.json | 10 ++- oci/tests/test_catalog_web_links.py | 102 ++++++++++++++++++++++ 6 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 oci/tests/test_catalog_web_links.py diff --git a/.github/workflows/test-oci-registries.yml b/.github/workflows/test-oci-registries.yml index a4f0b150..558af6aa 100644 --- a/.github/workflows/test-oci-registries.yml +++ b/.github/workflows/test-oci-registries.yml @@ -80,3 +80,7 @@ jobs: python3 -m unittest discover -s oci/tests -p 'test_instance_reconcile.py' -v python3 -m unittest discover -s oci/tests -p 'test_guessed_endpoint_healthcheck.py' -v python3 -m unittest discover -s oci/tests -p 'test_runtime_home_*.py' -v + - name: Run catalog web-link regressions + env: + PYTHONDONTWRITEBYTECODE: '1' + run: python3 -m unittest discover -s oci/tests -p test_catalog_web_links.py -v diff --git a/oci/catalog/apps/jdownloader.json b/oci/catalog/apps/jdownloader.json index 1870658f..9d526744 100644 --- a/oci/catalog/apps/jdownloader.json +++ b/oci/catalog/apps/jdownloader.json @@ -24,7 +24,7 @@ "amd64" ], "launch": { - "scheme": "http", + "scheme": "https", "port": 5800, "path": "/" }, diff --git a/oci/catalog/apps/suite-arr.json b/oci/catalog/apps/suite-arr.json index d3de36f8..ae5426e5 100644 --- a/oci/catalog/apps/suite-arr.json +++ b/oci/catalog/apps/suite-arr.json @@ -135,7 +135,15 @@ "original_compose": "---\nservices:\n prowlarr:\n image: lscr.io/linuxserver/prowlarr:latest\n container_name: prowlarr\n environment:\n - PUID=1000\n - PGID=1000\n - TZ=Etc/UTC\n volumes:\n - /path/to/prowlarr/data:/config\n ports:\n - 9696:9696\n restart: unless-stopped\n" }, "first_run": { - "endpoints": [], + "endpoints": [ + { + "label": "Web UI", + "scheme": "http", + "port": 9696, + "path": "/", + "source": "upstream-linuxserver-readme" + } + ], "credentials": [] }, "proxmox": { diff --git a/oci/catalog/curated/jdownloader.json b/oci/catalog/curated/jdownloader.json index e040a7bd..4db2d1b3 100644 --- a/oci/catalog/curated/jdownloader.json +++ b/oci/catalog/curated/jdownloader.json @@ -24,7 +24,7 @@ "amd64" ], "launch": { - "scheme": "http", + "scheme": "https", "port": 5800, "path": "/" }, diff --git a/oci/catalog/curated/suite-arr.json b/oci/catalog/curated/suite-arr.json index a89c8ef2..a08e35b0 100644 --- a/oci/catalog/curated/suite-arr.json +++ b/oci/catalog/curated/suite-arr.json @@ -135,7 +135,15 @@ "original_compose": "---\nservices:\n prowlarr:\n image: lscr.io/linuxserver/prowlarr:latest\n container_name: prowlarr\n environment:\n - PUID=1000\n - PGID=1000\n - TZ=Etc/UTC\n volumes:\n - /path/to/prowlarr/data:/config\n ports:\n - 9696:9696\n restart: unless-stopped\n" }, "first_run": { - "endpoints": [], + "endpoints": [ + { + "label": "Web UI", + "scheme": "http", + "port": 9696, + "path": "/", + "source": "upstream-linuxserver-readme" + } + ], "credentials": [] }, "proxmox": { diff --git a/oci/tests/test_catalog_web_links.py b/oci/tests/test_catalog_web_links.py new file mode 100644 index 00000000..324d7a05 --- /dev/null +++ b/oci/tests/test_catalog_web_links.py @@ -0,0 +1,102 @@ +"""Catalog links agree with the configured service and preserve suite choices.""" + +import copy +import json +from pathlib import Path +import sys +import unittest +from unittest.mock import patch + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "src")) +sys.path.insert(0, str(ROOT / "remote")) +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from proxmenux_oci.catalog import Catalog +from proxmenux_oci.installer import DEFAULT_MODE, build_deployment +from proxmenux_oci.stack import StackError +from test_advanced_flow_order import RecordingUI, addresses, storages +import oci_description +import oci_instance_transaction as transaction + + +class CatalogWebLinks(unittest.TestCase): + catalog = Catalog(ROOT) + + def test_jdownloader_launch_matches_its_secure_web_service(self): + template = self.catalog.compose("jdownloader") + environment = {entry["name"]: entry.get("example") + for entry in template["container_contract"]["environment"]} + self.assertEqual(environment["SECURE_CONNECTION"], "1") + launch = template["catalog_ui"]["launch"] + self.assertEqual(launch, {"scheme": "https", "port": 5800, "path": "/"}) + endpoint = template["first_run"]["endpoints"][0] + self.assertEqual({key: endpoint[key] for key in launch}, launch) + self.assertIn('href="https://192.0.2.10:5800/"', + oci_description.render(template, "", "test-installation", "192.0.2.10")) + + def test_shipped_templates_keep_the_curated_link_contract(self): + for app in ("jdownloader", "suite-arr"): + with self.subTest(app=app): + curated = json.loads((ROOT / "catalog/curated" / (app + ".json")).read_text()) + generated = json.loads((ROOT / "catalog/apps" / (app + ".json")).read_text()) + self.assertEqual(curated["catalog_ui"]["launch"], generated["catalog_ui"]["launch"]) + self.assertEqual(curated["first_run"], generated["first_run"]) + + def test_suite_parent_names_the_documented_prowlarr_web_endpoint(self): + template = self.catalog.compose("suite-arr") + endpoints = template["first_run"]["endpoints"] + self.assertEqual(len(endpoints), 1) + endpoint = endpoints[0] + self.assertEqual((endpoint["scheme"], endpoint["port"], endpoint["path"]), + ("http", 9696, "/")) + self.assertEqual(endpoint["label"], "Web UI") + self.assertNotEqual(endpoint["source"], transaction.GUESSED_ENDPOINT) + check = transaction.effective_healthcheck(template) + self.assertEqual((check["scheme"], check["port"]), ("http", 9696)) + self.assertNotIn("accept_non_http", check) + + @patch("proxmenux_oci.installer.host.gpus", return_value={"intel": [], "amd": [], "nvidia": False}) + @patch("proxmenux_oci.i18n.language", return_value="en") + @patch("proxmenux_oci.installer.host.storages", side_effect=storages) + @patch("proxmenux_oci.installer.host.bridges", return_value=[{"iface": "vmbr0", "cidr": "192.0.2.1/24"}]) + @patch("proxmenux_oci.installer.host.timezone", return_value="Etc/UTC") + @patch("proxmenux_oci.installer.access.ask_ipv4", return_value=("dhcp", None)) + @patch("proxmenux_oci.installer.access.ask_addresses", side_effect=addresses) + def test_suite_members_keep_their_own_links_with_or_without_prowlarr(self, *_): + template = self.catalog.compose("suite-arr") + before = copy.deepcopy(template) + with patch("subprocess.run") as command: + for selected in (["prowlarr", "sonarr"], ["sonarr"]): + ui = RecordingUI({"Arr suite: applications to install": selected, + "Media server": "none"}) + plan = build_deployment(copy.deepcopy(template), ui, DEFAULT_MODE) + self.assertEqual([member["name"] for member in plan["services"]], selected) + for member in plan["services"]: + port = 9696 if member["name"] == "prowlarr" else 8989 + endpoint = member["template"]["first_run"]["endpoints"][0] + self.assertEqual(endpoint["port"], port) + self.assertEqual(member["healthcheck"]["endpoint"]["port"], port) + notes = oci_description.render(member["template"], "", "test-installation", "192.0.2.10") + self.assertIn(f':{port}/"', notes) + if port != 9696: + self.assertNotIn(':9696/"', notes) + command.assert_not_called() + self.assertEqual(template, before) + + def test_empty_suite_selection_has_no_host_or_runtime_effect(self): + template = self.catalog.compose("suite-arr") + before = copy.deepcopy(template) + ui = RecordingUI({"Arr suite: applications to install": [], "Media server": "none"}) + with patch("proxmenux_oci.i18n.language", return_value="en"), \ + patch("proxmenux_oci.installer.host.storages") as host, \ + patch("subprocess.run") as command: + with self.assertRaises(StackError): + build_deployment(template, ui, DEFAULT_MODE) + host.assert_not_called() + command.assert_not_called() + self.assertEqual(template, before) + + +if __name__ == "__main__": + unittest.main() From 3e9c9eb25da8340bba5ed394fbd6058095d7d28b Mon Sep 17 00:00:00 2001 From: VAIO73 <50487331+Vaso73@users.noreply.github.com> Date: Mon, 19 Oct 2026 12:23:49 +0000 Subject: [PATCH 2/4] fix(oci): prepare managed format directories before startup --- .github/workflows/test-oci-registries.yml | 4 + oci/remote/install_oci.sh | 17 +- oci/remote/oci_console_mark.sh | 7 + oci/remote/oci_instance_transaction.py | 25 +- oci/remote/oci_managed_volumes.py | 183 +++++++++++ oci/remote/oci_restore_recovery.py | 15 +- oci/src/proxmenux_oci/installer.py | 4 +- oci/tests/test_managed_volume_installer.py | 162 ++++++++++ oci/tests/test_managed_volume_startup.py | 301 ++++++++++++++++++ oci/tests/test_readonly_volume_preparation.py | 10 +- 10 files changed, 694 insertions(+), 34 deletions(-) create mode 100644 oci/remote/oci_managed_volumes.py create mode 100644 oci/tests/test_managed_volume_installer.py create mode 100644 oci/tests/test_managed_volume_startup.py diff --git a/.github/workflows/test-oci-registries.yml b/.github/workflows/test-oci-registries.yml index a4f0b150..6c67aa1d 100644 --- a/.github/workflows/test-oci-registries.yml +++ b/.github/workflows/test-oci-registries.yml @@ -80,3 +80,7 @@ jobs: python3 -m unittest discover -s oci/tests -p 'test_instance_reconcile.py' -v python3 -m unittest discover -s oci/tests -p 'test_guessed_endpoint_healthcheck.py' -v python3 -m unittest discover -s oci/tests -p 'test_runtime_home_*.py' -v + - name: Run managed volume startup regressions + env: + PYTHONDONTWRITEBYTECODE: '1' + run: python3 -m unittest discover -s oci/tests -p 'test_managed_volume_*.py' -v diff --git a/oci/remote/install_oci.sh b/oci/remote/install_oci.sh index b3b100e6..0251776e 100755 --- a/oci/remote/install_oci.sh +++ b/oci/remote/install_oci.sh @@ -1250,7 +1250,8 @@ apply_installer_profile() { continue fi if [[ $remove_lost_found == true ]]; then - rm -rf "${rootfs}${target}/lost+found" || { failed=1; break; } + oci_quiet python3 "${SCRIPT_DIR}/oci_managed_volumes.py" prepare \ + "$rootfs" "$DEPLOYMENT_FILE" --target "$target" || { failed=1; break; } fi case "$owner_strategy" in mapped-root) chown "$HOST_ROOT_UID:$HOST_ROOT_GID" "${rootfs}${target}" || failed=1 ;; @@ -2058,6 +2059,20 @@ while IFS=$'\t' read -r NAME ENCODED; do set_runtime_environment "$NAME" "$VALUE" done < <(jq -r '.environment[] | [.name, (.value | @base64)] | @tsv' "$DEPLOYMENT_FILE") +prepare_managed_format_directories() { + local rootfs=$1 preparation_failed=0 + # Stack members receive their real mounts later in attach_mounts(). + [[ $(jq -r '.stack_managed // false' "$DEPLOYMENT_FILE") != true ]] || return 0 + (( MOUNT_ENTRIES > 0 )) || return 0 + mount_ct_rootfs + oci_quiet python3 "${SCRIPT_DIR}/oci_managed_volumes.py" prepare "$rootfs" "$DEPLOYMENT_FILE" \ + || preparation_failed=1 + oci_quiet pct unmount "$VMID" || preparation_failed=1 + (( preparation_failed == 0 )) \ + || die "$(translate "Could not add the mount point:") CT $VMID" +} + +prepare_managed_format_directories "/var/lib/lxc/${VMID}/rootfs" apply_extra_hosts apply_installer_profile ensure_runtime_home diff --git a/oci/remote/oci_console_mark.sh b/oci/remote/oci_console_mark.sh index e6c7dda3..622b7194 100755 --- a/oci/remote/oci_console_mark.sh +++ b/oci/remote/oci_console_mark.sh @@ -5,6 +5,13 @@ case "$1" in ''|*[!0-9]*) exit 0 ;; esac +# Direct PVE restore formats managed filesystems again. The installed record +# authorizes this narrow cleanup; missing/busy records leave the guest alone. +# A hook failure must not prevent the console marker or the DHCP timer. +volumes="${0%/*}/oci_managed_volumes.py" +if [ -r "$volumes" ] && command -v timeout >/dev/null 2>&1; then + timeout --signal=TERM --kill-after=2 10 python3 "$volumes" startup "$1" >/dev/null 2>&1 || : +fi printf '\n=== ProxMenux: container started %s ===\n' "$(date '+%Y-%m-%d %H:%M:%S')" \ >> "/var/log/proxmenux/oci/$1.console.log" 2>/dev/null lease="${0%/*}/oci_dhcp_lease.py" diff --git a/oci/remote/oci_instance_transaction.py b/oci/remote/oci_instance_transaction.py index cc36b0d0..06b1397d 100644 --- a/oci/remote/oci_instance_transaction.py +++ b/oci/remote/oci_instance_transaction.py @@ -1261,28 +1261,9 @@ def apply(root, vmid, archive, operation, proposal=None, registry_digest=None, i def remove_empty_format_directories(rootfs, deployment): - """Remove only empty, unmapped mkfs lost+found on restored managed disks.""" - root = Path(rootfs) - for mount in deployment.get('mounts', []): - if mount.get('type') != 'managed-volume' or not mount.get('backup'): - continue - current = root - for part in Path(mount['container_path']).parts[1:]: - if part in ('.', '..'): - raise ValueError(translate('Invalid restored volume path')) - current /= part - if current.is_symlink(): - raise ValueError(translate('Symbolic link in a restored volume path')) - candidate = current / 'lost+found' - if not candidate.exists() or candidate.is_symlink(): - continue - info = candidate.lstat() - if (stat.S_ISDIR(info.st_mode) and info.st_ino == 11 - and info.st_uid == 0 and info.st_gid == 0 - and stat.S_IMODE(info.st_mode) == 0o700): - if not any(candidate.iterdir()): - candidate.rmdir() - + """Keep the recovery entrypoint, sharing the guarded format-dir cleanup.""" + from oci_managed_volumes import remove_empty_format_directories as clean + return clean(rootfs, deployment) def cleanup_restored_format_dirs(vmid, deployment, installation_id): owned(vmid, installation_id) diff --git a/oci/remote/oci_managed_volumes.py b/oci/remote/oci_managed_volumes.py new file mode 100644 index 00000000..20be099e --- /dev/null +++ b/oci/remote/oci_managed_volumes.py @@ -0,0 +1,183 @@ +#!/usr/bin/env python3 +"""Remove only empty, unshifted ext4 format directories on managed mounts.""" +from __future__ import annotations + +import argparse +import errno +import fcntl +import json +import os +from pathlib import Path +import re +import stat +import subprocess + + +def mount_path(value): + if (not isinstance(value, str) or not value.startswith('/') or value == '/' + or '\x00' in value or '\n' in value + or not value.strip('/') + or any(part in ('.', '..') for part in value.split('/'))): + raise ValueError('Invalid managed volume path') + return '/' + '/'.join(part for part in value.split('/') if part) + + +def eligible_paths(deployment, backed_only=True): + mounts = deployment.get('mounts', []) + if not isinstance(mounts, list) or any(not isinstance(m, dict) for m in mounts): + raise ValueError('Invalid managed mount contract') + paths = [(mount_path(m.get('container_path')), m) for m in mounts] + if len({path for path, _ in paths}) != len(paths): + raise ValueError('Duplicate managed volume path') + selected = [] + for path, mount in paths: + if (mount.get('type') != 'managed-volume' or mount.get('read_only') + or (backed_only and not mount.get('backup'))): + continue + candidate = path + '/lost+found' + # A nested mount can replace the directory or something inside it. + if any(other == candidate or other.startswith(candidate + '/') + for other, _ in paths): + continue + selected.append(path) + return selected + + +def format_directory(info): + return (stat.S_ISDIR(info.st_mode) and info.st_ino == 11 + and info.st_uid == 0 and info.st_gid == 0 + and stat.S_IMODE(info.st_mode) == 0o700) + + +def remove_empty_at(parent): + try: + before = os.stat('lost+found', dir_fd=parent, follow_symlinks=False) + except FileNotFoundError: + return False + if not format_directory(before): + return False + flags = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW + child = os.open('lost+found', flags, dir_fd=parent) + try: + opened = os.fstat(child) + if (opened.st_dev, opened.st_ino) != (before.st_dev, before.st_ino): + return False + with os.scandir(child) as entries: + if next(entries, None) is not None: + return False + current = os.stat('lost+found', dir_fd=parent, follow_symlinks=False) + if ((current.st_dev, current.st_ino) != (opened.st_dev, opened.st_ino) + or not format_directory(current)): + return False + # rmdir cannot delete files, and does not follow a replacement link. + os.rmdir('lost+found', dir_fd=parent) + return True + finally: + os.close(child) + + +def remove_empty_format_directories(rootfs, deployment, backed_only=True, target=None): + paths = eligible_paths(deployment, backed_only) + if target is not None: + wanted = mount_path(target) + paths = [path for path in paths if path == wanted] + if not paths: + return 0 + flags = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW + try: + root = os.open(rootfs, flags) + except FileNotFoundError: + return 0 + removed = 0 + try: + for path in paths: + current = os.dup(root) + try: + for part in path.lstrip('/').split('/'): + following = os.open(part, flags, dir_fd=current) + os.close(current) + current = following + removed += int(remove_empty_at(current)) + except FileNotFoundError: + continue + except OSError as error: + if error.errno in (errno.ELOOP, errno.ENOTDIR): + raise ValueError('Symbolic link or invalid managed volume path') from error + raise + finally: + os.close(current) + finally: + os.close(root) + return removed + + +def startup_mounts(config, deployment): + """Use the current PVE mount type/RO flag, gated by the saved contract.""" + declared = {mount_path(m.get('container_path')): m + for m in deployment.get('mounts', [])} + mounts = [] + for key, value in config.items(): + if not re.fullmatch(r'mp[0-9]+', key): + continue + source, *parts = value.split(',') + options = dict(part.split('=', 1) for part in parts if '=' in part) + path = mount_path(options.get('mp')) + saved = declared.get(path, {}) + managed = (saved.get('type') == 'managed-volume' and not source.startswith('/') + and ':' in source) + mounts.append({'container_path': path, + 'type': 'managed-volume' if managed else 'host-bind', + 'backup': bool(saved.get('backup')) and options.get('backup') == '1', + 'read_only': bool(saved.get('read_only')) or options.get('ro', '0') != '0'}) + return {'mounts': mounts} + + +def cleanup_startup(root, vmid, rootfs_base=Path('/var/lib/lxc')): + # Locks are nonblocking. A transaction already prepares its own restore + # before starting it; the hook must not wait on its parent's registry lock. + import oci_instances as instances + from oci_installation_state import parse_config + lock = os.open(root / '.lock', os.O_RDONLY | os.O_NOFOLLOW) + try: + fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) + record = instances.read(root, vmid) + if record.get('status') != 'installed' or record.get('pending_transaction'): + return 0 + if not eligible_paths(record['deployment']): + return 0 + result = subprocess.run(['pct', 'config', str(vmid)], capture_output=True, timeout=3) + if result.returncode or instances.identity(result.stdout) != record['installation_id']: + return 0 + mounts = startup_mounts(parse_config(result.stdout), record['deployment']) + return remove_empty_format_directories(rootfs_base / str(vmid) / 'rootfs', mounts) + finally: + os.close(lock) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + sub = parser.add_subparsers(dest='action', required=True) + prepare = sub.add_parser('prepare') + prepare.add_argument('rootfs', type=Path) + prepare.add_argument('deployment', type=Path) + prepare.add_argument('--target') + startup = sub.add_parser('startup') + startup.add_argument('vmid', type=int) + args = parser.parse_args() + try: + if args.action == 'prepare': + remove_empty_format_directories(args.rootfs, json.loads(args.deployment.read_text()), + backed_only=False, target=args.target) + elif args.vmid > 0: + import oci_instances as instances + cleanup_startup(instances.ROOT, args.vmid) + else: + return 1 + except (OSError, ValueError, KeyError, TypeError, subprocess.SubprocessError): + # Private contracts/configuration must never be printed by a hook. + return 1 + return 0 + + +if __name__ == '__main__': + raise SystemExit(main()) diff --git a/oci/remote/oci_restore_recovery.py b/oci/remote/oci_restore_recovery.py index fd9a2d92..ab1827df 100644 --- a/oci/remote/oci_restore_recovery.py +++ b/oci/remote/oci_restore_recovery.py @@ -757,13 +757,18 @@ def clean_format_directories(entry): """A restore formats the volumes again, and the `lost+found` of each new one belongs to the host: an application that walks its data cannot enter it and fails to start.""" + from oci_managed_volumes import eligible_paths mounts = [] for key, value in parse_config(entry['config'].encode()).items(): - if re.fullmatch(r'mp[0-9]+', key) and not value.startswith('/'): - target = oci_stack_modify.options(value).get('mp') - if target: - mounts.append({'type': 'managed-volume', 'backup': True, 'container_path': target}) - if not mounts: + if not re.fullmatch(r'mp[0-9]+', key): + continue + options = oci_stack_modify.options(value) + target = options.get('mp') + if target: + mounts.append({'type': 'host-bind' if value.startswith('/') else 'managed-volume', + 'backup': True, 'container_path': target, + 'read_only': options.get('ro', '0') != '0'}) + if not eligible_paths({'mounts': mounts}): return with carried.container_root(entry['vmid']) as rootfs: if rootfs is not None: diff --git a/oci/src/proxmenux_oci/installer.py b/oci/src/proxmenux_oci/installer.py index f8e88d43..ce067425 100644 --- a/oci/src/proxmenux_oci/installer.py +++ b/oci/src/proxmenux_oci/installer.py @@ -1588,7 +1588,7 @@ def _run_remote_install( shutil.copy2(project_root / 'remote' / 'haos_healthcheck.py', temporary / 'haos_healthcheck.py') shutil.copy2(project_root / 'remote' / 'oci_installation_state.py', temporary / 'oci_installation_state.py') shutil.copy2(project_root / 'remote' / 'oci_instances.py', temporary / 'oci_instances.py') - for helper in ('oci_ui.sh', 'oci_ui.py', 'oci_description.py', 'oci_console.py', 'oci_native_stack.py', 'oci_native_stack.sh', 'oci_instance_transaction.py', 'oci_host_mounts.py', 'oci_nested_mounts.py', 'oci_runtime_settings.py', 'oci_gpu_devices.py', 'oci_accelerators.py', 'oci_nvidia_runtime.py', 'oci_nvidia_refresh.py', 'oci_nvidia_dynamic.py', 'oci_update_current.py', 'oci_resource_modify.py', 'oci_native_modify.py', 'oci_stack_replay.py', 'oci_stack_plan.py', 'oci_stack_transaction.py', 'oci_stack_native.py', 'oci_image_cache.py', 'nvidia_lxc_mount_lab.sh', 'oci_nvidia_setup.sh', 'oci_immich_ml.sh', 'oci_rocm_check.py'): + for helper in ('oci_ui.sh', 'oci_ui.py', 'oci_description.py', 'oci_console.py', 'oci_native_stack.py', 'oci_native_stack.sh', 'oci_instance_transaction.py', 'oci_managed_volumes.py', 'oci_host_mounts.py', 'oci_nested_mounts.py', 'oci_runtime_settings.py', 'oci_gpu_devices.py', 'oci_accelerators.py', 'oci_nvidia_runtime.py', 'oci_nvidia_refresh.py', 'oci_nvidia_dynamic.py', 'oci_update_current.py', 'oci_resource_modify.py', 'oci_native_modify.py', 'oci_stack_replay.py', 'oci_stack_plan.py', 'oci_stack_transaction.py', 'oci_stack_native.py', 'oci_image_cache.py', 'nvidia_lxc_mount_lab.sh', 'oci_nvidia_setup.sh', 'oci_immich_ml.sh', 'oci_rocm_check.py'): shutil.copy2(project_root / 'remote' / helper, temporary / helper) if deployment_kind == 'generic-multi-lxc-stack': shutil.copy2(project_root / 'remote' / 'install_generic_stack.py', temporary / 'install_generic_stack.py') @@ -1625,7 +1625,7 @@ def _run_remote_install( archive.add(project_root / 'remote' / 'haos_healthcheck.py', arcname='haos_healthcheck.py') archive.add(project_root / 'remote' / 'oci_installation_state.py', arcname='oci_installation_state.py') archive.add(project_root / 'remote' / 'oci_instances.py', arcname='oci_instances.py') - for helper in ('oci_ui.sh', 'oci_ui.py', 'oci_description.py', 'oci_console.py', 'oci_native_stack.py', 'oci_native_stack.sh', 'oci_instance_transaction.py', 'oci_host_mounts.py', 'oci_nested_mounts.py', 'oci_runtime_settings.py', 'oci_gpu_devices.py', 'oci_accelerators.py', 'oci_nvidia_runtime.py', 'oci_nvidia_refresh.py', 'oci_nvidia_dynamic.py', 'oci_update_current.py', 'oci_resource_modify.py', 'oci_native_modify.py', 'oci_stack_replay.py', 'oci_stack_plan.py', 'oci_stack_transaction.py', 'oci_stack_native.py', 'oci_image_cache.py', 'nvidia_lxc_mount_lab.sh', 'oci_nvidia_setup.sh', 'oci_immich_ml.sh', 'oci_rocm_check.py'): + for helper in ('oci_ui.sh', 'oci_ui.py', 'oci_description.py', 'oci_console.py', 'oci_native_stack.py', 'oci_native_stack.sh', 'oci_instance_transaction.py', 'oci_managed_volumes.py', 'oci_host_mounts.py', 'oci_nested_mounts.py', 'oci_runtime_settings.py', 'oci_gpu_devices.py', 'oci_accelerators.py', 'oci_nvidia_runtime.py', 'oci_nvidia_refresh.py', 'oci_nvidia_dynamic.py', 'oci_update_current.py', 'oci_resource_modify.py', 'oci_native_modify.py', 'oci_stack_replay.py', 'oci_stack_plan.py', 'oci_stack_transaction.py', 'oci_stack_native.py', 'oci_image_cache.py', 'nvidia_lxc_mount_lab.sh', 'oci_nvidia_setup.sh', 'oci_immich_ml.sh', 'oci_rocm_check.py'): archive.add(project_root / 'remote' / helper, arcname=helper) if deployment_kind == 'generic-multi-lxc-stack': archive.add(project_root / 'remote' / 'install_generic_stack.py', arcname='install_generic_stack.py') diff --git a/oci/tests/test_managed_volume_installer.py b/oci/tests/test_managed_volume_installer.py new file mode 100644 index 00000000..5ff8ac23 --- /dev/null +++ b/oci/tests/test_managed_volume_installer.py @@ -0,0 +1,162 @@ +"""Exercise actual installer and persisted hook entrypoints with local fixtures.""" +from pathlib import Path +import json +import os +import shlex +import subprocess +import sys +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[1] + + +class FirstInstallTests(unittest.TestCase): + def run_case(self, mount_type='managed-volume', read_only=False, stack=False, + invalid=False, empty=False): + with tempfile.TemporaryDirectory() as tmp: + base = Path(tmp); rootfs = base / 'rootfs' + lost = rootfs / 'config/lost+found'; lost.mkdir(parents=True, mode=0o700) + sentinel = rootfs / 'config/keep'; sentinel.write_text('application data') + deployment = {'stack_managed': stack, 'mounts': [] if empty else [{ + 'type': mount_type, 'container_path': 'relative' if invalid else '/config', + 'backup': True, 'read_only': read_only}]} + plan = base / 'deployment.json'; plan.write_text(json.dumps(deployment)) + wrapper = base / 'python3' + wrapper.write_text('#!' + sys.executable + '\n' + + 'import sys\nsys.path[:0]=' + repr([str(ROOT / 'remote'), str(ROOT / 'tests')]) + '\n' + + 'from test_managed_volume_startup import ext4_metadata\n' + + 'import oci_managed_volumes\nsys.argv=sys.argv[1:]\n' + + 'with ext4_metadata():\n raise SystemExit(oci_managed_volumes.main())\n') + wrapper.chmod(0o700) + source = (ROOT / 'remote/install_oci.sh').read_text() + start = source.index('prepare_managed_format_directories() {') + end = source.index('\nprepare_managed_format_directories "/var/lib/lxc/', start) + function = source[start:end] + script = '''set -Eeuo pipefail +VMID=1000 +MOUNT_ENTRIES=$COUNT +mount_ct_rootfs() { printf 'MOUNT\n'; } +oci_quiet() { + if [[ $1 == pct ]]; then printf 'UNMOUNT\n'; else "$@"; fi +} +translate() { printf '%s' "$1"; } +die() { printf 'FAILURE\n'; exit 70; } +''' + function + '\nprepare_managed_format_directories "$ROOTFS"\n' + result = subprocess.run(['bash', '-c', script], capture_output=True, text=True, timeout=10, + env=dict(os.environ, PATH=str(base) + ':' + os.environ['PATH'], + SCRIPT_DIR=str(ROOT / 'remote'), DEPLOYMENT_FILE=str(plan), + ROOTFS=str(rootfs), COUNT='0' if empty else '1')) + self.assertEqual(sentinel.read_text(), 'application data') + return result, lost.exists() + + def test_unannotated_config_volume_is_prepared_before_first_start(self): + result, exists = self.run_case() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertFalse(exists) + self.assertEqual(result.stdout.splitlines(), ['MOUNT', 'UNMOUNT']) + + def test_readonly_and_bind_sources_are_unchanged(self): + for options in ({'read_only': True}, {'mount_type': 'host-bind'}): + with self.subTest(options=options): + result, exists = self.run_case(**options) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertTrue(exists) + + def test_cleanup_failure_unmounts_and_stops_before_start(self): + result, exists = self.run_case(invalid=True) + self.assertEqual(result.returncode, 70) + self.assertEqual(result.stdout.splitlines(), ['MOUNT', 'UNMOUNT', 'FAILURE']) + self.assertTrue(exists) + + def test_stack_member_and_empty_mount_list_have_no_mount_or_cleanup_calls(self): + for options in ({'stack': True}, {'empty': True}): + with self.subTest(options=options): + result, exists = self.run_case(**options) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, '') + self.assertTrue(exists) + + +class PersistedHookTests(unittest.TestCase): + def run_case(self, *, failure=False, helper=True, invalid=False, timeout=False): + with tempfile.TemporaryDirectory() as tmp: + base = Path(tmp); marker = base / '1000.console.log'; events = base / 'events' + source = (ROOT / 'remote/oci_console_mark.sh').read_text() + source = source.replace('/var/log/proxmenux/oci/', str(base) + '/') + script = base / 'oci_console_mark.sh'; script.write_text(source) + (base / 'oci_dhcp_lease.py').touch() + if helper: (base / 'oci_managed_volumes.py').touch() + python = base / 'python3' + python.write_text('#!/bin/sh\n' + + 'test ! -f "$MARKER" || exit 80\n' + + 'printf "cleanup %s\\n" "$3" >> "$EVENTS"\n' + + ('sleep 30\n' if timeout else '') + ('exit 1\n' if failure else 'exit 0\n')) + python.chmod(0o700) + timer = base / 'systemd-run' + timer.write_text('#!/bin/sh\ntest -s "$MARKER" || exit 81\nprintf "dhcp\\n" >> "$EVENTS"\n') + timer.chmod(0o700) + result = subprocess.run(['sh', str(script), 'invalid;id' if invalid else '1000'], + capture_output=True, text=True, timeout=15, + env=dict(os.environ, PATH=str(base) + ':' + os.environ['PATH'], + MARKER=str(marker), EVENTS=str(events))) + return result, marker.exists(), events.read_text().splitlines() if events.exists() else [] + + def test_cleanup_precedes_console_marker_and_dhcp_timer(self): + result, marker, events = self.run_case() + self.assertEqual(result.returncode, 0, result.stderr) + self.assertTrue(marker) + self.assertEqual(events, ['cleanup 1000', 'dhcp']) + + def test_missing_or_failed_helper_keeps_marker_and_dhcp(self): + for options, expected in (({'helper': False}, ['dhcp']), + ({'failure': True}, ['cleanup 1000', 'dhcp'])): + with self.subTest(options=options): + result, marker, events = self.run_case(**options) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertTrue(marker) + self.assertEqual(events, expected) + + def test_hung_cleanup_is_bounded_and_dhcp_still_runs(self): + result, marker, events = self.run_case(timeout=True) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertTrue(marker) + self.assertEqual(events, ['cleanup 1000', 'dhcp']) + + def test_invalid_vmid_has_no_cleanup_marker_or_timer(self): + result, marker, events = self.run_case(invalid=True) + self.assertEqual(result.returncode, 0) + self.assertFalse(marker) + self.assertEqual(events, []) + + +class InstallerTransportTests(unittest.TestCase): + def test_local_and_ssh_staging_include_the_guarded_helper_without_host_calls(self): + import io + import tarfile + from unittest.mock import patch + sys.path.insert(0, str(ROOT / 'src')) + from proxmenux_oci import installer + expected = (ROOT / 'remote/oci_managed_volumes.py').read_bytes() + with tempfile.TemporaryDirectory() as tmp: + template = Path(tmp) / 'template.json'; template.write_text('{}') + for transport in ('local', 'unused-test-host'): + for kind in (None, 'generic-multi-lxc-stack'): + with self.subTest(transport=transport, kind=kind): + def inspect(command, payload=None): + if transport == 'local': + folder = Path(command[1]).parent + self.assertEqual((folder / 'oci_managed_volumes.py').read_bytes(), expected) + else: + with tarfile.open(fileobj=io.BytesIO(payload), mode='r:gz') as archive: + self.assertEqual(archive.extractfile('oci_managed_volumes.py').read(), expected) + return {'checked': True} + with patch.object(installer, '_stream_process', side_effect=inspect), \ + patch.object(installer.subprocess, 'run') as host: + self.assertEqual(installer._run_remote_install(ROOT, template, + {'deployment_kind': kind}, transport, dry_run=True), {'checked': True}) + host.assert_not_called() + + +if __name__ == '__main__': + unittest.main() diff --git a/oci/tests/test_managed_volume_startup.py b/oci/tests/test_managed_volume_startup.py new file mode 100644 index 00000000..2af33395 --- /dev/null +++ b/oci/tests/test_managed_volume_startup.py @@ -0,0 +1,301 @@ +"""Managed filesystem preparation must preserve read-only data.""" +from pathlib import Path +import sys +import tempfile +import unittest +from unittest.mock import patch + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / 'remote')) +import oci_instance_transaction as transaction + + +class RecoveryGuardTests(unittest.TestCase): + def test_read_only_managed_volume_is_not_probed_or_changed(self): + deployment = {'mounts': [{'type': 'managed-volume', 'backup': True, + 'container_path': '/config', 'read_only': True}]} + with patch.object(Path, 'is_symlink', return_value=False) as probe, \ + patch.object(Path, 'exists', return_value=False): + transaction.remove_empty_format_directories('/fixture/rootfs', deployment) + probe.assert_not_called() + + def test_nested_read_only_mount_protects_the_format_directory(self): + deployment = {'mounts': [ + {'type': 'managed-volume', 'backup': True, 'container_path': '/config'}, + {'type': 'host-bind', 'container_path': '/config/lost+found', 'read_only': True}, + ]} + with patch.object(Path, 'is_symlink', return_value=False) as probe, \ + patch.object(Path, 'exists', return_value=False): + transaction.remove_empty_format_directories('/fixture/rootfs', deployment) + probe.assert_not_called() + + def test_a_relative_mount_path_is_rejected_before_filesystem_probes(self): + deployment = {'mounts': [{'type': 'managed-volume', 'backup': True, + 'container_path': 'config'}]} + with patch.object(Path, 'is_symlink', return_value=False) as probe, \ + patch.object(Path, 'exists', return_value=False): + with self.assertRaises(ValueError): + transaction.remove_empty_format_directories('/fixture/rootfs', deployment) + probe.assert_not_called() + + +import contextlib +import copy +import fcntl +import json +import os +from types import SimpleNamespace +import subprocess + +import oci_managed_volumes as volumes +import oci_restore_recovery as recovery +import oci_instances as instances + + +@contextlib.contextmanager +def ext4_metadata(**overrides): + """Use real filesystem operations; only mkfs inode/ownership are fixtures.""" + real_stat, real_fstat = os.stat, os.fstat + def info(value): + fields = {key: getattr(value, key) for key in ('st_dev', 'st_ino', 'st_uid', 'st_gid', 'st_mode')} + fields.update(st_ino=11, st_uid=0, st_gid=0) + fields.update(overrides) + return SimpleNamespace(**fields) + def observed(path, *args, **kwargs): + value = real_stat(path, *args, **kwargs) + return info(value) if str(path) == 'lost+found' else value + def opened(fd): + value = real_fstat(fd) + return info(value) if os.readlink(f'/proc/self/fd/{fd}').endswith('/lost+found') else value + with patch.object(volumes.os, 'stat', side_effect=observed), \ + patch.object(volumes.os, 'fstat', side_effect=opened): + yield + + +class FormatDirectoryTests(unittest.TestCase): + def fixture(self, root): + target = root / 'config' + target.mkdir() + lost = target / 'lost+found' + lost.mkdir(mode=0o700) + (target / 'keep').write_bytes(b'application data') + return lost, {'mounts': [{'container_path': '/config', 'type': 'managed-volume', 'backup': True}]} + + def test_only_an_empty_format_directory_is_removed(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp); lost, deployment = self.fixture(root) + before = copy.deepcopy(deployment) + with ext4_metadata(): + self.assertEqual(volumes.remove_empty_format_directories(root, deployment), 1) + self.assertFalse(lost.exists()) + self.assertEqual((root / 'config/keep').read_bytes(), b'application data') + self.assertEqual(deployment, before) + + def test_nonempty_format_directory_has_no_remove_call(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp); lost, deployment = self.fixture(root) + (lost / 'recovered-file').write_bytes(b'must survive') + with ext4_metadata(), patch.object(volumes.os, 'rmdir') as remove: + self.assertEqual(volumes.remove_empty_format_directories(root, deployment), 0) + remove.assert_not_called() + self.assertEqual((lost / 'recovered-file').read_bytes(), b'must survive') + + def test_nonstandard_metadata_is_preserved_without_remove(self): + for override in ({'st_ino': 12}, {'st_uid': 100000}, {'st_gid': 100000}, {'st_mode': 0o40755}): + with self.subTest(override=override), tempfile.TemporaryDirectory() as tmp: + root = Path(tmp); lost, deployment = self.fixture(root) + with ext4_metadata(**override), patch.object(volumes.os, 'rmdir') as remove: + self.assertEqual(volumes.remove_empty_format_directories(root, deployment), 0) + remove.assert_not_called() + self.assertTrue(lost.is_dir()) + + def test_link_and_parent_link_never_reach_external_data(self): + for parent in (False, True): + with self.subTest(parent=parent), tempfile.TemporaryDirectory() as tmp: + base = Path(tmp); root = base / 'root'; root.mkdir() + outside = base / 'outside'; outside.mkdir() + (outside / 'keep').write_text('external') + if parent: + (root / 'config').symlink_to(outside, target_is_directory=True) + else: + (root / 'config').mkdir() + (root / 'config/lost+found').symlink_to(outside, target_is_directory=True) + deployment = {'mounts': [{'container_path': '/config', 'type': 'managed-volume', 'backup': True}]} + with patch.object(volumes.os, 'rmdir') as remove: + if parent: + with self.assertRaises(ValueError): + volumes.remove_empty_format_directories(root, deployment) + else: + self.assertEqual(volumes.remove_empty_format_directories(root, deployment), 0) + remove.assert_not_called() + self.assertEqual((outside / 'keep').read_text(), 'external') + + def test_bind_readonly_and_nonbacked_recovery_make_no_filesystem_calls(self): + for options in ({'type': 'host-bind'}, {'read_only': True}, {'backup': False}): + mount = {'container_path': '/config', 'type': 'managed-volume', 'backup': True, **options} + with self.subTest(options=options), patch.object(volumes.os, 'open') as opened, \ + patch.object(volumes.os, 'rmdir') as remove: + self.assertEqual(volumes.remove_empty_format_directories('/rootfs', {'mounts': [mount]}), 0) + opened.assert_not_called(); remove.assert_not_called() + + def test_new_nonbacked_managed_disk_is_prepared_without_changing_backup_policy(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp); lost, deployment = self.fixture(root) + deployment['mounts'][0]['backup'] = False + with ext4_metadata(): + self.assertEqual(volumes.remove_empty_format_directories(root, deployment, backed_only=False), 1) + self.assertFalse(lost.exists()) + self.assertFalse(deployment['mounts'][0]['backup']) + + def test_read_failure_does_not_mean_empty_and_does_not_remove(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp); lost, deployment = self.fixture(root) + with ext4_metadata(), patch.object(volumes.os, 'scandir', side_effect=PermissionError), \ + patch.object(volumes.os, 'rmdir') as remove: + with self.assertRaises(PermissionError): + volumes.remove_empty_format_directories(root, deployment) + remove.assert_not_called() + self.assertTrue(lost.is_dir()) + + def test_invalid_paths_are_rejected_before_any_filesystem_call(self): + for path in ('config', '/', '//', '/config/../outside', '/config/./data', '/bad\npath'): + with self.subTest(path=path), patch.object(volumes.os, 'open') as opened: + with self.assertRaises(ValueError): + volumes.remove_empty_format_directories('/rootfs', {'mounts': [ + {'container_path': path, 'type': 'managed-volume', 'backup': True}]}) + opened.assert_not_called() + + + def test_duplicate_paths_are_rejected_before_filesystem_calls(self): + mount = {'container_path': '/config', 'type': 'managed-volume', 'backup': True} + with patch.object(volumes.os, 'open') as opened: + with self.assertRaises(ValueError): + volumes.remove_empty_format_directories('/rootfs', {'mounts': [mount, dict(mount)]}) + opened.assert_not_called() + + +class StartupContractTests(unittest.TestCase): + def setUp(self): + directory = tempfile.TemporaryDirectory(); self.addCleanup(directory.cleanup) + self.base = Path(directory.name); self.registry = self.base / 'registry' + self.registry.mkdir(); (self.registry / '.lock').touch(mode=0o600) + self.record = {'schema_version': 1, 'vmid': 1000, + 'installation_id': '11111111-1111-4111-8111-111111111111', + 'status': 'installed', 'deployment': {'mounts': [ + {'container_path': '/config', 'type': 'managed-volume', 'backup': True}]}} + (self.registry / '1000').mkdir() + self.record_path = self.registry / '1000/oci-compose.json' + self.record_path.write_text(json.dumps(self.record)) + self.rootfs = self.base / 'guests/1000/rootfs' + self.rootfs.mkdir(parents=True) + self.lost = self.rootfs / 'config/lost+found'; self.lost.mkdir(parents=True, mode=0o700) + (self.rootfs / 'config/keep').write_text('application data') + self.config = (f'description: \n' + 'mp0: local:1000/vm-1000-disk-1.raw,mp=/config,backup=1,size=16G\n').encode() + + def call(self, *, config=None, returncode=0): + response = SimpleNamespace(returncode=returncode, stdout=self.config if config is None else config) + before = self.record_path.read_bytes() + with patch.object(volumes.subprocess, 'run', return_value=response) as command: + result = volumes.cleanup_startup(self.registry, 1000, self.base / 'guests') + self.assertEqual(self.record_path.read_bytes(), before) + return result, command + + def test_current_matching_managed_mount_cleans_before_start_without_guest_exec(self): + with ext4_metadata(): + result, command = self.call() + self.assertEqual(result, 1); self.assertFalse(self.lost.exists()) + command.assert_called_once_with(['pct', 'config', '1000'], capture_output=True, timeout=3) + self.assertEqual((self.rootfs / 'config/keep').read_text(), 'application data') + + def test_wrong_identity_failed_command_empty_output_and_changed_mount_do_not_remove(self): + cases = [(self.config.replace(b'11111111', b'22222222'), 0), (b'', 0), (self.config, 1), + (self.config.replace(b'local:1000/vm-1000-disk-1.raw', b'/host/data'), 0), + (self.config.replace(b'backup=1', b'backup=0'), 0), + (self.config.replace(b'backup=1', b'backup=1,ro=1'), 0)] + for config, code in cases: + with self.subTest(config=config, code=code), ext4_metadata(), patch.object(volumes.os, 'rmdir') as remove: + result, _ = self.call(config=config, returncode=code) + self.assertEqual(result, 0); remove.assert_not_called() + self.assertTrue(self.lost.is_dir()) + + def test_pending_unsupported_or_hostonly_record_never_calls_pct(self): + for change in ({'status': 'updating'}, {'status': 'failed'}, {'pending_transaction': '/private/journal'}, + {'deployment': {'mounts': [{'container_path': '/config', 'type': 'host-bind'}]}}): + with self.subTest(change=change): + self.record_path.write_text(json.dumps({**self.record, **change})) + result, command = self.call() + self.assertEqual(result, 0); command.assert_not_called() + self.assertTrue(self.lost.is_dir()) + + def test_busy_lock_is_immediate_and_leaves_files_unchanged(self): + with (self.registry / '.lock').open('r') as lock: + fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) + with patch.object(volumes.subprocess, 'run') as command, patch.object(volumes.os, 'rmdir') as remove: + with self.assertRaises(BlockingIOError): + volumes.cleanup_startup(self.registry, 1000, self.base / 'guests') + command.assert_not_called(); remove.assert_not_called() + self.assertTrue(self.lost.is_dir()) + + def test_missing_or_invalid_record_and_timeout_never_remove(self): + for value in ('missing', 'invalid', 'timeout'): + with self.subTest(value=value): + if value == 'missing': self.record_path.unlink() + elif value == 'invalid': self.record_path.write_text('{') + else: self.record_path.write_text(json.dumps(self.record)) + with patch.object(volumes.subprocess, 'run', side_effect=subprocess.TimeoutExpired('pct', 3)), \ + patch.object(volumes.os, 'rmdir') as remove: + with self.assertRaises((OSError, ValueError, subprocess.TimeoutExpired)): + volumes.cleanup_startup(self.registry, 1000, self.base / 'guests') + remove.assert_not_called() + self.assertTrue(self.lost.is_dir()) + + +class RecoveryConsumerTests(unittest.TestCase): + def test_transaction_recovery_uses_real_cleanup_and_always_unmounts(self): + with tempfile.TemporaryDirectory() as tmp: + base = Path(tmp); root = base / '1000/rootfs'; root.mkdir(parents=True) + lost, deployment = FormatDirectoryTests().fixture(root) + with patch.object(transaction, 'owned') as owned, patch.object(transaction, 'run') as command, \ + patch.object(transaction, 'Path', return_value=base), ext4_metadata(), \ + patch.object(volumes, 'remove_empty_format_directories', wraps=volumes.remove_empty_format_directories) as clean: + transaction.cleanup_restored_format_dirs(1000, deployment, 'identity') + owned.assert_called_once_with(1000, 'identity') + self.assertEqual([c.args for c in command.call_args_list], + [('pct', 'mount', '1000'), ('pct', 'unmount', '1000')]) + clean.assert_called_once() + self.assertFalse(lost.exists()) + self.assertEqual((root / 'config/keep').read_bytes(), b'application data') + + def test_transaction_failure_still_unmounts_and_identity_failure_never_mounts(self): + deployment = {'mounts': [{'type': 'managed-volume', 'container_path': '/config', 'backup': True}]} + with patch.object(transaction, 'owned'), patch.object(transaction, 'run') as command, \ + patch.object(volumes, 'remove_empty_format_directories', side_effect=OSError('read failed')): + with self.assertRaises(OSError): + transaction.cleanup_restored_format_dirs(1000, deployment, 'identity') + self.assertEqual(command.call_args_list[-1].args, ('pct', 'unmount', '1000')) + with patch.object(transaction, 'owned', side_effect=ValueError('identity')), \ + patch.object(transaction, 'run') as command: + with self.assertRaises(ValueError): + transaction.cleanup_restored_format_dirs(1000, deployment, 'identity') + command.assert_not_called() + + def test_recover_menu_consumer_cleans_managed_and_preserves_ro_or_nested_bind(self): + for protected in ('none', 'readonly', 'nested-bind'): + with self.subTest(protected=protected), tempfile.TemporaryDirectory() as tmp: + root = Path(tmp); lost, _ = FormatDirectoryTests().fixture(root) + config = 'mp0: local:1000/vm-1000-disk-1.raw,mp=/config,backup=1' + if protected == 'readonly': config += ',ro=1' + if protected == 'nested-bind': config += '\nmp1: /host/recovery,mp=/config/lost+found,ro=1' + with ext4_metadata(), patch.object(recovery.carried, 'container_root', return_value=contextlib.nullcontext(root)), \ + patch.object(volumes.os, 'rmdir', wraps=os.rmdir) as remove: + recovery.clean_format_directories({'vmid': 1000, 'config': config}) + if protected == 'none': + self.assertFalse(lost.exists()); remove.assert_called_once() + else: + self.assertTrue(lost.is_dir()); remove.assert_not_called() + self.assertEqual((root / 'config/keep').read_bytes(), b'application data') + + +if __name__ == '__main__': + unittest.main() diff --git a/oci/tests/test_readonly_volume_preparation.py b/oci/tests/test_readonly_volume_preparation.py index 37f67011..e823e612 100644 --- a/oci/tests/test_readonly_volume_preparation.py +++ b/oci/tests/test_readonly_volume_preparation.py @@ -1,7 +1,7 @@ """Run the installer's volume preparation loop against filesystem fixtures. Write doubles reject every mutation of the protected directory, including -lost+found removal. Writable paths still run real rm/chmod; chown is recorded +lost+found removal. Writable paths keep ordinary directories and run real chmod; chown is recorded without requiring root. These tests never invoke Proxmox or touch a container. """ @@ -62,6 +62,8 @@ TEMPLATE_FILE="$FIXTURE/template.json" DEPLOYMENT_FILE="$FIXTURE/deployment.json" HOST_BIND_UID=110001; HOST_BIND_GID=110001 HOST_ROOT_UID=100000; HOST_ROOT_GID=100000 +SCRIPT_DIR="$REMOTE" +oci_quiet() { printf "WRITE cleanup %s\n" "${!#}"; "$@"; } failed=0; volumes_prepared=0 oci_log() { printf 'log %s\n' "$*"; } write_op() { @@ -87,7 +89,7 @@ test "$failed" = 0 ''' result = subprocess.run( ["bash", "-c", script], capture_output=True, text=True, - env=dict(os.environ, FIXTURE=str(fixture), + env=dict(os.environ, FIXTURE=str(fixture), REMOTE=str(ROOT / "remote"), PROTECTED=str(protected) if read_only else ""), timeout=15, ) @@ -102,7 +104,7 @@ test "$failed" = 0 self.assertTrue((protected / "lost+found").is_dir()) elif "prepared=1" in result.stdout: self.assertEqual(protected.stat().st_mode & 0o777, 0o750) - self.assertFalse((protected / "lost+found").exists()) + self.assertTrue((protected / "lost+found").is_dir()) return result def assert_skipped(self, result): @@ -113,7 +115,7 @@ test "$failed" = 0 def assert_prepared(self, result): self.assertEqual(result.returncode, 0, result.stderr + result.stdout) - for operation in ("rm", "chown", "chmod"): + for operation in ("cleanup", "chown", "chmod"): self.assertIn(f"WRITE {operation} ", result.stdout) self.assertIn("prepared=1", result.stdout) From 1511680a95995e2d3282db1b37deb3e030813d5e Mon Sep 17 00:00:00 2001 From: VAIO73 <50487331+Vaso73@users.noreply.github.com> Date: Mon, 19 Oct 2026 12:53:34 +0000 Subject: [PATCH 3/4] fix(oci): reject missing managed mount roots --- oci/remote/oci_managed_volumes.py | 7 +------ oci/tests/test_managed_volume_startup.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/oci/remote/oci_managed_volumes.py b/oci/remote/oci_managed_volumes.py index 20be099e..a0068939 100644 --- a/oci/remote/oci_managed_volumes.py +++ b/oci/remote/oci_managed_volumes.py @@ -84,10 +84,7 @@ def remove_empty_format_directories(rootfs, deployment, backed_only=True, target if not paths: return 0 flags = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW - try: - root = os.open(rootfs, flags) - except FileNotFoundError: - return 0 + root = os.open(rootfs, flags) removed = 0 try: for path in paths: @@ -98,8 +95,6 @@ def remove_empty_format_directories(rootfs, deployment, backed_only=True, target os.close(current) current = following removed += int(remove_empty_at(current)) - except FileNotFoundError: - continue except OSError as error: if error.errno in (errno.ELOOP, errno.ENOTDIR): raise ValueError('Symbolic link or invalid managed volume path') from error diff --git a/oci/tests/test_managed_volume_startup.py b/oci/tests/test_managed_volume_startup.py index 2af33395..e2a4ef8d 100644 --- a/oci/tests/test_managed_volume_startup.py +++ b/oci/tests/test_managed_volume_startup.py @@ -174,6 +174,21 @@ class FormatDirectoryTests(unittest.TestCase): opened.assert_not_called() + def test_missing_rootfs_is_an_error_without_remove(self): + with tempfile.TemporaryDirectory() as tmp, patch.object(volumes.os, 'rmdir') as remove: + with self.assertRaises(FileNotFoundError): + volumes.remove_empty_format_directories(Path(tmp) / 'missing', {'mounts': [ + {'container_path': '/config', 'type': 'managed-volume', 'backup': True}]}) + remove.assert_not_called() + + def test_missing_managed_mount_is_an_error_without_remove(self): + with tempfile.TemporaryDirectory() as tmp, patch.object(volumes.os, 'rmdir') as remove: + with self.assertRaises(FileNotFoundError): + volumes.remove_empty_format_directories(Path(tmp), {'mounts': [ + {'container_path': '/missing', 'type': 'managed-volume', 'backup': True}]}) + remove.assert_not_called() + + class StartupContractTests(unittest.TestCase): def setUp(self): directory = tempfile.TemporaryDirectory(); self.addCleanup(directory.cleanup) From 3e3e0916f220badc7b97c48218e2100520e6f442 Mon Sep 17 00:00:00 2001 From: MacRimi <123239993+MacRimi@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:06:49 +0200 Subject: [PATCH 4/4] fix(oci): clean format directories of a running container - Open the container root without O_NOFOLLOW, as /proc//root is a link - Keep the /config preparation of JDownloader in its overlay --- oci/catalog/overlays/jdownloader.json | 5 +++++ oci/remote/oci_managed_volumes.py | 3 ++- oci/tests/test_managed_volume_startup.py | 10 ++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/oci/catalog/overlays/jdownloader.json b/oci/catalog/overlays/jdownloader.json index 8125de69..e8007e6d 100644 --- a/oci/catalog/overlays/jdownloader.json +++ b/oci/catalog/overlays/jdownloader.json @@ -2,6 +2,11 @@ "proxmox": { "installer_profile": { "volume_preparations": [ + { + "container_path": "/config", + "remove_lost_found": true, + "owner_strategy": "mapped-root" + }, { "container_path": "/output", "remove_lost_found": true, diff --git a/oci/remote/oci_managed_volumes.py b/oci/remote/oci_managed_volumes.py index a0068939..575d16aa 100644 --- a/oci/remote/oci_managed_volumes.py +++ b/oci/remote/oci_managed_volumes.py @@ -84,7 +84,8 @@ def remove_empty_format_directories(rootfs, deployment, backed_only=True, target if not paths: return 0 flags = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW - root = os.open(rootfs, flags) + # A running container is reached through /proc//root, a link. + root = os.open(rootfs, os.O_RDONLY | os.O_DIRECTORY) removed = 0 try: for path in paths: diff --git a/oci/tests/test_managed_volume_startup.py b/oci/tests/test_managed_volume_startup.py index e2a4ef8d..17df7d1b 100644 --- a/oci/tests/test_managed_volume_startup.py +++ b/oci/tests/test_managed_volume_startup.py @@ -91,6 +91,16 @@ class FormatDirectoryTests(unittest.TestCase): self.assertEqual((root / 'config/keep').read_bytes(), b'application data') self.assertEqual(deployment, before) + def test_running_container_root_reached_through_a_link_is_cleaned(self): + with tempfile.TemporaryDirectory() as tmp: + base = Path(tmp); real = base / 'rootfs'; real.mkdir() + lost, deployment = self.fixture(real) + proc_root = base / 'root'; proc_root.symlink_to(real, target_is_directory=True) + with ext4_metadata(): + self.assertEqual(volumes.remove_empty_format_directories(proc_root, deployment), 1) + self.assertFalse(lost.exists()) + self.assertEqual((real / 'config/keep').read_bytes(), b'application data') + def test_nonempty_format_directory_has_no_remove_call(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp); lost, deployment = self.fixture(root)