From cfdd2749794c1d5beb5fea7c0b22f9d0f5175847 Mon Sep 17 00:00:00 2001 From: Stephan de Wit Date: Wed, 22 Jan 2025 09:51:04 +0100 Subject: [PATCH] system: optimize system status collection - split up the logic into class collection and status collection so that out of scope objects don't need to check their status. - with the previous, status dismissal doesn't need a status check either anymore - remove the UI delay from head.inc as well - scale up the disk space status thresholds a bit for systems with lower assigned disk space - non-persistent status objects without a location had their pointer-events removed, making it non-dismissable --- .../OPNsense/System/AbstractStatus.php | 5 ++ .../System/Status/CrashReporterStatus.php | 9 ++-- .../System/Status/DiskSpaceStatus.php | 10 ++-- .../OPNsense/System/Status/FirewallStatus.php | 3 ++ .../System/Status/LiveMediaStatus.php | 3 ++ .../System/Status/OpensshOverrideStatus.php | 3 ++ .../System/Status/SystemBootingStatus.php | 3 ++ .../library/OPNsense/System/SystemStatus.php | 52 +++++++++---------- src/opnsense/scripts/system/status.php | 2 +- src/opnsense/www/js/opnsense_status.js | 2 +- src/www/head.inc | 5 +- 11 files changed, 56 insertions(+), 41 deletions(-) diff --git a/src/opnsense/mvc/app/library/OPNsense/System/AbstractStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/AbstractStatus.php index 8d532c370..f255743f2 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/AbstractStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/AbstractStatus.php @@ -88,6 +88,11 @@ abstract class AbstractStatus return $this->internalScope; } + public function collectStatus() + { + /* To be overridden by the child status classes */ + } + public function dismissStatus() { /* To be overridden by the child status classes */ diff --git a/src/opnsense/mvc/app/library/OPNsense/System/Status/CrashReporterStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/Status/CrashReporterStatus.php index e75cc8275..c744e5817 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/Status/CrashReporterStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/Status/CrashReporterStatus.php @@ -36,12 +36,15 @@ class CrashReporterStatus extends AbstractStatus { public function __construct() { - $src_logs = array_merge(glob('/var/crash/textdump*'), glob('/var/crash/vmcore*')); - $php_log = '/tmp/PHP_errors.log'; - $this->internalPriority = 10; $this->internalTitle = gettext('Crash Reporter'); $this->internalLocation = '/crash_reporter.php'; + } + + public function collectStatus() + { + $src_logs = array_merge(glob('/var/crash/textdump*'), glob('/var/crash/vmcore*')); + $php_log = '/tmp/PHP_errors.log'; $src_errors = count($src_logs) > 0; if ($src_errors) { diff --git a/src/opnsense/mvc/app/library/OPNsense/System/Status/DiskSpaceStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/Status/DiskSpaceStatus.php index 97440c905..63263a84a 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/Status/DiskSpaceStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/Status/DiskSpaceStatus.php @@ -37,9 +37,12 @@ class DiskSpaceStatus extends AbstractStatus public function __construct() { $this->internalPriority = 5; - $this->internalPersistent = false; + $this->internalPersistent = true; $this->internalTitle = gettext('Disk Space'); + } + public function collectStatus() + { $backend = new Backend(); $output = json_decode($backend->configdRun('system diag disk'), true); @@ -54,8 +57,8 @@ class DiskSpaceStatus extends AbstractStatus $usedPercent = intval($filesystem['used-percent']); $totalSpace = $used + $available; - $warningThresholdGB = min(10, 0.15 * $totalSpace); - $errorThresholdGB = min(5, 0.5 * $totalSpace); + $warningThresholdGB = min(10, 0.2 * $totalSpace); + $errorThresholdGB = min(5, 0.1 * $totalSpace); if ($available <= $warningThresholdGB && $available > $errorThresholdGB) { $this->internalStatus = SystemStatusCode::WARNING; @@ -67,7 +70,6 @@ class DiskSpaceStatus extends AbstractStatus $available ); } elseif ($available <= $errorThresholdGB) { - $this->internalPersistent = true; $this->internalStatus = SystemStatusCode::ERROR; $this->internalMessage = sprintf( gettext('Disk space on the root filesystem is critically full (' . diff --git a/src/opnsense/mvc/app/library/OPNsense/System/Status/FirewallStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/Status/FirewallStatus.php index 8743f9d93..b7b5e142f 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/Status/FirewallStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/Status/FirewallStatus.php @@ -40,7 +40,10 @@ class FirewallStatus extends AbstractStatus $this->internalPriority = 20; $this->internalTitle = gettext('Firewall'); $this->internalLocation = '/ui/diagnostics/log/core/firewall'; + } + public function collectStatus() + { if (file_exists($this->rules_error)) { $this->internalMessage = file_get_contents($this->rules_error); /* XXX */ $this->internalStatus = SystemStatusCode::ERROR; diff --git a/src/opnsense/mvc/app/library/OPNsense/System/Status/LiveMediaStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/Status/LiveMediaStatus.php index 37075c7b0..858f42738 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/Status/LiveMediaStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/Status/LiveMediaStatus.php @@ -40,7 +40,10 @@ class LiveMediaStatus extends AbstractStatus $this->internalPersistent = true; $this->internalIsBanner = true; $this->internalTitle = gettext('Live Media'); + } + public function collectStatus() + { /* * Despite unionfs underneath, / is still not writeable, * making the following the perfect test for install media. diff --git a/src/opnsense/mvc/app/library/OPNsense/System/Status/OpensshOverrideStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/Status/OpensshOverrideStatus.php index dd17ddee2..3dd71483d 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/Status/OpensshOverrideStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/Status/OpensshOverrideStatus.php @@ -40,7 +40,10 @@ class OpensshOverrideStatus extends AbstractStatus $this->internalTitle = gettext('OpenSSH config override'); $this->internalIsBanner = true; $this->internalScope[] = '/system_advanced_admin.php'; + } + public function collectStatus() + { if (count(glob('/usr/local/etc/ssh/sshd_config.d/*.conf'))) { $this->internalMessage = gettext('The OpenSSH GUI configuration may be overridden by currently provided files on the disk.'); $this->internalStatus = SystemStatusCode::NOTICE; diff --git a/src/opnsense/mvc/app/library/OPNsense/System/Status/SystemBootingStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/Status/SystemBootingStatus.php index ee6c069be..999fac4f5 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/Status/SystemBootingStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/Status/SystemBootingStatus.php @@ -39,7 +39,10 @@ class SystemBootingStatus extends AbstractStatus $this->internalPersistent = true; $this->internalIsBanner = true; $this->internalTitle = gettext('System Booting'); + } + public function collectStatus() + { /* XXX boot detection from final class product in config.inc */ $fp = fopen('/var/run/booting', 'a+e'); if ($fp) { diff --git a/src/opnsense/mvc/app/library/OPNsense/System/SystemStatus.php b/src/opnsense/mvc/app/library/OPNsense/System/SystemStatus.php index e34f9faa1..dff6d7b79 100644 --- a/src/opnsense/mvc/app/library/OPNsense/System/SystemStatus.php +++ b/src/opnsense/mvc/app/library/OPNsense/System/SystemStatus.php @@ -35,20 +35,13 @@ namespace OPNsense\System; */ class SystemStatus { - private $objectMap = []; - private $status = []; - - public function __construct() - { - $this->status = $this->collectStatus(); - } /** * @throws \Exception */ - private function collectStatus() + private function collectClasses() { - $result = []; + $objectMap = []; $all = glob(__DIR__ . '/Status/*.php'); $classes = array_map(function ($file) { if (strpos($file, 'Status') !== false) { @@ -64,13 +57,31 @@ class SystemStatus $obj = new $statusClass(); $reflect = new \ReflectionClass($obj); $shortName = strtolower(str_replace('Status', '', $reflect->getShortName())); - $this->objectMap[$shortName] = $obj; if ($shortName == 'System') { /* reserved for front-end usage */ throw new \Exception("SystemStatus classname is reserved"); } + $objectMap[$shortName] = $obj; + } + + return $objectMap; + } + + public function collectStatus($scope = null) + { + $result = []; + $objectMap = $this->collectClasses(); + foreach ($objectMap as $shortName => $obj) { + $objScope = $obj->getScope(); + if (!empty($objScope) && !$this->matchPath($scope, $objScope)) { + /* don't probe if unnecessary */ + continue; + } + + $obj->collectStatus(); + if ($obj->getStatus() == SystemStatusCode::OK) { continue; } @@ -91,26 +102,11 @@ class SystemStatus return $result; } - /** - * @return array An array containing a parseable format of every status object - */ - public function getSystemStatus($scope = null) - { - if ($scope !== null) { - $filteredStatuses = array_filter($this->status, function ($item) use ($scope) { - return empty($item['scope']) || $this->matchPath($scope, $item['scope']); - }); - - return $filteredStatuses; - } - - return $this->status; - } - public function dismissStatus($subsystem) { - if (array_key_exists($subsystem, $this->objectMap)) { - $this->objectMap[$subsystem]->dismissStatus(); + $objectMap = $this->collectClasses(); + if (array_key_exists($subsystem, $objectMap) && !$objectMap[$subsystem]->getPersistent()) { + $objectMap[$subsystem]->dismissStatus(); } } diff --git a/src/opnsense/scripts/system/status.php b/src/opnsense/scripts/system/status.php index 39dd794b1..376f42fc4 100755 --- a/src/opnsense/scripts/system/status.php +++ b/src/opnsense/scripts/system/status.php @@ -34,5 +34,5 @@ $status = new \OPNsense\System\SystemStatus(); if (isset($argv[1]) && $argv[1] == 'dismiss' && isset($argv[2])) { $status->dismissStatus($argv[2]); } else { - echo json_encode($status->getSystemStatus($argv[1] ?? null)) . PHP_EOL; + echo json_encode($status->collectStatus($argv[1] ?? null)) . PHP_EOL; } diff --git a/src/opnsense/www/js/opnsense_status.js b/src/opnsense/www/js/opnsense_status.js index 8ed031049..f9fef0e49 100644 --- a/src/opnsense/www/js/opnsense_status.js +++ b/src/opnsense/www/js/opnsense_status.js @@ -164,7 +164,7 @@ class StatusDialog { } let ref = subject.location != null ? `href="${subject.location}"` : ''; - let hoverStyle = subject.location == null ? 'cursor: default; pointer-events: none;' : ''; + let hoverStyle = (subject.location == null && subject.persistent) ? 'cursor: default; pointer-events: none;' : ''; let $closeBtn = `