From 2c80f476fdf0f1cdf89f5836108594abe483ab9d Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Sun, 25 Oct 2026 20:20:32 +0100 Subject: [PATCH] perf(os): skip the package manager query when nothing changed The hourly operating system job ran the package manager's full update query every time: apt-get -s dist-upgrade, dnf check-update with updateinfo and rpm -qa, zypper list-patches/list-updates/locks. dnf and zypper load all repository metadata for this, which costs seconds of CPU and a lot of memory, although the answer can only change when the installed packages, the repository metadata or the configuration that selects or holds back updates changed. Each package manager now names those inputs: the package database (dpkg status, rpmdb, pacman's local directory, apk's installed file), the repository metadata (apt lists, the dnf and zypp caches, pacman's sync databases, apk indexes) and the sources, pinning and lock files. Every run records their modification times with the status. The hourly job reuses the previous pending updates, held packages and last upgrade when the times are identical, the same package manager on the same release produced a complete answer, and that answer is less than a day old; reboot, services, time sync and the other checks still run. Any path that cannot be checked, a failed or partial query, more than 500 updates, updates from the root helper and Arch with network access (checkupdates and arch-audit answer from the network) mean a full run. "Check now" and occ serverinfo:os-status --refresh always do one. First-seen times are still applied on every run, so how long an update has been pending keeps counting. The status records when the updates were last checked and whether they were reused, and the occ command shows the time. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- README.md | 18 +- lib/Commands/ShowOsStatus.php | 1 + lib/Jobs/UpdateOsStatus.php | 2 +- lib/OsStatus/OsStatus.php | 32 +++ lib/OsStatus/OsStatusCollector.php | 91 ++++++++- lib/OsStatus/Packages/Apk.php | 15 ++ lib/OsStatus/Packages/Apt.php | 22 +++ lib/OsStatus/Packages/Dnf.php | 25 +++ lib/OsStatus/Packages/IPackageManager.php | 11 ++ lib/OsStatus/Packages/Pacman.php | 20 ++ lib/OsStatus/Packages/ProbeHelper.php | 40 ++++ lib/OsStatus/Packages/Rpm.php | 12 ++ lib/OsStatus/Packages/Zypper.php | 25 +++ tests/lib/Commands/ShowOsStatusTest.php | 14 ++ tests/lib/Jobs/RefreshOsStatusTest.php | 2 +- tests/lib/Jobs/UpdateOsStatusTest.php | 2 +- tests/lib/OsStatus/OsStatusCollectorTest.php | 193 +++++++++++++++++++ tests/lib/OsStatus/UpdateInputsTest.php | 142 ++++++++++++++ 18 files changed, 653 insertions(+), 14 deletions(-) create mode 100644 tests/lib/OsStatus/UpdateInputsTest.php diff --git a/README.md b/README.md index bd3b1faf..d417637b 100644 --- a/README.md +++ b/README.md @@ -65,13 +65,27 @@ request for minutes. A status older than three job intervals (at least six hours), or background jobs that do not run from system cron, count as a warning, so an old all-clear is never shown as current. "Check now" on the page queues a check for the next cron run and shows the result when it is done; -only one check runs at a time. Admins can also run: +only one check runs at a time. + +The hourly job asks the package manager for pending updates only when +something they depend on changed: it compares the modification times of the +installed-package database, the repository metadata and the package manager's +sources, pinning and lock configuration with those of the last run. If none +changed, it keeps the last list of pending updates, held packages and last +upgrade, and checks everything else (reboot, services, time sync) again. The +package manager is still asked at least once a day, and whenever a file cannot +be checked, the last query failed or was incomplete, more than 500 updates are +pending, or the updates came from the root helper. On Arch this only applies +with `os_allow_network` off, because `checkupdates` and `arch-audit` answer +from the network. "Check now" and `--refresh` below always ask the package +manager. Admins can also run: ``` occ serverinfo:os-status [--refresh] [--output=json] ``` -The command shows how old the status is and exits with 0, 1, 2 or 3 for ok, +The command shows how old the status is and when the pending updates were +last checked (`updatesCheckedAt`), and exits with 0, 1, 2 or 3 for ok, warning, error and unknown, so it can be used as a monitoring check. Unknown means the status has not been collected yet, or nothing is wrong but the pending updates could not be determined. The same verdict appears as setup checks on Settings → Overview diff --git a/lib/Commands/ShowOsStatus.php b/lib/Commands/ShowOsStatus.php index bda8200e..4e78da27 100644 --- a/lib/Commands/ShowOsStatus.php +++ b/lib/Commands/ShowOsStatus.php @@ -90,6 +90,7 @@ public function __invoke( 'packageManager' => $status->packageManager ?? 'none', 'securityUpdates' => $securityUpdates === null ? 'unknown' : count($securityUpdates), 'updates' => $status->updates === null ? 'unknown' : count($status->updates) + $status->updatesOmitted, + 'updatesCheckedAt' => $status->updatesCheckedAt === null ? 'unknown' : date(DATE_ATOM, $status->updatesCheckedAt), 'updatesPendingDays' => is_int($pendingDays) ? $pendingDays : 'none', 'rebootRequired' => $status->rebootReasons === null ? 'unknown' : ($status->rebootReasons !== [] ? 'yes' : 'no'), 'helper' => $status->helper->state, diff --git a/lib/Jobs/UpdateOsStatus.php b/lib/Jobs/UpdateOsStatus.php index 07fdbc7f..e1aea66d 100644 --- a/lib/Jobs/UpdateOsStatus.php +++ b/lib/Jobs/UpdateOsStatus.php @@ -47,7 +47,7 @@ protected function run($argument): void { return; } - if ($this->collector->collectAndStore() !== null) { + if ($this->collector->collectAndStore(reuseUnchangedUpdates: true) !== null) { $this->notifications->sync(); } } diff --git a/lib/OsStatus/OsStatus.php b/lib/OsStatus/OsStatus.php index 64e72149..bd0c46c5 100644 --- a/lib/OsStatus/OsStatus.php +++ b/lib/OsStatus/OsStatus.php @@ -48,6 +48,7 @@ final class OsStatus { * @param list|null $failedUnits * @param array{all: ?string, security: ?string} $upgradeCommands * @param list $diagnostics + * @param array|null $updateInputs */ public function __construct( public readonly string $state, @@ -81,6 +82,12 @@ public function __construct( public readonly HelperState $helper = new HelperState(HelperState::STATE_ABSENT, HelperReportReader::DEFAULT_PATH), public readonly ?int $bootTime = null, public readonly array $diagnostics = [], + /** When the pending updates were last asked of the package manager or written by the helper */ + public readonly ?int $updatesCheckedAt = null, + /** Whether the pending updates were taken over from the previous run, because nothing they depend on changed */ + public readonly bool $updatesReused = false, + /** Modification times of what the pending updates depend on, set when a later run may reuse them */ + public readonly ?array $updateInputs = null, ) { } @@ -135,6 +142,9 @@ public function toArray(): array { 'helper' => $this->helper->toArray(), 'bootTime' => $this->bootTime, 'diagnostics' => $this->diagnostics, + 'updatesCheckedAt' => $this->updatesCheckedAt, + 'updatesReused' => $this->updatesReused, + 'updateInputs' => $this->updateInputs, ]; } @@ -186,9 +196,31 @@ public static function fromArray(mixed $data): ?self { HelperState::fromArray($data['helper'] ?? null, HelperReportReader::DEFAULT_PATH), Values::int($data['bootTime'] ?? null), self::stringList($data['diagnostics'] ?? null) ?? [], + Values::int($data['updatesCheckedAt'] ?? null), + ($data['updatesReused'] ?? false) === true, + self::modificationTimes($data['updateInputs'] ?? null), ); } + /** + * @return array|null null unless every entry is a path with a time + */ + private static function modificationTimes(mixed $items): ?array { + if (!is_array($items) || $items === []) { + return null; + } + + $times = []; + foreach ($items as $path => $time) { + if (!is_string($path) || !is_int($time)) { + return null; + } + $times[$path] = $time; + } + + return $times; + } + /** * @template T * @param callable(mixed): (T|null) $map diff --git a/lib/OsStatus/OsStatusCollector.php b/lib/OsStatus/OsStatusCollector.php index be842fa4..122a6979 100644 --- a/lib/OsStatus/OsStatusCollector.php +++ b/lib/OsStatus/OsStatusCollector.php @@ -24,6 +24,7 @@ use OCA\ServerInfo\OsStatus\Packages\PackageManagerFactory; use OCA\ServerInfo\OsStatus\Packages\PendingUpdate; use OCA\ServerInfo\OsStatus\Packages\ProbeException; +use OCA\ServerInfo\OsStatus\Packages\ProbeHelper; use OCA\ServerInfo\OsStatus\Reboot\CoreLibraryProbe; use OCA\ServerInfo\OsStatus\Reboot\KernelProbe; use OCA\ServerInfo\OsStatus\Reboot\RebootReason; @@ -53,6 +54,9 @@ class OsStatusCollector { /** Held while a collection runs, so package managers never run twice at once. */ public const LOCK = 'serverinfo/os-status-collection'; + /** Pending updates are asked of the package manager at least this often, even when nothing changed. */ + public const REUSE_UPDATES_FOR = 86400; + public function __construct( private IHost $host, private IAppConfig $appConfig, @@ -74,7 +78,13 @@ public function __construct( ) { } - public function collect(): OsStatus { + /** + * With $reuseUnchangedUpdates, the pending updates, held packages and + * last upgrade of the previous run are kept when nothing they depend on + * changed, which spares a scheduled run the package manager's query. + * Everything else is asked again. + */ + public function collect(bool $reuseUnchangedUpdates = false): OsStatus { $now = $this->timeFactory->getTime(); if ($this->appConfig->getAppValueString('restricted_mode', 'no') === 'yes') { @@ -125,11 +135,26 @@ public function collect(): OsStatus { $updates = null; $updatesSource = null; + $updatesCheckedAt = null; $needsRoot = null; + $inputs = null; + $reused = null; if ($packageManager !== null) { + // Taken before the query, so that a change while it runs shows on the next run. + $inputs = $this->probe('What the pending updates depend on', fn (): ?array => $this->updateInputs($packageManager), null, $diagnostics); + if ($reuseUnchangedUpdates && $inputs !== null && $release !== null) { + $reused = $this->reusableUpdates($packageManager, $release, $inputs, $now); + } + } + if ($reused !== null) { + $updates = $reused->updates; + $updatesSource = OsStatus::SOURCE_SYSTEM; + $updatesCheckedAt = $reused->updatesCheckedAt; + } elseif ($packageManager !== null) { try { $updates = $packageManager->getPendingUpdates(); $updatesSource = OsStatus::SOURCE_SYSTEM; + $updatesCheckedAt = $now; } catch (NeedsRootException $e) { $needsRoot = $e->getMessage(); } catch (ProbeException $e) { @@ -143,6 +168,7 @@ public function collect(): OsStatus { if ($updates === null && $reportedUpdates !== null && $environment->managesPackages()) { $updates = $reportedUpdates; $updatesSource = OsStatus::SOURCE_HELPER; + $updatesCheckedAt = $report?->generatedAt; } if ($needsRoot !== null && $updatesSource !== OsStatus::SOURCE_HELPER) { $diagnostics[] = $needsRoot; @@ -156,6 +182,7 @@ public function collect(): OsStatus { if ($updates === [] && $updatesSource === OsStatus::SOURCE_SYSTEM && $listsRefreshedAt === null && in_array($packageManager?->getId(), OsVerdict::LISTS_AGE_MATTERS, true)) { $updates = null; $updatesSource = null; + $updatesCheckedAt = null; $diagnostics[] = 'The package lists could not be found, so pending updates are unknown until the lists are refreshed.'; } @@ -164,7 +191,7 @@ public function collect(): OsStatus { : null; $restartServices = $environment->managesPackages() ? $report?->restartServices : null; - $classified = $packageManager?->classifiesSecurity() ?? ($updatesSource === OsStatus::SOURCE_HELPER); + $classified = $reused?->securityClassified ?? $packageManager?->classifiesSecurity() ?? ($updatesSource === OsStatus::SOURCE_HELPER); [$updates, $rebootReasons, $restartServices] = $this->probe( 'How long the findings have been pending', fn (): array => $this->applyFirstSeen($updates, $classified, $rebootReasons, $restartServices, $now), @@ -179,9 +206,11 @@ public function collect(): OsStatus { $updates = array_slice($updates, 0, self::MAX_LISTED_UPDATES); } - if ($packageManager instanceof IReportsDiagnostics) { - array_push($diagnostics, ...$packageManager->getDiagnostics()); - } + $packageDiagnostics = $reused === null && $packageManager instanceof IReportsDiagnostics ? $packageManager->getDiagnostics() : []; + array_push($diagnostics, ...$packageDiagnostics); + + // Only a complete answer of the package manager is worth reusing. + $reusableInputs = $updatesSource === OsStatus::SOURCE_SYSTEM && $packageDiagnostics === [] && $omitted === 0 ? $inputs : null; $timeSync = $this->probe('The time synchronisation', fn (): array => $this->timeSync->probe(), ['synchronized' => null, 'source' => null], $diagnostics); @@ -198,9 +227,9 @@ public function collect(): OsStatus { $updatesSource, $classified, $omitted, - $packageManager !== null ? $this->probe('The held packages', fn (): ?array => $packageManager->getHeldPackages(), null, $diagnostics) : null, + $reused?->held ?? ($packageManager !== null ? $this->probe('The held packages', fn (): ?array => $packageManager->getHeldPackages(), null, $diagnostics) : null), $listsRefreshedAt, - $packageManager !== null ? $this->probe('The last upgrade', fn (): ?int => $packageManager->getLastUpgradeAt(), null, $diagnostics) : null, + $reused?->lastUpgradeAt ?? ($packageManager !== null ? $this->probe('The last upgrade', fn (): ?int => $packageManager->getLastUpgradeAt(), null, $diagnostics) : null), $packageManager !== null ? $this->probe('The automatic updates', fn (): AutomaticUpdates => $packageManager->getAutomaticUpdates(), AutomaticUpdates::unknown(), $diagnostics) : AutomaticUpdates::unknown(), @@ -217,9 +246,51 @@ public function collect(): OsStatus { $helperState, $bootTime, $diagnostics, + $updatesCheckedAt, + $reused !== null, + $reusableInputs, ); } + /** + * @return array|null null when any of it cannot be checked + */ + private function updateInputs(IPackageManager $packageManager): ?array { + $paths = $packageManager->getUpdateInputs(); + $inputs = $paths === null ? null : ProbeHelper::fingerprint($this->host, $paths); + + return $inputs === [] ? null : $inputs; + } + + /** + * The previous status, when its pending updates still hold: the same + * package manager on the same release found them completely, less than + * REUSE_UPDATES_FOR ago, and nothing they depend on was touched since. + * + * @param array $inputs + */ + private function reusableUpdates(IPackageManager $packageManager, OsRelease $release, array $inputs, int $now): ?OsStatus { + $previous = $this->store->load(); + $checkedAt = $previous?->updatesCheckedAt; + if ($previous === null + || $previous->updateInputs === null + || $previous->updates === null + || $previous->updatesSource !== OsStatus::SOURCE_SYSTEM + || $previous->packageManager !== $packageManager->getId() + || $previous->release?->toArray() !== $release->toArray() + || $checkedAt === null + || $checkedAt > $now + || $now - $checkedAt >= self::REUSE_UPDATES_FOR) { + return null; + } + + $stored = $previous->updateInputs; + ksort($stored); + ksort($inputs); + + return $stored === $inputs ? $previous : null; + } + /** * Runs one probe, so that a probe that throws costs its own finding and * not the whole run. @@ -247,8 +318,10 @@ private function probe(string $finding, callable $probe, mixed $fallback, array * Collects and stores the result, logging rather than throwing so a * failure does not take the background job queue down with it. Returns * null without collecting while another collection runs. + * + * @param bool $reuseUnchangedUpdates as for collect() */ - public function collectAndStore(): ?OsStatus { + public function collectAndStore(bool $reuseUnchangedUpdates = false): ?OsStatus { try { $this->locks->acquireLock(self::LOCK, ILockingProvider::LOCK_EXCLUSIVE); } catch (LockedException) { @@ -258,7 +331,7 @@ public function collectAndStore(): ?OsStatus { } try { - $status = $this->collect(); + $status = $this->collect($reuseUnchangedUpdates); $this->store->save($status); return $status; diff --git a/lib/OsStatus/Packages/Apk.php b/lib/OsStatus/Packages/Apk.php index 7f4382b0..f3f23865 100644 --- a/lib/OsStatus/Packages/Apk.php +++ b/lib/OsStatus/Packages/Apk.php @@ -225,4 +225,19 @@ public function getUpgradeCommands(): array { public function getPackageDatabasePath(): ?string { return '/lib/apk/db/installed'; } + + /** + * The installed packages, the cached indexes, the repositories and the + * constraints in /etc/apk/world. + */ + #[\Override] + public function getUpdateInputs(): ?array { + return [ + '/lib/apk/db/installed', + '/var/cache/apk', + '/var/cache/apk/APKINDEX.*.tar.gz', + '/etc/apk/repositories', + self::WORLD, + ]; + } } diff --git a/lib/OsStatus/Packages/Apt.php b/lib/OsStatus/Packages/Apt.php index ed2c5063..5fab8bab 100644 --- a/lib/OsStatus/Packages/Apt.php +++ b/lib/OsStatus/Packages/Apt.php @@ -330,4 +330,26 @@ public function getUpgradeCommands(): array { public function getPackageDatabasePath(): ?string { return self::DPKG_STATUS; } + + /** + * dpkg rewrites its status file, which also records holds, on every + * change, and apt update moves each changed list into the lists + * directory. Sources, pinning and apt's configuration choose among them. + */ + #[\Override] + public function getUpdateInputs(): ?array { + return [ + self::DPKG_STATUS, + '/var/lib/dpkg/updates', + self::LISTS, + '/etc/apt/sources.list', + '/etc/apt/sources.list.d', + '/etc/apt/sources.list.d/*', + '/etc/apt/preferences', + '/etc/apt/preferences.d', + '/etc/apt/preferences.d/*', + '/etc/apt/apt.conf.d', + '/etc/apt/apt.conf.d/*', + ]; + } } diff --git a/lib/OsStatus/Packages/Dnf.php b/lib/OsStatus/Packages/Dnf.php index 147af5c4..a8fa3d43 100644 --- a/lib/OsStatus/Packages/Dnf.php +++ b/lib/OsStatus/Packages/Dnf.php @@ -451,6 +451,31 @@ public function getPackageDatabasePath(): ?string { return $this->rpm->getDatabasePath(); } + /** + * The RPM database, the system cache that check-update and updateinfo + * read with --cacheonly, the repositories and the version locks. + */ + #[\Override] + public function getUpdateInputs(): ?array { + $database = $this->rpm->getUpdateInputs(); + if ($database === null) { + return null; + } + + return [ + ...$database, + '/var/cache/dnf', + '/var/cache/dnf/*/repodata/repomd.xml', + '/var/cache/libdnf5', + '/var/cache/libdnf5/*/repodata/repomd.xml', + '/etc/dnf/dnf.conf', + '/etc/dnf/plugins/versionlock.list', + '/etc/dnf/versionlock.toml', + '/etc/yum.repos.d', + '/etc/yum.repos.d/*.repo', + ]; + } + /** * Told apart by the binaries rather than by running dnf, because running * dnf 5 as the web server is what has to be avoided. Where both are diff --git a/lib/OsStatus/Packages/IPackageManager.php b/lib/OsStatus/Packages/IPackageManager.php index 67eabd09..80958e25 100644 --- a/lib/OsStatus/Packages/IPackageManager.php +++ b/lib/OsStatus/Packages/IPackageManager.php @@ -83,4 +83,15 @@ public function getUpgradeCommands(): array; * tell that a cached status is out of date. */ public function getPackageDatabasePath(): ?string; + + /** + * Files, directories and glob patterns whose modification times change + * whenever getPendingUpdates() or getHeldPackages() could answer + * differently: the installed packages, the repository metadata and the + * configuration that selects or holds back updates. + * + * @return list|null null when the answer depends on more than + * local files, e.g. on the network + */ + public function getUpdateInputs(): ?array; } diff --git a/lib/OsStatus/Packages/Pacman.php b/lib/OsStatus/Packages/Pacman.php index 89edfb83..16e52d65 100644 --- a/lib/OsStatus/Packages/Pacman.php +++ b/lib/OsStatus/Packages/Pacman.php @@ -238,4 +238,24 @@ public function getUpgradeCommands(): array { public function getPackageDatabasePath(): ?string { return '/var/lib/pacman/local'; } + + /** + * Only without the network: checkupdates and arch-audit answer from + * what the mirrors and security.archlinux.org publish, which no local + * file shows. pacman -Qu reads the installed packages, whose directory + * gains and loses an entry per package, and root's sync databases. + */ + #[\Override] + public function getUpdateInputs(): ?array { + if ($this->networkAllowed() && ($this->host->binaryState('checkupdates') !== false || $this->host->binaryState('arch-audit') !== false)) { + return null; + } + + return [ + '/var/lib/pacman/local', + '/var/lib/pacman/sync', + '/var/lib/pacman/sync/*.db', + '/etc/pacman.conf', + ]; + } } diff --git a/lib/OsStatus/Packages/ProbeHelper.php b/lib/OsStatus/Packages/ProbeHelper.php index a4009823..dd6df0ed 100644 --- a/lib/OsStatus/Packages/ProbeHelper.php +++ b/lib/OsStatus/Packages/ProbeHelper.php @@ -15,6 +15,9 @@ * Small pieces the package managers share. */ final class ProbeHelper { + /** Files a fingerprint checks at most, to keep it a handful of stat calls */ + private const MAX_FINGERPRINT_PATHS = 512; + /** * The newest modification time among the files matching the patterns; * a pattern without a wildcard names a single file. @@ -35,6 +38,43 @@ public static function newestModified(IHost $host, array $patterns): ?int { return $newest; } + /** + * The modification time of every path, and of every file a glob pattern + * matches, so that comparing two fingerprints tells whether anything was + * written, added or removed. A path that does not exist counts as 0 when + * its directory can be seen. + * + * @param list $paths + * @return array|null null when any path cannot be checked, + * or a pattern matches too many files + */ + public static function fingerprint(IHost $host, array $paths): ?array { + $fingerprint = []; + foreach ($paths as $path) { + if (str_contains($path, '*')) { + $matches = $host->glob($path); + if (count($fingerprint) + count($matches) > self::MAX_FINGERPRINT_PATHS) { + return null; + } + } else { + $matches = [$path]; + } + + foreach ($matches as $file) { + $time = $host->modifiedAt($file); + if ($time === null) { + if ($host->fileExists($file) || $host->modifiedAt(dirname($file)) === null) { + return null; + } + $time = 0; + } + $fingerprint[$file] = $time; + } + } + + return $fingerprint; + } + /** * The first line of a command's output, short enough for a diagnostic. */ diff --git a/lib/OsStatus/Packages/Rpm.php b/lib/OsStatus/Packages/Rpm.php index 67ad3ba3..66528fa9 100644 --- a/lib/OsStatus/Packages/Rpm.php +++ b/lib/OsStatus/Packages/Rpm.php @@ -52,6 +52,18 @@ public function getDatabasePath(): ?string { return null; } + /** + * The database and its directory, where the sqlite backend keeps its + * write-ahead log until it is merged back. + * + * @return list|null null when the database cannot be found + */ + public function getUpdateInputs(): ?array { + $path = $this->getDatabasePath(); + + return $path === null ? null : [$path, dirname($path)]; + } + /** * Installed version (with epoch when it has one) by package name. * diff --git a/lib/OsStatus/Packages/Zypper.php b/lib/OsStatus/Packages/Zypper.php index bdbe9a20..09cbd88b 100644 --- a/lib/OsStatus/Packages/Zypper.php +++ b/lib/OsStatus/Packages/Zypper.php @@ -461,4 +461,29 @@ public function getUpgradeCommands(): array { public function getPackageDatabasePath(): ?string { return $this->rpm->getDatabasePath(); } + + /** + * The RPM database, the solv cache that zypper reads with --no-refresh, + * the repositories and the package locks. + */ + #[\Override] + public function getUpdateInputs(): ?array { + $database = $this->rpm->getUpdateInputs(); + if ($database === null) { + return null; + } + + return [ + ...$database, + '/var/cache/zypp/solv', + '/var/cache/zypp/solv/*/cookie', + '/var/cache/zypp/solv/*/solv', + '/var/cache/zypp/raw', + '/var/cache/zypp/raw/*/repodata/repomd.xml', + '/etc/zypp/zypp.conf', + '/etc/zypp/locks', + '/etc/zypp/repos.d', + '/etc/zypp/repos.d/*.repo', + ]; + } } diff --git a/tests/lib/Commands/ShowOsStatusTest.php b/tests/lib/Commands/ShowOsStatusTest.php index 9bf60796..eccd65f2 100644 --- a/tests/lib/Commands/ShowOsStatusTest.php +++ b/tests/lib/Commands/ShowOsStatusTest.php @@ -63,6 +63,19 @@ public function testRefreshDoesNotWaitForARunningCollection(): void { $this->assertSame(ShowOsStatus::EXIT_UNKNOWN, $this->execute($this->createMock(OsStatusService::class), $written, true, $collector)); } + public function testRefreshAsksThePackageManagerAndShowsWhen(): void { + $status = new OsStatus(OsStatus::STATE_OK, 1792411200, updates: [], updatesCheckedAt: 1792407600, updatesReused: true); + $collector = $this->createMock(OsStatusCollector::class); + $collector->expects($this->once())->method('collectAndStore')->with(false)->willReturn($status); + $service = $this->createMock(OsStatusService::class); + $service->method('getStatus')->willReturn($status); + + $written = []; + $this->execute($service, $written, true, $collector); + + $this->assertSame(date(DATE_ATOM, 1792407600), $written['updatesCheckedAt'] ?? null); + } + public function testAStaleStatusWarnsAndShowsItsAge(): void { $status = new OsStatus(OsStatus::STATE_OK, 1792411200 - 3 * 86400 - 7200); $service = $this->createMock(OsStatusService::class); @@ -72,6 +85,7 @@ public function testAStaleStatusWarnsAndShowsItsAge(): void { $written = []; $this->assertSame(ShowOsStatus::EXIT_WARNING, $this->execute($service, $written)); $this->assertSame('3 d 2 h', $written['statusAge'] ?? null); + $this->assertSame('unknown', $written['updatesCheckedAt'] ?? null); } public static function dataAges(): array { diff --git a/tests/lib/Jobs/RefreshOsStatusTest.php b/tests/lib/Jobs/RefreshOsStatusTest.php index 7bdd5dbd..58e5fa1a 100644 --- a/tests/lib/Jobs/RefreshOsStatusTest.php +++ b/tests/lib/Jobs/RefreshOsStatusTest.php @@ -32,7 +32,7 @@ public function testCollectsOnlyFromSystemCron(string $mode, ?OsStatus $result, $appConfig = $this->createMock(IAppConfig::class); $appConfig->method('getValueString')->with('core', 'backgroundjobs_mode', 'ajax')->willReturn($mode); $collector = $this->createMock(OsStatusCollector::class); - $collector->expects($this->exactly($collections))->method('collectAndStore')->willReturn($result); + $collector->expects($this->exactly($collections))->method('collectAndStore')->with(false)->willReturn($result); $notifications = $this->createMock(OsNotificationSender::class); $notifications->expects($this->exactly($syncs))->method('sync'); diff --git a/tests/lib/Jobs/UpdateOsStatusTest.php b/tests/lib/Jobs/UpdateOsStatusTest.php index 6ba1db12..2258868b 100644 --- a/tests/lib/Jobs/UpdateOsStatusTest.php +++ b/tests/lib/Jobs/UpdateOsStatusTest.php @@ -32,7 +32,7 @@ public function testRunsOnlyFromSystemCron(string $mode, int $runs): void { $appConfig->method('getValueInt')->willReturn(3600); $appConfig->method('getValueString')->with('core', 'backgroundjobs_mode', 'ajax')->willReturn($mode); $collector = $this->createMock(OsStatusCollector::class); - $collector->expects($this->exactly($runs))->method('collectAndStore')->willReturn(OsStatus::restricted(1792411200)); + $collector->expects($this->exactly($runs))->method('collectAndStore')->with(true)->willReturn(OsStatus::restricted(1792411200)); $notifications = $this->createMock(OsNotificationSender::class); $notifications->expects($this->exactly($runs))->method('sync'); diff --git a/tests/lib/OsStatus/OsStatusCollectorTest.php b/tests/lib/OsStatus/OsStatusCollectorTest.php index 64021509..f3fd4d3e 100644 --- a/tests/lib/OsStatus/OsStatusCollectorTest.php +++ b/tests/lib/OsStatus/OsStatusCollectorTest.php @@ -36,12 +36,14 @@ use OCP\AppFramework\Services\IAppConfig; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\IAppData; +use OCP\Files\NotFoundException; use OCP\Files\SimpleFS\ISimpleFile; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\ICache; use OCP\ICacheFactory; use OCP\Lock\ILockingProvider; use OCP\Lock\LockedException; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -112,7 +114,14 @@ private function collector(int $now = self::NOW): OsStatusCollector { * App data that keeps the stored status in $this->stored. */ private function appData(): IAppData { + $file = $this->createMock(ISimpleFile::class); + $file->method('getContent')->willReturnCallback(fn (): string => $this->stored ?? throw new NotFoundException()); + $file->method('putContent')->willReturnCallback(function (mixed $content): void { + $this->stored = (string)$content; + }); $folder = $this->createMock(ISimpleFolder::class); + $folder->method('fileExists')->willReturnCallback(fn (): bool => $this->stored !== null); + $folder->method('getFile')->willReturnCallback(fn (): ISimpleFile => $this->stored !== null ? $file : throw new NotFoundException()); $folder->method('newFile')->willReturnCallback(function (string $name, mixed $content = null): ISimpleFile { $this->stored = (string)$content; @@ -399,4 +408,188 @@ public function testCollectAndStoreSkipsWhileAnotherCollectionRuns(): void { $this->assertSame([], $this->host->ran); $this->assertNull($this->stored, 'Nothing was saved'); } + + private const APT_SIMULATION = 'apt-get -s -o Debug::NoLocking=1 dist-upgrade'; + + /** + * An Ubuntu host whose package state, lists and apt configuration have + * known modification times. + */ + private function withAptHost(): void { + $this->withPendingUpdates(); + $this->host->withCommand('apt-mark', ['showhold'], "nginx\n") + ->withCommand('timedatectl', ['show', '--property=NTPSynchronized', '--value'], "yes\n"); + foreach ([ + '/var/lib/dpkg/status' => self::NOW - 7200, + '/var/lib/dpkg/updates' => self::NOW - 7200, + '/var/lib/apt/lists' => self::NOW - 3600, + '/var/lib/apt/periodic/update-success-stamp' => self::NOW - 3600, + '/etc/apt' => self::NOW - 86400, + '/etc/apt/sources.list.d' => self::NOW - 86400, + '/etc/apt/apt.conf.d' => self::NOW - 86400, + ] as $path => $time) { + $this->host->modified[$path] = $time; + } + } + + private function scheduledRun(int $now): OsStatus { + $this->host->ran = []; + $status = $this->collector($now)->collectAndStore(reuseUnchangedUpdates: true); + $this->assertNotNull($status); + + return $status; + } + + public function testUnchangedInputsReuseThePendingUpdates(): void { + $this->withAptHost(); + $first = $this->scheduledRun(self::NOW); + $this->assertContains(self::APT_SIMULATION, $this->host->ran); + $this->assertFalse($first->updatesReused); + $this->assertSame(self::NOW, $first->updatesCheckedAt); + $this->assertSame(self::NOW - 7200, $first->updateInputs['/var/lib/dpkg/status'] ?? null); + $this->assertSame(0, $first->updateInputs['/etc/apt/preferences'] ?? null, 'A missing file counts as absent'); + + $this->host->withFile('/run/reboot-required', "*** System restart required ***\n", self::NOW + 1800); + $second = $this->scheduledRun(self::NOW + 3600); + + $this->assertNotContains(self::APT_SIMULATION, $this->host->ran); + $this->assertNotContains('apt-mark showhold', $this->host->ran); + $this->assertContains('timedatectl show --property=NTPSynchronized --value', $this->host->ran, 'The cheap checks still run'); + $this->assertTrue($second->updatesReused); + $this->assertSame(self::NOW, $second->updatesCheckedAt); + $this->assertSame(self::NOW + 3600, $second->collectedAt); + $this->assertSame(['libssl3t64', 'tzdata'], array_map(static fn ($u) => $u->name, $second->updates ?? [])); + $this->assertTrue($second->securityClassified); + $this->assertSame('nginx', $second->held[0]->name ?? null); + $this->assertSame(self::NOW, $second->updates[0]->since, 'First seen still counts from the first run'); + $this->assertSame(RebootReason::KIND_DISTRIBUTION, $second->rebootReasons[0]->kind ?? null, 'Reboots are checked again'); + $this->assertSame($first->updateInputs, $second->updateInputs); + + $third = $this->scheduledRun(self::NOW + 7200); + $this->assertTrue($third->updatesReused); + $this->assertSame(self::NOW, $third->updatesCheckedAt, 'Reuse does not move the time of the last real check'); + } + + public static function dataChanges(): array { + return [ + 'packages installed' => [static function (FakeHost $host): void { + $host->modified['/var/lib/dpkg/status'] = self::NOW + 600; + }], + 'lists refreshed' => [static function (FakeHost $host): void { + $host->modified['/var/lib/apt/lists'] = self::NOW + 600; + }], + 'source added' => [static function (FakeHost $host): void { + $host->globs['/etc/apt/sources.list.d/*'] = ['/etc/apt/sources.list.d/extra.sources']; + $host->modified['/etc/apt/sources.list.d/extra.sources'] = self::NOW - 86400; + }], + 'pinning added' => [static function (FakeHost $host): void { + $host->modified['/etc/apt/preferences'] = self::NOW + 600; + }], + 'lists hidden' => [static function (FakeHost $host): void { + unset($host->modified['/var/lib/apt/lists']); + $host->files['/var/lib/apt/lists'] = ''; + }], + 'release upgraded' => [static function (FakeHost $host): void { + $host->withFile('/etc/os-release', str_replace('24.04.3', '24.04.4', FakeHost::fixture('ubuntu-24.04-os-release'))); + }], + ]; + } + + /** + * @param \Closure(FakeHost): void $change + */ + #[DataProvider('dataChanges')] + public function testAChangeAsksThePackageManagerAgain(\Closure $change): void { + $this->withAptHost(); + $this->scheduledRun(self::NOW); + + $change($this->host); + $status = $this->scheduledRun(self::NOW + 3600); + + $this->assertContains(self::APT_SIMULATION, $this->host->ran); + $this->assertContains('apt-mark showhold', $this->host->ran); + $this->assertFalse($status->updatesReused); + $this->assertSame(self::NOW + 3600, $status->updatesCheckedAt); + } + + public function testThePackageManagerIsAskedAtLeastDaily(): void { + $this->withAptHost(); + $this->scheduledRun(self::NOW); + + $this->assertTrue($this->scheduledRun(self::NOW + OsStatusCollector::REUSE_UPDATES_FOR - 1)->updatesReused); + $status = $this->scheduledRun(self::NOW + OsStatusCollector::REUSE_UPDATES_FOR); + + $this->assertContains(self::APT_SIMULATION, $this->host->ran); + $this->assertFalse($status->updatesReused); + } + + public function testAFailedQueryIsNotReused(): void { + $this->withAptHost(); + $this->host->withCommand('apt-get', ['-s', '-o', 'Debug::NoLocking=1', 'dist-upgrade'], '', 100, "E: Could not get lock\n"); + $failed = $this->scheduledRun(self::NOW); + $this->assertNull($failed->updates); + $this->assertNull($failed->updateInputs); + + $this->withPendingUpdates(); + $status = $this->scheduledRun(self::NOW + 3600); + + $this->assertContains(self::APT_SIMULATION, $this->host->ran); + $this->assertCount(2, $status->updates ?? []); + } + + public function testAnIncompleteAnswerIsNotReused(): void { + $this->host->withFile('/etc/os-release', "NAME=\"AlmaLinux\"\nID=\"almalinux\"\nID_LIKE=\"rhel centos fedora\"\nVERSION_ID=\"9.8\"\nPRETTY_NAME=\"AlmaLinux 9.8\"\n"); + $this->host->binaries = ['dnf', 'dnf-3']; + $this->host->globs['/var/cache/dnf/*-updateinfo.solvx'] = ['/var/cache/dnf/baseos-updateinfo.solvx']; + foreach (['/var/cache/dnf/last_makecache', '/var/lib/rpm/rpmdb.sqlite', '/var/lib/rpm', '/var/cache', '/var/cache/dnf', '/etc/dnf', '/etc/dnf/plugins', '/etc/yum.repos.d'] as $path) { + $this->host->modified[$path] = self::NOW - 600; + } + $this->host->withCommand('dnf-3', ['-q', '--cacheonly', 'check-update'], FakeHost::fixture('alma-9-dnf-check-update-pending'), 100) + ->withCommand('dnf-3', ['-q', '--cacheonly', 'updateinfo', 'list', '--security', '--available'], '', 1, "Error: something broke\n"); + + $first = $this->scheduledRun(self::NOW); + $this->assertCount(9, $first->updates ?? []); + $this->assertNull($first->updateInputs, 'The advisories are missing'); + + $this->scheduledRun(self::NOW + 3600); + $this->assertContains('dnf-3 -q --cacheonly check-update', $this->host->ran); + } + + public function testUpdatesFromTheHelperAreNotReused(): void { + $this->withRootsHelperReportPath(); + $this->host->withFile(HelperReportReader::DEFAULT_PATH, (string)json_encode([ + 'schema' => 1, 'helperVersion' => '1.0.0', 'generatedAt' => self::NOW - 600, 'hostname' => 'cloud.example.org', + 'bootTime' => self::BOOT, + 'pendingUpdates' => [['name' => 'openssl', 'installed' => '1', 'candidate' => '2', 'security' => true]], + ])); + + $status = $this->scheduledRun(self::NOW); + + $this->assertSame(OsStatus::SOURCE_HELPER, $status->updatesSource); + $this->assertSame(self::NOW - 600, $status->updatesCheckedAt, 'When the helper asked'); + $this->assertNull($status->updateInputs); + } + + public function testCheckNowAlwaysAsksThePackageManager(): void { + $this->withAptHost(); + $this->scheduledRun(self::NOW); + + $this->host->ran = []; + $status = $this->collector(self::NOW + 3600)->collectAndStore(); + + $this->assertContains(self::APT_SIMULATION, $this->host->ran); + $this->assertFalse($status?->updatesReused); + $this->assertSame(self::NOW + 3600, $status?->updatesCheckedAt); + } + + public function testTheReuseFieldsSurviveStorage(): void { + $status = new OsStatus(OsStatus::STATE_OK, self::NOW, updatesCheckedAt: self::NOW - 60, updatesReused: true, updateInputs: ['/var/lib/dpkg/status' => self::NOW - 600]); + + $restored = OsStatus::fromArray(json_decode((string)json_encode($status->toArray()), true)); + + $this->assertSame(self::NOW - 60, $restored?->updatesCheckedAt); + $this->assertTrue($restored?->updatesReused); + $this->assertSame(['/var/lib/dpkg/status' => self::NOW - 600], $restored?->updateInputs); + $this->assertNull(OsStatus::fromArray(['updateInputs' => ['/x' => 'soon']] + $status->toArray())?->updateInputs, 'A damaged fingerprint is no fingerprint'); + } } diff --git a/tests/lib/OsStatus/UpdateInputsTest.php b/tests/lib/OsStatus/UpdateInputsTest.php new file mode 100644 index 00000000..162f5ebb --- /dev/null +++ b/tests/lib/OsStatus/UpdateInputsTest.php @@ -0,0 +1,142 @@ +modified['/var/lib/dpkg/status'] = self::NOW - 60; + $host->modified['/etc/apt/sources.list.d'] = self::NOW - 600; + $host->modified['/etc/apt/sources.list.d/a.sources'] = self::NOW - 700; + $host->modified['/etc/apt/sources.list.d/b.sources'] = self::NOW - 800; + $host->globs['/etc/apt/sources.list.d/*'] = ['/etc/apt/sources.list.d/a.sources', '/etc/apt/sources.list.d/b.sources']; + + $this->assertSame([ + '/var/lib/dpkg/status' => self::NOW - 60, + '/etc/apt/sources.list.d' => self::NOW - 600, + '/etc/apt/sources.list.d/a.sources' => self::NOW - 700, + '/etc/apt/sources.list.d/b.sources' => self::NOW - 800, + ], ProbeHelper::fingerprint($host, ['/var/lib/dpkg/status', '/etc/apt/sources.list.d', '/etc/apt/sources.list.d/*'])); + } + + public function testAMissingFileInAVisibleDirectoryCountsAsAbsent(): void { + $host = new FakeHost(); + $host->modified['/etc/apt'] = self::NOW - 600; + + $this->assertSame(['/etc/apt/preferences' => 0], ProbeHelper::fingerprint($host, ['/etc/apt/preferences'])); + } + + public function testAPathThatCannotBeCheckedLeavesNoFingerprint(): void { + $host = new FakeHost(); + $host->modified['/var/lib/dpkg/status'] = self::NOW - 60; + + $this->assertNull(ProbeHelper::fingerprint($host, ['/var/lib/dpkg/status', '/var/lib/apt/lists']), 'Its directory is hidden too'); + + $host->files['/var/lib/apt/lists'] = ''; + $host->modified['/var/lib'] = self::NOW; + $this->assertNull(ProbeHelper::fingerprint($host, ['/var/lib/dpkg/status', '/var/lib/apt/lists']), 'It exists, but its time is unknown'); + } + + public function testTooManyMatchesLeaveNoFingerprint(): void { + $host = new FakeHost(); + $files = array_map(static fn (int $i): string => '/var/cache/dnf/repo' . $i . '/repodata/repomd.xml', range(1, 513)); + $host->globs['/var/cache/dnf/*/repodata/repomd.xml'] = $files; + foreach ($files as $file) { + $host->modified[$file] = self::NOW; + } + + $this->assertNull(ProbeHelper::fingerprint($host, ['/var/cache/dnf/*/repodata/repomd.xml'])); + } + + public function testAptWatchesTheDpkgDatabaseTheListsAndItsConfiguration(): void { + $inputs = (new Apt(new FakeHost(), new SystemdProbe(new FakeHost())))->getUpdateInputs() ?? []; + + foreach (['/var/lib/dpkg/status', '/var/lib/apt/lists', '/etc/apt/sources.list.d', '/etc/apt/preferences.d', '/etc/apt/apt.conf.d'] as $path) { + $this->assertContains($path, $inputs); + } + } + + public function testRpmBasedManagersWatchTheRpmDatabaseAndTheirCache(): void { + $host = new FakeHost(); + $host->modified['/var/lib/rpm/rpmdb.sqlite'] = self::NOW; + $rpm = new Rpm($host); + $systemd = new SystemdProbe($host); + + $dnf = (new Dnf($host, $rpm, $systemd))->getUpdateInputs() ?? []; + $zypper = (new Zypper($host, $rpm, $systemd))->getUpdateInputs() ?? []; + + foreach (['/var/lib/rpm/rpmdb.sqlite', '/var/lib/rpm', '/var/cache/dnf', '/var/cache/dnf/*/repodata/repomd.xml', '/etc/yum.repos.d', '/etc/dnf/plugins/versionlock.list'] as $path) { + $this->assertContains($path, $dnf); + } + foreach (['/var/lib/rpm/rpmdb.sqlite', '/var/lib/rpm', '/var/cache/zypp/solv', '/etc/zypp/repos.d', '/etc/zypp/locks'] as $path) { + $this->assertContains($path, $zypper); + } + } + + public function testWithoutAnRpmDatabaseNothingIsWatched(): void { + $host = new FakeHost(); + $rpm = new Rpm($host); + $systemd = new SystemdProbe($host); + + $this->assertNull((new Dnf($host, $rpm, $systemd))->getUpdateInputs()); + $this->assertNull((new Zypper($host, $rpm, $systemd))->getUpdateInputs()); + } + + public function testApkWatchesTheInstalledDatabaseTheIndexesAndTheWorld(): void { + $inputs = (new Apk(new FakeHost()))->getUpdateInputs() ?? []; + + foreach (['/lib/apk/db/installed', '/var/cache/apk', '/var/cache/apk/APKINDEX.*.tar.gz', '/etc/apk/repositories', '/etc/apk/world'] as $path) { + $this->assertContains($path, $inputs); + } + } + + public static function dataPacman(): array { + return [ + 'checkupdates online' => [true, ['checkupdates'], false], + 'arch-audit online' => [true, ['arch-audit'], false], + 'neither installed' => [true, [], true], + 'offline' => [false, ['checkupdates', 'arch-audit'], true], + ]; + } + + /** + * @param list $binaries + */ + #[DataProvider('dataPacman')] + public function testPacmanIsWatchedOnlyWhenItAnswersFromLocalFiles(bool $network, array $binaries, bool $watched): void { + $host = new FakeHost(); + $host->binaries = ['pacman', ...$binaries]; + $appConfig = $this->createMock(IAppConfig::class); + $appConfig->method('getAppValueBool')->willReturnMap([[ConfigLexicon::OS_ALLOW_NETWORK, true, true, $network]]); + + $inputs = (new Pacman($host, $this->createMock(ITimeFactory::class), $appConfig))->getUpdateInputs(); + + if ($watched) { + $this->assertSame(['/var/lib/pacman/local', '/var/lib/pacman/sync', '/var/lib/pacman/sync/*.db', '/etc/pacman.conf'], $inputs); + } else { + $this->assertNull($inputs); + } + } +}