Mirrors/ProxMenux
1
0
Fork 0
mirror of https://github.com/MacRimi/ProxMenux.git synced 2026-11-01 10:22:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(monitor): the observation history of a disk is permanent

Browse Source 
- An observation is never left out for its age and cannot be dismissed:
  the rule that hid it after 30 days without a repetition, and the
  clean-up that marked the ones of a missing device, are removed.
- The ones earlier versions left out are shown again; an error that came
  back while hidden appears with its first date and its full count.
- The audit attributes recorded events to a disk by its serial number.
This commit is contained in:
MacRimi
2026-10-08 17:32:47 +02:00
parent 07713d5365
commit 693e6ae481
10 changed files with 146 additions and 162 deletions
Download Patch File Download Diff File Expand all files Collapse all files
AppImage/scripts/audit_checks_pve.py
+6 -10
View File
@@ -3268,15 +3268,12 @@ def _disk_errors(ctx):
"""
records = _recorded_disk_events()
if not records:
# An empty store and a store whose entries were all dismissed
# look the same from here, and neither supports the claim that
# no disk reported an error. Saying which of the two it is is
# not possible; saying there is nothing to grade is.
# An empty store does not support the claim that no disk ever
# reported an error: the monitor may not have been running when
# one did. Saying there is nothing to grade is what can be said.
return {"classification": CLASS_NOT_APPLICABLE,
"summary_key": "noEvents",
"evidence": "The health monitor holds no undismissed disk "
"event. A dismissed observation is dropped by "
"the monitor and is not read back here."}
"evidence": "The health monitor holds no disk event."}
now = time.time()
recent_days = ctx.policy.threshold("disk_error_recent_days")
@@ -3316,9 +3313,8 @@ def _disk_errors(ctx):
evidence += ("\nRecorded because Linux reported them; neither SMART nor "
"Proxmox surfaces these, and both may report the device as "
"healthy. Stated as separate warnings without overriding "
"that current health result. Events the reader dismissed are not "
"listed: the "
"monitor drops them and this reads what the monitor keeps. "
"that current health result. Every recorded event is "
"listed: the history of a disk is permanent. "
"Recorded by the health monitor as events occurred. SMART "
"reports the device's present opinion of itself and is read "
"by a separate check; a device can report healthy while its "
AppImage/scripts/audit_inventory.py
+28 -1
View File
@@ -587,6 +587,33 @@ def _disk_observations() -> dict[str, list[dict[str, Any]]]:
return grouped
def _observations_of(name: str, serial: str, by_name: dict[str, list[dict[str, Any]]]) -> list[dict[str, Any]]:
"""The events of the disk that is plugged there now.
A device name is a connector: the disk that was there before keeps its
own history. The Monitor tells them apart by serial number; the events
grouped by name are only used when it cannot be asked.
"""
server = sys.modules.get("flask_server") or sys.modules.get("__main__")
getter = getattr(getattr(server, "health_persistence", None), "get_disk_observations", None)
if getter is None:
return by_name.get(name, [])
try:
records = getter(name, serial or None) or []
except Exception:
return by_name.get(name, [])
entries = [{
"type": record.get("error_type", ""),
"severity": record.get("severity", ""),
"count": record.get("occurrence_count", 0),
"first_seen": record.get("first_occurrence"),
"last_seen": record.get("last_occurrence"),
"message": (record.get("raw_message") or "")[:400],
} for record in records]
entries.sort(key=lambda e: e.get("last_seen") or 0, reverse=True)
return entries
def _physical_disks(ctx) -> list[dict[str, Any]]:
observations = _disk_observations()
# The SMART cache is keyed by device, each entry a (collected_at, data)
@@ -613,7 +640,7 @@ def _physical_disks(ctx) -> list[dict[str, Any]]:
"health": health.get("smart_status"),
"temperature": health.get("temperature"),
"power_on_hours": health.get("power_on_hours"),
"observations": observations.get(name, []),
"observations": _observations_of(name, (row.get("SERIAL") or "").strip(), observations),
})
return sorted(disks, key=lambda d: d["name"])
AppImage/scripts/flask_health_routes.py
-6
View File
@@ -325,12 +325,6 @@ def cleanup_orphan_errors():
health_persistence.resolve_error(err_key, 'Device no longer present (manual cleanup)')
cleaned.append({'error_key': err_key, 'device': dev_path})
# Also cleanup disk_observations for non-existent devices
try:
health_persistence.cleanup_orphan_observations()
except Exception:
pass
return jsonify({
'success': True,
'cleaned_count': len(cleaned),
AppImage/scripts/flask_server.py
-2
View File
@@ -4620,8 +4620,6 @@ def get_storage_info():
for disk_name, disk_info in physical_disks.items():
disk_info['observations_count'] = health_persistence.count_disk_observations(
disk_name, disk_info.get('serial', ''))
# Auto-dismiss stale observations (> 30 days old)
health_persistence.cleanup_stale_observations()
except Exception:
pass
AppImage/scripts/health_persistence.py
+46 -128
View File
@@ -463,9 +463,8 @@ class HealthPersistence:
# and looks "new" again on the next boot. Audit Tier 5 — Health stack.
#
# IMPORTANT: Only cleans the `errors` table (health monitor state).
# The `disk_observations` table is a PERMANENT historical record
# and must NEVER be auto-modified on startup. Users dismiss
# observations manually from the disk detail UI.
# The `disk_observations` table is a PERMANENT historical record:
# nothing removes, hides or dismisses an observation.
#
# Covers: disk I/O (smart_*, disk_*), VM/CT (vm_*, ct_*, vmct_*),
# and log errors (log_*) — all journal-sourced categories.
@@ -510,6 +509,30 @@ class HealthPersistence:
except Exception as e:
print(f"[HealthPersistence] Startup cleanup warning: {e}")
# Earlier versions marked an observation as dismissed once it had not
# been seen for 30 days, or when its device was gone, and left it out
# of the history of its disk even when the error came back. The mark
# is no longer used; it is taken off once so the table says what is
# shown.
_OBSERVATION_HISTORY_VERSION = '1'
try:
cursor = conn.cursor()
cursor.execute('SELECT setting_value FROM user_settings WHERE setting_key = ?',
('observation_history_version',))
row = cursor.fetchone()
if not (row and row[0] == _OBSERVATION_HISTORY_VERSION):
cursor.execute('UPDATE disk_observations SET dismissed = 0 WHERE dismissed = 1')
restored = cursor.rowcount
cursor.execute('''
INSERT OR REPLACE INTO user_settings (setting_key, setting_value, updated_at)
VALUES (?, ?, ?)
''', ('observation_history_version', _OBSERVATION_HISTORY_VERSION, datetime.now().isoformat()))
conn.commit()
if restored > 0:
print(f"[HealthPersistence] Disk observation history: {restored} observation(s) shown again")
except Exception as e:
print(f"[HealthPersistence] Observation history warning: {e}")
conn.close()
@staticmethod
@@ -1437,12 +1460,8 @@ class HealthPersistence:
# Clean up errors for resources that no longer exist (VMs/CTs deleted, disks removed)
self._cleanup_stale_resources()
# NOTE: cleanup_orphan_observations() is deliberately NOT invoked here.
# Running it on the 5-minute auto-resolve cycle silently dismissed legitimate
# observations (ZFS pool errors, ATA host events, dm-* aliases) before the user
# could see them in the UI history, even though notifications were already sent.
# The cleanup is still available as an explicit user action via
# POST /api/health/cleanup-disconnected-disks (flask_health_routes.py).
# NOTE: nothing here touches `disk_observations`. That table is the
# permanent history of each disk and no cycle removes or hides rows.
def _cleanup_stale_resources(self):
"""Resolve errors for resources that no longer exist.
@@ -2663,17 +2682,10 @@ class HealthPersistence:
first_col = 'first_occurrence' if 'first_occurrence' in columns else 'first_seen'
last_col = 'last_occurrence' if 'last_occurrence' in columns else 'last_seen'
# Upsert observation: if same (disk, type, signature), bump count + update last timestamp.
# IMPORTANT: Do NOT reset dismissed — if the user dismissed this observation,
# re-detecting the same journal entry must not un-dismiss it. BUT we DO
# keep counting + updating last_occurrence even when dismissed, because the
# responsible-monitoring contract is: every error counts toward the
# accumulated total shown in the disk modal ("324 connection errors"),
# even errors of the same signature the user already saw once. Dismissed
# only mutes notifications, NOT the per-disk error history surfaced in the
# UI. Reverting the earlier "WHERE dismissed=0" gate that froze the
# counter and last_occurrence for /dev/sdh on 2026-05-09, leaving 10
# silent days of unreported ATA errors (Pedro Rico, 19/05).
# The same observation seen again adds to its count and moves its
# last date; a different one is a new row. Observations are never
# dismissed nor hidden: the `dismissed` column stays in the table
# for databases of earlier versions and is not used.
cursor.execute(f'''
INSERT INTO disk_observations
(disk_registry_id, {type_col}, error_signature, {first_col},
@@ -2752,7 +2764,7 @@ class HealthPersistence:
return '(' + ' OR '.join(clauses) + ')', params
def count_disk_observations(self, device_name: str, serial: Optional[str] = None) -> int:
"""Active observations of one disk: the number its badge shows, counted
"""How many observations a disk has: the number its badge shows, counted
the way `get_disk_observations` lists them."""
try:
with self._db_connection() as conn:
@@ -2762,7 +2774,7 @@ class HealthPersistence:
return 0
where, params = scope
cursor.execute(
f'SELECT COUNT(*) FROM disk_observations o WHERE {where} AND o.dismissed = 0', params)
f'SELECT COUNT(*) FROM disk_observations o WHERE {where}', params)
return int(cursor.fetchone()[0])
except Exception as e:
print(f"[HealthPersistence] Error counting observations: {e}")
@@ -2770,7 +2782,12 @@ class HealthPersistence:
def get_disk_observations(self, device_name: Optional[str] = None,
serial: Optional[str] = None) -> List[Dict[str, Any]]:
"""Get active (non-dismissed) observations for one disk or all disks."""
"""The observations of one disk, or of every disk.
The history of a disk is permanent and an observation cannot be
dismissed: each different one is listed, with the number of times it
was seen.
"""
try:
conn = self._get_conn()
cursor = conn.cursor()
@@ -2792,22 +2809,21 @@ class HealthPersistence:
cursor.execute(f'''
SELECT o.id, o.{type_col}, o.error_signature,
o.{first_col}, o.{last_col},
o.occurrence_count, o.raw_message, o.severity, o.dismissed,
o.occurrence_count, o.raw_message, o.severity,
d.device_name, d.serial, d.model
FROM disk_observations o
JOIN disk_registry d ON o.disk_registry_id = d.id
WHERE {where} AND o.dismissed = 0
WHERE {where}
ORDER BY o.{last_col} DESC
''', params)
else:
cursor.execute(f'''
SELECT o.id, o.{type_col}, o.error_signature,
o.{first_col}, o.{last_col},
o.occurrence_count, o.raw_message, o.severity, o.dismissed,
o.occurrence_count, o.raw_message, o.severity,
d.device_name, d.serial, d.model
FROM disk_observations o
JOIN disk_registry d ON o.disk_registry_id = d.id
WHERE o.dismissed = 0
ORDER BY o.{last_col} DESC
''')
@@ -2823,10 +2839,9 @@ class HealthPersistence:
'occurrence_count': r[5],
'raw_message': r[6] or '',
'severity': r[7],
'dismissed': bool(r[8]),
'device_name': r[9],
'serial': r[10],
'model': r[11],
'device_name': r[8],
'serial': r[9],
'model': r[10],
} for r in rows]
except Exception as e:
print(f"[HealthPersistence] Error getting observations: {e}")
@@ -2848,7 +2863,6 @@ class HealthPersistence:
SELECT DISTINCT dr.device_name, dr.serial
FROM disk_observations o
JOIN disk_registry dr ON o.disk_registry_id = dr.id
WHERE o.dismissed = 0
''')
rows = cursor.fetchall()
return [{'device_name': r[0], 'serial': r[1] or ''} for r in rows]
@@ -2856,42 +2870,6 @@ class HealthPersistence:
print(f"[HealthPersistence] get_all_observed_devices failed: {e}")
return []
def dismiss_disk_observation(self, observation_id: int):
"""Mark a single observation as dismissed."""
try:
conn = self._get_conn()
cursor = conn.cursor()
cursor.execute(
'UPDATE disk_observations SET dismissed = 1 WHERE id = ?',
(observation_id,))
conn.commit()
conn.close()
except Exception as e:
print(f"[HealthPersistence] Error dismissing observation: {e}")
def cleanup_stale_observations(self, max_age_days: int = 30):
"""Auto-dismiss observations not seen in max_age_days."""
try:
from datetime import timedelta
cutoff = (datetime.now() - timedelta(days=max_age_days)).isoformat()
conn = self._get_conn()
cursor = conn.cursor()
# Detect column name for backward compatibility
cursor.execute('PRAGMA table_info(disk_observations)')
columns = [col[1] for col in cursor.fetchall()]
last_col = 'last_occurrence' if 'last_occurrence' in columns else 'last_seen'
cursor.execute(f'''
UPDATE disk_observations
SET dismissed = 1
WHERE dismissed = 0 AND {last_col} < ?
''', (cutoff,))
conn.commit()
conn.close()
except Exception as e:
print(f"[HealthPersistence] Error cleaning stale observations: {e}")
def mark_removed_disks(self, active_device_names: List[str],
active_serials: Optional[Dict[str, str]] = None):
"""Mark disks not in active_device_names as removed. With
@@ -2920,66 +2898,6 @@ class HealthPersistence:
except Exception as e:
print(f"[HealthPersistence] Error marking removed disks: {e}")
# Logical (non-block) device-name prefixes used as observation keys for events that
# don't map to a /dev/<name> entry: ZFS pool names, ATA host identifiers (e.g. "ata8"
# from "ata8.00: exception ..." journal lines), device-mapper aliases, etc. These are
# never visible in /dev/ by design, so the original presence-based cleanup would
# always wrongly dismiss them. They are excluded from automatic cleanup; the user's
# explicit "clean up disconnected disks" action also skips them.
_LOGICAL_DEVICE_PREFIXES = ('zpool_', 'ata', 'dm-', 'nbd', 'loop', 'sr')
def cleanup_orphan_observations(self):
"""
Dismiss observations for devices that no longer exist in /dev/.
Useful for cleaning up after USB drives or temporary devices are disconnected.
Observations whose `device_name` uses a logical (non-block) prefix are skipped —
ZFS pools, ATA hosts and dm-* aliases never appear under /dev/ by design and were
being silently dismissed by the previous version of this routine.
"""
import os
import re
try:
conn = self._get_conn()
cursor = conn.cursor()
# Get all active (non-dismissed) observations with device info from disk_registry
cursor.execute('''
SELECT do.id, dr.device_name, dr.serial
FROM disk_observations do
JOIN disk_registry dr ON do.disk_registry_id = dr.id
WHERE do.dismissed = 0
''')
observations = cursor.fetchall()
dismissed_count = 0
for obs_id, device_name, serial in observations:
# Skip non-block observations (ZFS pools, ATA hosts, dm-mapper, etc.)
if device_name and device_name.startswith(self._LOGICAL_DEVICE_PREFIXES):
continue
# Check if device exists
dev_path = f'/dev/{device_name}'
# Also check base device (remove partition number)
base_dev = disk_base_name(device_name)
base_path = f'/dev/{base_dev}'
if not os.path.exists(dev_path) and not os.path.exists(base_path):
cursor.execute('''
UPDATE disk_observations SET dismissed = 1
WHERE id = ?
''', (obs_id,))
dismissed_count += 1
conn.commit()
conn.close()
if dismissed_count > 0:
print(f"[HealthPersistence] Cleaned up {dismissed_count} orphan observations")
return dismissed_count
except Exception as e:
print(f"[HealthPersistence] Error cleaning orphan observations: {e}")
return 0
# ── Remote Storage Exclusions Methods ──
# Types considered "remote" and eligible for exclusion
AppImage/scripts/test_disk_observation.py
-1
View File
@@ -103,7 +103,6 @@ def main():
print(f" First: {obs['first_occurrence']}")
print(f" Last: {obs['last_occurrence']}")
print(f" Count: {obs['occurrence_count']}")
print(f" Dismissed: {obs['dismissed']}")
print()
else:
print(" No observations found!\n")
AppImage/scripts/tests/test_disk_observations_follow_the_disk.py
+58 -3
View File
@@ -103,13 +103,68 @@ class DiskObservationIdentityTests(unittest.TestCase):
self.assertEqual(self.seen('sdd'), ['error_on_sdd'])
self.assertEqual(self.store.count_disk_observations('sda', 'Unknown'), 1)
def test_a_dismissed_observation_is_not_counted(self):
def test_each_different_observation_is_a_row_and_a_repeated_one_adds_to_its_count(self):
self.record('sdb', 'SN-1', 'read_error')
self.record('sdb', 'SN-1', 'read_error')
self.record('sdb', 'SN-1', 'read_error')
self.record('sdb', 'SN-1', 'crc_error')
first = self.store.get_disk_observations('sdb', 'SN-1')[0]['id']
self.store.dismiss_disk_observation(first)
listed = {o['error_signature']: o['occurrence_count'] for o in self.store.get_disk_observations('sdb', 'SN-1')}
self.assertEqual(listed, {'read_error': 3, 'crc_error': 1})
self.assertEqual(self.store.count_disk_observations('sdb', 'SN-1'), 2)
def test_an_observation_cannot_be_dismissed(self):
for name in ('dismiss_disk_observation', 'cleanup_stale_observations', 'cleanup_orphan_observations'):
self.assertFalse(hasattr(self.store, name), name)
self.record('sdb', 'SN-1', 'read_error')
self.assertNotIn('dismissed', self.store.get_disk_observations('sdb', 'SN-1')[0])
routes = (SCRIPTS / 'flask_health_routes.py').read_text() + (SCRIPTS / 'flask_server.py').read_text()
self.assertNotIn('cleanup_orphan_observations', routes)
self.assertNotIn('cleanup_stale_observations', routes)
panel = (SCRIPTS.parent / 'components/storage-overview.tsx').read_text()
self.assertNotIn('obs.dismissed', panel)
def test_an_old_observation_is_never_hidden(self):
self.record('sdb', 'SN-1', 'read_error')
self.set_time('disk_observations', 'last_occurrence', '2020-01-01T00:00:00', '1 = 1', ())
self.assertEqual(self.seen('sdb', 'SN-1'), ['read_error'])
self.assertEqual(self.store.count_disk_observations('sdb', 'SN-1'), 1)
def test_what_earlier_versions_hid_is_in_the_history_again(self):
self.record('sdb', 'SN-1', 'read_error')
conn = self.store._get_conn()
conn.execute('UPDATE disk_observations SET dismissed = 1')
conn.execute("DELETE FROM user_settings WHERE setting_key = 'observation_history_version'")
conn.commit()
conn.close()
# Listed whatever the old mark says, and the mark is taken off.
self.assertEqual(self.seen('sdb', 'SN-1'), ['read_error'])
self.assertEqual(self.store.count_disk_observations('sdb', 'SN-1'), 1)
with patch('builtins.print'):
self.store._init_database()
conn = self.store._get_conn()
marked = conn.execute('SELECT COUNT(*) FROM disk_observations WHERE dismissed = 1').fetchone()[0]
conn.close()
self.assertEqual(marked, 0)
class AuditAttributionTests(unittest.TestCase):
def test_the_audit_lists_the_events_of_the_disk_that_is_there_now(self):
import types
import audit_inventory
asked = []
def getter(name=None, serial=None):
asked.append((name, serial))
return [{'error_type': 'io_error', 'severity': 'warning', 'occurrence_count': 3,
'first_occurrence': '2026-10-01T00:00:00', 'last_occurrence': '2026-10-02T00:00:00',
'raw_message': 'crc'}] if serial == 'NEW-GOOD' else []
server = types.SimpleNamespace(health_persistence=types.SimpleNamespace(get_disk_observations=getter))
by_name = {'sdb': [{'type': 'io_error', 'count': 99, 'message': 'of the previous disk'}]}
with patch.dict(sys.modules, {'flask_server': server}):
entries = audit_inventory._observations_of('sdb', 'NEW-GOOD', by_name)
self.assertEqual(asked, [('sdb', 'NEW-GOOD')])
self.assertEqual([(e['type'], e['count']) for e in entries], [('io_error', 3)])
if __name__ == '__main__':
unittest.main()