fix: preserve verified LXC state before a failed start

This commit is contained in:
VAIO73
2026-10-14 17:51:53 +00:00
parent 3489dc0530
commit 638552e5f8
7 changed files with 228 additions and 19 deletions
+20 -1
View File
@@ -2076,7 +2076,14 @@ def _refresh_started_guest(vmid: int, vm_type: str) -> None:
lxc_apps.invalidate_suggestion_probes(vmid) lxc_apps.invalidate_suggestion_probes(vmid)
try:
managed_installs.refresh_lxc(vmid) managed_installs.refresh_lxc(vmid)
except Exception as exc:
# Package inventory is one stage of a started-guest rebuild.
# A slow/failed `pct list` must not skip app, cache, and UI
# refreshes that can still complete for this guest.
print(f'[ProxMenux] lifecycle refresh lxc {vmid}: '
f'package inventory: {exc}', flush=True)
sidecar = lxc_apps.check_all(vmid, force=True) sidecar = lxc_apps.check_all(vmid, force=True)
sidecar = sidecar or {'vmid': vmid, 'apps': []} sidecar = sidecar or {'vmid': vmid, 'apps': []}
@@ -14551,10 +14558,22 @@ def _finalize_lxc_update(
import managed_installs import managed_installs
import lxc_update_observation import lxc_update_observation
observation = lxc_update_observation.read(vmid, safe_run_id) observation = lxc_update_observation.read(vmid, safe_run_id)
started = lxc_update_observation.read_started(vmid, safe_run_id)
marker_write_failed = str(reason or '').endswith('code 8')
# A failed start, foreign-node rejection, or pre-start cancel has
# no trusted start marker and must not erase the last observed
# package state. A rejected marker is unknown and is handled as a
# touched run; runner exit 8 also means a start occurred but its
# marker could not be recorded.
if observation is None and started is False and not marker_write_failed:
verified = True
untouched = True
else:
verified = managed_installs.store_lxc_update_observation(vmid, observation) verified = managed_installs.store_lxc_update_observation(vmid, observation)
untouched = False
print(f'[ProxMenux] stopped LXC verification: vmid={vmid} ' print(f'[ProxMenux] stopped LXC verification: vmid={vmid} '
f'run_id={safe_run_id} handoff={observation is not None} ' f'run_id={safe_run_id} handoff={observation is not None} '
f'verified={verified}', flush=True) f'started={started} untouched={untouched} verified={verified}', flush=True)
if not verified: if not verified:
verification_errors.append('post-update OS verification unavailable') verification_errors.append('post-update OS verification unavailable')
apps_persisted = True apps_persisted = True
+75 -2
View File
@@ -29,6 +29,11 @@ def _path(vmid: int, run_id: str) -> Path:
return OBSERVATION_DIR / f"{int(vmid)}-{run_id}.json" return OBSERVATION_DIR / f"{int(vmid)}-{run_id}.json"
def _started_path(vmid: int, run_id: str) -> Path:
_path(vmid, run_id)
return OBSERVATION_DIR / f"{int(vmid)}-{run_id}.started"
def _private_directory(*, create=True) -> None: def _private_directory(*, create=True) -> None:
if create: if create:
OBSERVATION_DIR.mkdir(mode=0o700, parents=True, exist_ok=True) OBSERVATION_DIR.mkdir(mode=0o700, parents=True, exist_ok=True)
@@ -38,6 +43,65 @@ def _private_directory(*, create=True) -> None:
raise OSError("unsafe LXC observation directory") raise OSError("unsafe LXC observation directory")
def mark_started(vmid: int, run_id: str) -> None:
"""Record that this run started a stopped CT before it can change state."""
path = _started_path(vmid, run_id)
_private_directory()
payload = {
"vmid": int(vmid),
"run_id": run_id,
"started_at": datetime.datetime.now(datetime.timezone.utc).isoformat(),
}
fd, temporary = tempfile.mkstemp(prefix=".started-", dir=OBSERVATION_DIR)
try:
os.fchmod(fd, 0o600)
with os.fdopen(fd, "w", encoding="utf-8") as handle:
json.dump(payload, handle)
os.replace(temporary, path)
print(f"[ProxMenux] LXC update started: vmid={int(vmid)} run_id={run_id}", flush=True)
finally:
if os.path.exists(temporary):
os.unlink(temporary)
def read_started(vmid: int, run_id: str) -> bool | None:
"""Return True for a trusted start marker, False when absent, else None."""
path = _started_path(vmid, run_id)
def reject(reason: str) -> None:
print(f"[ProxMenux] LXC update start marker rejected: vmid={int(vmid)} "
f"run_id={run_id} reason={reason}", flush=True)
return None
try:
_private_directory(create=False)
except FileNotFoundError:
return False
except OSError as exc:
return reject(type(exc).__name__)
try:
fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)
except FileNotFoundError:
return False
except OSError as exc:
return reject(type(exc).__name__)
try:
with os.fdopen(fd, "r", encoding="utf-8") as handle:
info = os.fstat(handle.fileno())
if (not stat.S_ISREG(info.st_mode) or info.st_uid != os.geteuid()
or info.st_mode & 0o077 or info.st_size > MAX_BYTES):
return reject('unsafe-file')
marker = json.load(handle)
started = datetime.datetime.fromisoformat(marker["started_at"])
age = (datetime.datetime.now(datetime.timezone.utc) - started).total_seconds()
if (not isinstance(marker, dict) or marker.get("vmid") != int(vmid)
or marker.get("run_id") != run_id or not 0 <= age <= MAX_AGE):
return reject('identity-age-or-shape')
return True
except (OSError, ValueError, TypeError, KeyError, OverflowError) as exc:
return reject(type(exc).__name__)
def capture(vmid: int, run_id: str, *, refresh_indexes: bool = False, def capture(vmid: int, run_id: str, *, refresh_indexes: bool = False,
include_apps: bool = False) -> dict: include_apps: bool = False) -> dict:
path = _path(vmid, run_id) path = _path(vmid, run_id)
@@ -46,7 +110,7 @@ def capture(vmid: int, run_id: str, *, refresh_indexes: bool = False,
now = datetime.datetime.now(datetime.timezone.utc) now = datetime.datetime.now(datetime.timezone.utc)
for old in OBSERVATION_DIR.iterdir(): for old in OBSERVATION_DIR.iterdir():
info = old.lstat() info = old.lstat()
if (re.fullmatch(r"[1-9][0-9]*-[A-Za-z0-9._:-]{1,128}\.json", old.name) if (re.fullmatch(r"[1-9][0-9]*-[A-Za-z0-9._:-]{1,128}(?:\.json|\.started)", old.name)
and stat.S_ISREG(info.st_mode) and info.st_uid == os.geteuid() and stat.S_ISREG(info.st_mode) and info.st_uid == os.geteuid()
and now.timestamp() - info.st_mtime > MAX_AGE): and now.timestamp() - info.st_mtime > MAX_AGE):
old.unlink() old.unlink()
@@ -156,7 +220,11 @@ def read(vmid: int, run_id: str) -> dict | None:
def discard(vmid: int, run_id: str) -> None: def discard(vmid: int, run_id: str) -> None:
try: try:
_private_directory(create=False) _private_directory(create=False)
_path(vmid, run_id).unlink() for path in (_path(vmid, run_id), _started_path(vmid, run_id)):
try:
path.unlink()
except FileNotFoundError:
pass
except FileNotFoundError: except FileNotFoundError:
pass pass
@@ -165,10 +233,15 @@ def main(argv=None) -> int:
parser = argparse.ArgumentParser(description=__doc__) parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("--vmid", required=True, type=int) parser.add_argument("--vmid", required=True, type=int)
parser.add_argument("--run-id", required=True) parser.add_argument("--run-id", required=True)
parser.add_argument('--mark-started', action='store_true',
help='Record that this run started a previously stopped CT')
parser.add_argument('--include-apps', action='store_true', parser.add_argument('--include-apps', action='store_true',
help='Capture registered installed versions after an update') help='Capture registered installed versions after an update')
args = parser.parse_args(argv) args = parser.parse_args(argv)
try: try:
if args.mark_started:
mark_started(args.vmid, args.run_id)
return 0
capture(args.vmid, args.run_id, include_apps=args.include_apps) capture(args.vmid, args.run_id, include_apps=args.include_apps)
return 0 return 0
except Exception as exc: except Exception as exc:
+5 -1
View File
@@ -540,6 +540,10 @@ def _list_pve_lxcs(*, strict: bool = False) -> list[dict]:
raise RuntimeError('LXC inventory could not be read') raise RuntimeError('LXC inventory could not be read')
return [] return []
lines = r.stdout.splitlines() lines = r.stdout.splitlines()
# `pct list` returns successful empty stdout when the node has no CTs.
# That is a complete inventory, so strict callers may safely retire rows.
if not any(line.strip() for line in lines):
return []
if strict and (not lines or not lines[0].strip().startswith('VMID') or 'Status' not in lines[0]): if strict and (not lines or not lines[0].strip().startswith('VMID') or 'Status' not in lines[0]):
raise RuntimeError('LXC inventory is incomplete') raise RuntimeError('LXC inventory is incomplete')
@@ -1892,7 +1896,7 @@ def store_lxc_update_observation(vmid: int, observation: Optional[dict]) -> bool
reg = _read_registry() reg = _read_registry()
items = reg.setdefault("items", []) items = reg.setdefault("items", [])
target = next((it for it in items if it.get("id") == f"lxc:{vmid}" target = next((it for it in items if it.get("id") == f"lxc:{vmid}"
and it.get("type") == "lxc" and not it.get("removed_at")), None) and it.get("type") == "lxc"), None)
if target is None: if target is None:
if observation is None: if observation is None:
return False return False
+10
View File
@@ -44,6 +44,7 @@
# nor explicitly-enabled verified helper) available in the CT # nor explicitly-enabled verified helper) available in the CT
# 6 post-update restart failed # 6 post-update restart failed
# 7 another ProxMenux update is already running for this CT # 7 another ProxMenux update is already running for this CT
# 8 started CT could not record its verification handoff identity
# #
# The frontend surfaces exit code + duration in a follow-up POST to # The frontend surfaces exit code + duration in a follow-up POST to
# /api/lxc-updates/<vmid>/applied so the notification event fires with # /api/lxc-updates/<vmid>/applied so the notification event fires with
@@ -183,6 +184,15 @@ if [[ "$STATE" != "running" ]]; then
exit 2 exit 2
fi fi
STARTED_BY_PROXMENUX=1 STARTED_BY_PROXMENUX=1
# The finalizer must distinguish a failed/pre-start run from a run that
# actually started this previously stopped CT but later lacks a result.
if [[ -n "${RUN_ID:-}" ]]; then
if ! python3 /usr/local/share/proxmenux/monitor-app/usr/bin/lxc_update_observation.py \
--vmid "$VMID" --run-id "$RUN_ID" --mark-started; then
echo "ERROR: started CT verification identity could not be recorded." >&2
exit 8
fi
fi
# give the CT a moment for services to come up # give the CT a moment for services to come up
sleep 3 sleep 3
fi fi
@@ -128,6 +128,24 @@ class StoppedRegistryTests(unittest.TestCase):
self.assertEqual(row['_os_family'], 'alpine') self.assertEqual(row['_os_family'], 'alpine')
self.assert_no_guest_probe() self.assert_no_guest_probe()
def test_targeted_observation_reactivates_removed_row_without_a_duplicate(self):
registry = managed._read_registry()
registry['items'][0]['removed_at'] = self.now
managed._write_registry(registry)
observation = {
'entry': {**self.row, '_os_family': 'debian'},
'result': {'available': True, 'last_check': self.now, '_count': 2},
}
self.assertTrue(managed.store_lxc_update_observation(210, observation))
registry = managed.get_registry()
rows = [row for row in registry['items'] if row['id'] == 'lxc:210']
self.assertEqual(len(rows), 1)
self.assertNotIn('removed_at', rows[0])
self.assertEqual(rows[0]['update_check']['_count'], 2)
self.assert_no_guest_probe()
def test_retained_result_still_expires_at_existing_48_hour_boundary(self): def test_retained_result_still_expires_at_existing_48_hour_boundary(self):
managed.detect_and_register() managed.detect_and_register()
current = managed.get_active_items()[0]['update_check'] current = managed.get_active_items()[0]['update_check']
@@ -145,7 +163,7 @@ class StoppedRegistryTests(unittest.TestCase):
class InventoryContractTests(unittest.TestCase): class InventoryContractTests(unittest.TestCase):
def test_default_failure_contract_and_strict_detector_contract(self): def test_default_failure_contract_and_strict_detector_contract(self):
for result in [subprocess.CompletedProcess([], 1, '', ''), subprocess.CompletedProcess([], 0, '', '')]: result = subprocess.CompletedProcess([], 1, '', '')
with patch.object(managed.subprocess, 'run', return_value=result): with patch.object(managed.subprocess, 'run', return_value=result):
self.assertEqual(managed._list_pve_lxcs(), []) self.assertEqual(managed._list_pve_lxcs(), [])
with self.assertRaises(RuntimeError): with self.assertRaises(RuntimeError):
@@ -162,7 +180,11 @@ class InventoryContractTests(unittest.TestCase):
managed._list_pve_lxcs(strict=True) managed._list_pve_lxcs(strict=True)
def test_confirmed_empty_inventory_is_not_a_failed_read(self): def test_confirmed_empty_inventory_is_not_a_failed_read(self):
with patch.object(managed.subprocess, 'run', return_value=subprocess.CompletedProcess([], 0, 'VMID Status Lock Name\n', '')): for stdout in ['', '\n', 'VMID Status Lock Name\n']:
with self.subTest(stdout=repr(stdout)), patch.object(
managed.subprocess, 'run',
return_value=subprocess.CompletedProcess([], 0, stdout, ''),
):
self.assertEqual(managed._list_pve_lxcs(strict=True), []) self.assertEqual(managed._list_pve_lxcs(strict=True), [])
@@ -59,9 +59,13 @@ root = pathlib.Path(os.environ['FIXTURE_DIR'])
args = sys.argv[1:] args = sys.argv[1:]
with (root/'commands').open('a') as log: log.write(json.dumps(args)+'\\n') with (root/'commands').open('a') as log: log.write(json.dumps(args)+'\\n')
state = root/'state' state = root/'state'
if args[0] == 'list': print('VMID Status Name\\n100 '+state.read_text()+' fixture') if args[0] == 'list':
if os.environ.get('FOREIGN') == '1': print('VMID Status Name\\n200 stopped foreign')
else: print('VMID Status Name\\n100 '+state.read_text()+' fixture')
elif args[0] == 'status': print('status: '+state.read_text()) elif args[0] == 'status': print('status: '+state.read_text())
elif args[0] == 'start': state.write_text('running') elif args[0] == 'start':
if os.environ.get('START_FAIL') == '1': sys.exit(1)
state.write_text('running')
elif args[0] == 'shutdown': elif args[0] == 'shutdown':
if os.environ.get('SHUTDOWN_FAIL') == '1': sys.exit(1) if os.environ.get('SHUTDOWN_FAIL') == '1': sys.exit(1)
state.write_text('stopped') state.write_text('stopped')
@@ -102,8 +106,10 @@ m._PCT_BIN = str(root/'bin/pct')
h.OBSERVATION_DIR = root/'observations' h.OBSERVATION_DIR = root/'observations'
apps._APPS_DIR = str(root/'apps') apps._APPS_DIR = str(root/'apps')
apps._PCT_BIN = str(root/'bin/pct') apps._PCT_BIN = str(root/'bin/pct')
with (root/'commands').open('a') as log: log.write('CAPTURE\\n') marker = '--mark-started' in os.sys.argv
if os.environ.get('CAPTURE_FAIL') == '1': raise SystemExit(1) with (root/'commands').open('a') as log: log.write('MARK_STARTED\\n' if marker else 'CAPTURE\\n')
if os.environ.get('MARKER_FAIL') == '1' and marker: raise SystemExit(1)
if os.environ.get('CAPTURE_FAIL') == '1' and not marker: raise SystemExit(1)
m._detect_lxc_containers = lambda **_: ([{'id':'lxc:100','type':'lxc','_vmid':'100', m._detect_lxc_containers = lambda **_: ([{'id':'lxc:100','type':'lxc','_vmid':'100',
'_is_oci':os.environ.get('OCI') == '1'}] if (root/'state').read_text() == 'running' else []) '_is_oci':os.environ.get('OCI') == '1'}] if (root/'state').read_text() == 'running' else [])
raise SystemExit(h.main()) raise SystemExit(h.main())
@@ -169,11 +175,12 @@ raise SystemExit(h.main())
self.assertIsNone(row['error']) self.assertIsNone(row['error'])
self.assertEqual(row['os_family'], 'debian') self.assertEqual(row['os_family'], 'debian')
log = self.log.read_text() log = self.log.read_text()
self.assertLess(log.index('MARK_STARTED'), log.index('CAPTURE'))
self.assertLess(log.index('CAPTURE'), log.index('shutdown')) self.assertLess(log.index('CAPTURE'), log.index('shutdown'))
self.assertLess(log.index('apt list --upgradable'), log.index('shutdown')) self.assertLess(log.index('apt list --upgradable'), log.index('shutdown'))
self.assertNotIn('apt-get update', log) self.assertNotIn('apt-get update', log)
self.assertEqual(log.count('dist-upgrade'), 1) self.assertEqual(log.count('dist-upgrade'), 1)
self.assertFalse(list(self.observations.glob('*.json'))) self.assertFalse(list(self.observations.iterdir()))
self.assertEqual(managed.get_registry()['items'][1]['update_check']['_count'], 7) self.assertEqual(managed.get_registry()['items'][1]['update_check']['_count'], 7)
self.assertEqual(self.fixture.commands, [], 'GET and stopped finalization run no guest probes') self.assertEqual(self.fixture.commands, [], 'GET and stopped finalization run no guest probes')
@@ -198,6 +205,33 @@ raise SystemExit(h.main())
def test_missing_capture_does_not_present_old_count_or_fresh_zero(self): def test_missing_capture_does_not_present_old_count_or_fresh_zero(self):
self.assert_unverified('CAPTURE_FAIL') self.assert_unverified('CAPTURE_FAIL')
def test_pre_start_failure_or_foreign_target_preserves_last_verified_result(self):
for condition in ('START_FAIL', 'FOREIGN'):
with self.subTest(condition=condition):
registry = managed.get_registry()
registry['items'][0]['update_check'] = {
'available': True, 'last_check': '2026-10-14T00:00:00Z',
'_count': 20, 'error': None,
}
managed._write_registry(registry)
result = self.run_manual(**{condition: '1'})
self.assertNotEqual(result.returncode, 0)
self.assertEqual(self.state.read_text(), 'stopped')
stored = managed.get_registry()['items'][0]['update_check']
self.assertEqual(stored['_count'], 20)
self.assertIsNone(stored['error'])
log = self.log.read_text()
self.assertNotIn('MARK_STARTED', log)
self.assertNotIn('CAPTURE', log)
def test_started_run_with_unwritable_marker_is_reported_as_unverified(self):
result = self.run_manual(MARKER_FAIL='1', CAPTURE_FAIL='1')
self.assertEqual(result.returncode, 8)
self.assertEqual(self.state.read_text(), 'stopped')
stored = managed.get_registry()['items'][0]['update_check']
self.assertTrue(stored['error'])
self.assertNotIn('_count', stored)
def test_web_runner_without_identity_reports_unverified_instead_of_silent_skip(self): def test_web_runner_without_identity_reports_unverified_instead_of_silent_skip(self):
env = {**self.env, 'EXECUTION_MODE': 'web'} env = {**self.env, 'EXECUTION_MODE': 'web'}
env.pop('RUN_ID') env.pop('RUN_ID')
@@ -210,6 +244,7 @@ raise SystemExit(h.main())
def test_rejected_handoff_is_retained_with_safe_diagnostic_and_no_false_zero(self): def test_rejected_handoff_is_retained_with_safe_diagnostic_and_no_false_zero(self):
self.observations.mkdir(mode=0o700) self.observations.mkdir(mode=0o700)
handoff.mark_started(100, 'manual-fixture')
path = self.observations / '100-manual-fixture.json' path = self.observations / '100-manual-fixture.json'
path.write_text(json.dumps({'private_payload': 'must-not-be-logged'})) path.write_text(json.dumps({'private_payload': 'must-not-be-logged'}))
path.chmod(0o600) path.chmod(0o600)
@@ -219,7 +254,7 @@ raise SystemExit(h.main())
status='success', source='manual', target='os') status='success', source='manual', target='os')
self.assertTrue(path.exists()) self.assertTrue(path.exists())
self.assertIn('reason=KeyError', output.getvalue()) self.assertIn('reason=KeyError', output.getvalue())
self.assertIn('handoff=False verified=False', output.getvalue()) self.assertIn('handoff=False started=True untouched=False verified=False', output.getvalue())
self.assertNotIn('must-not-be-logged', output.getvalue()) self.assertNotIn('must-not-be-logged', output.getvalue())
self.assertTrue(self.fixture.row()['update_check']['error']) self.assertTrue(self.fixture.row()['update_check']['error'])
self.assertNotIn('_count', managed.get_registry()['items'][0]['update_check']) self.assertNotIn('_count', managed.get_registry()['items'][0]['update_check'])
+46
View File
@@ -1,6 +1,9 @@
import ast
import sys import sys
import subprocess import subprocess
import tempfile import tempfile
import threading
import time
import unittest import unittest
from types import SimpleNamespace from types import SimpleNamespace
from pathlib import Path from pathlib import Path
@@ -23,6 +26,7 @@ class UpdateStrategyValidationTests(unittest.TestCase):
self.assertTrue(ok, config) self.assertTrue(ok, config)
self.assertEqual(config["update_strategy"], "custom_override") self.assertEqual(config["update_strategy"], "custom_override")
def test_legacy_helper_then_custom_is_normalised_to_override(self): def test_legacy_helper_then_custom_is_normalised_to_override(self):
ok, config = lxc_apps.validate_config({ ok, config = lxc_apps.validate_config({
"name": "Jellyfin", "name": "Jellyfin",
@@ -42,6 +46,48 @@ class UpdateStrategyValidationTests(unittest.TestCase):
self.assertEqual(config["update_strategy"], "custom_override") self.assertEqual(config["update_strategy"], "custom_override")
class LifecycleInventoryResilienceTests(unittest.TestCase):
def test_failed_targeted_inventory_refresh_does_not_abort_started_lxc_rebuild(self):
source = ast.parse((REPO_ROOT / 'AppImage/scripts/flask_server.py').read_text())
function = next(node for node in source.body if isinstance(node, ast.FunctionDef)
and node.name == '_refresh_started_guest')
warmed = []
published = MagicMock()
lxc = SimpleNamespace(
invalidate_suggestion_probes=MagicMock(),
check_all=MagicMock(return_value={'vmid': 100, 'apps': []}),
_pct_exec=MagicMock(return_value=(1, '', 'not installed')),
ensure_oci_registration=MagicMock(),
get_suggestions=MagicMock(return_value={'suggestions': []}),
)
managed = SimpleNamespace(
refresh_lxc=MagicMock(side_effect=RuntimeError('pct list timed out')),
)
scope = {
'time': time,
'_wait_for_started_guest': lambda *_: True,
'_vm_cache_invalidate': MagicMock(),
'_vm_disk_cache': {},
'_warm_started_guest_handler': lambda vmid, route, _: warmed.append((vmid, route)),
'get_vm_config': object(), 'api_vm_backups': object(),
'api_vm_apps_schedule': object(), 'api_lxc_mount_points': object(),
'_publish_guest_modal_cache_revision': published,
'_vm_cache_put': MagicMock(), '_vm_app_suggestions_cache': {},
'_guest_lifecycle_lock': threading.RLock(),
'_guest_refresh_inflight': {100}, '_guest_modal_cache_revision': {},
'_guest_modal_cache_epoch': 0,
}
with patch.dict(sys.modules, {'lxc_apps': lxc, 'managed_installs': managed}):
exec(compile(ast.Module(body=[function], type_ignores=[]), 'lifecycle-refresh', 'exec'), scope)
scope['_refresh_started_guest'](100, 'lxc')
managed.refresh_lxc.assert_called_once_with(100)
lxc.check_all.assert_called_once_with(100, force=True)
self.assertEqual(len(warmed), 4)
self.assertGreaterEqual(published.call_count, 1)
self.assertNotIn(100, scope['_guest_refresh_inflight'])
class HelperEvidenceTests(unittest.TestCase): class HelperEvidenceTests(unittest.TestCase):
def test_legacy_literal_wrapper_slug_is_supported(self): def test_legacy_literal_wrapper_slug_is_supported(self):
wrapper = ( wrapper = (