From 017b73fffdcfa27ad9681f9f63fc6add8f2a7b41 Mon Sep 17 00:00:00 2001 From: VAIO73 <50487331+Vaso73@users.noreply.github.com> Date: Thu, 15 Oct 2026 04:11:10 +0000 Subject: [PATCH] fix: clear obsolete LXC check failure markers --- AppImage/scripts/lxc_check_job.py | 54 +++++++++++++++++++++--- AppImage/scripts/managed_installs.py | 30 ++++++++++++- tests/lxc_updates/test_check_only_job.py | 21 +++++++++ tests/lxc_updates/test_observed_state.py | 32 ++++++++++++++ 4 files changed, 131 insertions(+), 6 deletions(-) diff --git a/AppImage/scripts/lxc_check_job.py b/AppImage/scripts/lxc_check_job.py index 548a3440..441dbd8d 100644 --- a/AppImage/scripts/lxc_check_job.py +++ b/AppImage/scripts/lxc_check_job.py @@ -38,6 +38,53 @@ def acquire(vmid): raise +def _invalid_marker(vmid): + return LOCK_DIR / f'proxmenux-lxc-check-invalid-{int(vmid)}' + + +def _acquire_invalid_marker_guard(vmid): + path = LOCK_DIR / f'proxmenux-lxc-check-invalid-{int(vmid)}.guard' + handle = os.fdopen(os.open(path, os.O_RDWR | os.O_CREAT | os.O_NOFOLLOW, 0o600), 'r+') + try: + fcntl.flock(handle, fcntl.LOCK_EX) + return handle + except BaseException: + handle.close() + raise + + +def _mark_check_invalid(vmid): + handle = _acquire_invalid_marker_guard(vmid) + try: + _invalid_marker(vmid).touch(mode=0o600) + finally: + handle.close() + + +def clear_invalid_marker(vmid, *, check_started_ns=None): + """Clear an obsolete failed-check marker without hiding a newer run.""" + try: + handle = _acquire_invalid_marker_guard(vmid) + except OSError: + return False + try: + marker = _invalid_marker(vmid) + try: + if (check_started_ns is not None + and marker.stat().st_mtime_ns > check_started_ns): + return False + marker.unlink() + return True + except FileNotFoundError: + # A concurrent successful check may already have published the + # valid state; there is no stale marker left to clear. + return True + except OSError: + return False + finally: + handle.close() + + def guard(vmid): if not managed_installs._lxc_updates_detection_enabled(): raise RuntimeError('LXC update detection is disabled') @@ -101,9 +148,8 @@ def checked_os(vmid): def run(vmid, job_id, reservation): error = None check_started = False - invalid = LOCK_DIR / f'proxmenux-lxc-check-invalid-{int(vmid)}' try: - invalid.touch(mode=0o600) + _mark_check_invalid(vmid) guard(vmid) if status(vmid) != 'running': raise RuntimeError('Container is stopped; start it first') @@ -124,9 +170,7 @@ def run(vmid, job_id, reservation): error += '; package result could not be persisted' finally: if not error: - try: - invalid.unlink() - except OSError: + if not clear_invalid_marker(vmid): error = 'Successful check could not be published for installation' reservation.close() with _lock: diff --git a/AppImage/scripts/managed_installs.py b/AppImage/scripts/managed_installs.py index e85f3fb7..b05ab307 100644 --- a/AppImage/scripts/managed_installs.py +++ b/AppImage/scripts/managed_installs.py @@ -1796,6 +1796,22 @@ def _store_update_result(item: dict, result: dict) -> None: item["update_check"][extra_key] = result[extra_key] +def _clear_lxc_check_invalid_marker(vmid: int, check_started_ns: int) -> bool: + """Publish a persisted newer LXC check to the installation gate. + + The marker is created by the explicit refresh worker. Its own lock also + protects this cleanup against a later refresh starting while a scheduled + or lifecycle check is still completing. + """ + try: + import lxc_check_job + return lxc_check_job.clear_invalid_marker( + vmid, check_started_ns=check_started_ns) + except Exception as exc: + print(f'[ProxMenux] could not clear stale LXC check marker for {vmid}: {exc}') + return False + + def refresh_oci_app(app_id: str) -> Optional[dict]: """Read one OCI-managed application again and store what it reports. @@ -1834,6 +1850,7 @@ def check_for_updates(force: bool = False) -> list[dict]: _nvidia_cache["fetched_at"] = 0 updates_available: list[dict] = [] + successful_lxc_checks: list[tuple[int, int]] = [] with _lock: reg = _read_registry() items = reg.get("items", []) @@ -1854,6 +1871,7 @@ def check_for_updates(force: bool = False) -> list[dict]: checker = _CHECKERS.get(it.get("type")) if not checker: continue + check_started_ns = time.time_ns() if it.get('type') == 'lxc' else None try: result = checker(it) except Exception as e: @@ -1863,13 +1881,20 @@ def check_for_updates(force: bool = False) -> list[dict]: "last_check": _now_iso(), "error": str(e)} _store_update_result(it, result) + if check_started_ns is not None and result.get('error') is None: + try: + successful_lxc_checks.append((int(it.get('_vmid')), check_started_ns)) + except (TypeError, ValueError): + pass if it["update_check"]["available"]: updates_available.append(it) reg["items"] = items reg["last_check_run"] = _now_iso() - _write_registry(reg) + if _write_registry(reg): + for vmid, check_started_ns in successful_lxc_checks: + _clear_lxc_check_invalid_marker(vmid, check_started_ns) return updates_available @@ -1945,6 +1970,7 @@ def refresh_lxc(vmid: int, *, refresh_package_cache: bool = True) -> Optional[di else: _merge_detected_entry(target, entry, now) + check_started_ns = time.time_ns() try: result = (_check_lxc_updates(target) if refresh_package_cache else _check_lxc_updates(target, refresh_package_cache=False)) @@ -1961,4 +1987,6 @@ def refresh_lxc(vmid: int, *, refresh_package_cache: bool = True) -> Optional[di reg["last_targeted_refresh"] = now if not _write_registry(reg): raise OSError("could not persist LXC update check") + if result.get('error') is None: + _clear_lxc_check_invalid_marker(target_vmid, check_started_ns) return dict(target) diff --git a/tests/lxc_updates/test_check_only_job.py b/tests/lxc_updates/test_check_only_job.py index a6635970..f9804861 100644 --- a/tests/lxc_updates/test_check_only_job.py +++ b/tests/lxc_updates/test_check_only_job.py @@ -1,6 +1,7 @@ """Real job, Flask routes and shared flock; guest transport is isolated.""" import ast import datetime +import os import subprocess import sys import tempfile @@ -90,6 +91,26 @@ class CheckOnlyTests(unittest.TestCase): self.assertEqual(result['state'], 'complete') self.assertFalse((self.folder / 'proxmenux-lxc-check-invalid-100').exists()) + def test_marker_cleanup_failure_reports_failure_and_releases_the_operation_lock(self): + self.state = 'running' + with patch.object(job, 'clear_invalid_marker', return_value=False): + result, _, _ = self.run_check() + self.assertEqual(result['state'], 'failed') + self.assertIn('could not be published', result['error']) + reservation = job.acquire(100) + reservation.close() + + def test_marker_created_after_a_check_started_is_not_cleared(self): + marker = self.folder / 'proxmenux-lxc-check-invalid-100' + marker.touch() + os.utime(marker, ns=(2_000, 2_000)) + + self.assertFalse(job.clear_invalid_marker(100, check_started_ns=1_000)) + self.assertTrue(marker.exists()) + + def test_already_cleared_marker_is_a_successful_publish(self): + self.assertTrue(job.clear_invalid_marker(100)) + def test_index_read_timeout_and_persistence_errors_keep_running_guest_running(self): self.state = 'running' for error in [RuntimeError('index failed'), OSError('read failed'), diff --git a/tests/lxc_updates/test_observed_state.py b/tests/lxc_updates/test_observed_state.py index 1c25828f..1a348259 100644 --- a/tests/lxc_updates/test_observed_state.py +++ b/tests/lxc_updates/test_observed_state.py @@ -365,6 +365,35 @@ class ObservedStateRoutesTests(unittest.TestCase): self.assertEqual(managed.get_registry()["items"][0]["_os_family"], "debian") self.assertEqual(self.row()["update_check"]["count"], 0) + def test_daily_successful_check_clears_an_older_failed_check_marker(self): + marker = Path(self.temp.name) / 'proxmenux-lxc-check-invalid-100' + marker.touch() + + with patch.object(managed, '_refresh_lxc_pkg_cache_if_stale', return_value={}): + managed.check_for_updates() + + self.assertFalse(marker.exists()) + self.assertIsNone(managed.get_registry()['items'][0]['update_check']['error']) + + def test_daily_check_keeps_a_marker_created_after_it_started(self): + marker = Path(self.temp.name) / 'proxmenux-lxc-check-invalid-100' + marker.touch() + os.utime(marker, ns=(2_000, 2_000)) + + with patch.object(managed.time, 'time_ns', return_value=1_000): + managed.check_for_updates() + + self.assertTrue(marker.exists()) + + def test_daily_check_does_not_clear_marker_when_result_cannot_be_persisted(self): + marker = Path(self.temp.name) / 'proxmenux-lxc-check-invalid-100' + marker.touch() + + with patch.object(managed, '_write_registry', return_value=False): + managed.check_for_updates() + + self.assertTrue(marker.exists()) + def test_existing_global_refresh_route_keeps_its_collector_contract(self): with patch.object(managed, "_DETECTORS", [managed._detect_lxc_containers]), \ patch.object(managed, "_refresh_lxc_pkg_cache_if_stale", return_value={}) as refresh: @@ -374,10 +403,13 @@ class ObservedStateRoutesTests(unittest.TestCase): self.assertEqual(managed.get_registry()["items"][0]["_os_family"], "debian") def test_existing_lifecycle_refresh_keeps_index_refresh_default(self): + marker = Path(self.temp.name) / 'proxmenux-lxc-check-invalid-100' + marker.touch() with patch.object(managed, "_refresh_lxc_pkg_cache_if_stale", return_value={}) as refresh: item = managed.refresh_lxc(100) refresh.assert_called_once_with("100", "debian") self.assertEqual(item["update_check"]["_count"], 0) + self.assertFalse(marker.exists()) def test_other_lxc_has_independent_ip_cache_and_manual_rate_limit(self): conf200 = Path(self.temp.name) / "200.conf"