Update maintenance:rrd-step to work with rrdcached (librenms#18623)

murrant · web-flow · commit 7f6daace9eb4 · 2025-12-17T08:02:55.000-06:00
* maintenance:rrd-step update to work with rrdcached
No longer uses a temp file or sed
Use RrdProcess and Rrd::getRrdFiles() (fix inconsistencies in getRrdFiles)

* Revert changes to Rrd datastore and use local code.
getRrdFiles is just too much of a mess to use. If someone wants to fix it, they can do so in a separate PR.

* Style fixes
diff --git a/LibreNMS/RRD/RrdProcess.php b/LibreNMS/RRD/RrdProcess.php
@@ -77,7 +77,13 @@ public function run(string $command, string $waitFor = self::COMMAND_COMPLETE):
             return str_contains($buffer, $waitFor);
         });
 
-        return rtrim($this->process->getOutput());
+        $output = $this->process->getOutput();
+
+        if ($waitFor === self::COMMAND_COMPLETE) {
+            $output = substr($output, 0, strrpos($output, $waitFor)); // remove OK line
+        }
+
+        return rtrim($output);
     }
 
     public function runAsync(string $command): void
diff --git a/app/Console/Commands/MaintenanceRrdStep.php b/app/Console/Commands/MaintenanceRrdStep.php
@@ -5,29 +5,38 @@
 use App\Console\LnmsCommand;
 use App\Facades\DeviceCache;
 use App\Facades\LibrenmsConfig;
-use App\Facades\Rrd;
-use Illuminate\Support\Str;
+use Exception;
+use LibreNMS\Exceptions\RrdException;
+use LibreNMS\RRD\RrdProcess;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputOption;
 
 class MaintenanceRrdStep extends LnmsCommand
 {
     protected $name = 'maintenance:rrd-step';
 
+    private int $systemStep;
+    private int $icmpStep;
+    private int $systemHeartbeat;
+
     public function __construct()
     {
         parent::__construct();
         $this->addArgument('device', InputArgument::REQUIRED);
         $this->addOption('confirm', null, InputOption::VALUE_NONE);
     }
 
-    public function handle(): int
+    public function handle(RrdProcess $rrdProcess): int
     {
         $this->configureOutputOptions();
 
+        $this->systemStep = (int) LibrenmsConfig::get('rrd.step', 300);
+        $this->icmpStep = (int) LibrenmsConfig::get('ping_rrd_step', $this->systemStep);
+        $this->systemHeartbeat = (int) LibrenmsConfig::get('rrd.heartbeat', $this->systemStep * 2);
+
         $hostname = (string) $this->argument('device');
         if ($hostname !== 'all') {
-            $hostname = DeviceCache::get($hostname)->hostname;
+            $hostname = DeviceCache::get($hostname)->hostname; // hostname may be numeric
         }
 
         if (empty($hostname)) {
@@ -40,76 +49,105 @@ public function handle(): int
             return 0;
         }
 
-        $systemStep = (int) LibrenmsConfig::get('rrd.step', 300);
-        $icmpStep = (int) LibrenmsConfig::get('ping_rrd_step', $systemStep);
-        $systemHeartbeat = (int) LibrenmsConfig::get('rrd.heartbeat', $systemStep * 2);
-        $rrdtool = (string) LibrenmsConfig::get('rrdtool', 'rrdtool');
-        $daemon = LibrenmsConfig::get('rrdcached') ? '-d ' . escapeshellarg(LibrenmsConfig::get('rrdcached')) : '';
-        $tmpPath = (string) LibrenmsConfig::get('temp_dir', '/tmp');
-        $rrdDir = (string) LibrenmsConfig::get('rrd_dir', LibrenmsConfig::get('install_dir') . '/rrd');
-
-        $files = glob($hostname === 'all'
-            ? Str::finish($rrdDir, '/') . '*/*.rrd'
-            : Rrd::dirFromHost($hostname) . '/*.rrd') ?: [];
-
         [$converted, $skipped, $failed] = [0, 0, 0];
 
-        foreach ($files as $file) {
-            $random = rtrim($tmpPath, '/') . '/' . mt_rand() . '.xml';
+        foreach ($this->listFiles($hostname, $rrdProcess) as $file) {
             $rrdFile = basename($file, '.rrd');
 
             [$step, $heartbeat] = $rrdFile === 'icmp-perf'
-                ? [$icmpStep, $icmpStep * 2]
-                : [$systemStep, $systemHeartbeat];
+                ? [$this->icmpStep, $this->icmpStep * 2]
+                : [$this->systemStep, $this->systemHeartbeat];
 
-            $rrdInfo = shell_exec("$rrdtool info $daemon " . escapeshellarg($file)) ?? '';
-
-            if (preg_match('/step = (\d+)/', $rrdInfo, $stepMatches) && $stepMatches[1] == $step) {
-                preg_match_all('/minimal_heartbeat = (\d+)/', $rrdInfo, $heartbeatMatches);
-
-                $allOk = true;
-                foreach ($heartbeatMatches[1] as $dsHeartbeat) {
-                    if ((int) $dsHeartbeat === (int) $heartbeat) {
-                        continue;
-                    }
-
-                    $allOk = false;
-                    $this->line(__('commands.maintenance:rrd-step.mismatched_heartbeat', ['file' => $file, 'ds' => $dsHeartbeat, 'hb' => $heartbeat]));
-                    break;
-                }
+            try {
+                $this->checkRrdFile($rrdProcess, $file, $step, $heartbeat);
 
-                if ($allOk) {
-                    if ($this->getOutput()->isVerbose()) {
-                        $this->info(__('commands.maintenance:rrd-step.skipping', ['file' => $file, 'step' => $step]));
-                    }
-                    $skipped++;
-                    continue;
+                if ($this->getOutput()->isVerbose()) {
+                    $this->info(__('commands.maintenance:rrd-step.skipping', ['file' => $file, 'step' => $step]));
                 }
+                $skipped++;
+                continue;
+            } catch (\RuntimeException $e) {
+                $this->line($e->getMessage()); // inconsistent data found
+            } catch (RrdException $e) {
+                $this->error($e->getMessage());
+
+                return 1;
             }
 
-            $this->getOutput()->write(__('commands.maintenance:rrd-step.converting', ['file' => $file]) . ' ');
+            try {
+                $this->getOutput()->write(__('commands.maintenance:rrd-step.converting', ['file' => $file]) . ' ');
 
-            $commands = [
-                "$rrdtool dump $daemon " . escapeshellarg($file) . ' > ' . escapeshellarg($random),
-                "sed -i 's/<step>\\([0-9]*\\)/<step>$step/' " . escapeshellarg($random),
-                "sed -i 's/<minimal_heartbeat>\\([0-9]*\\)/<minimal_heartbeat>$heartbeat/' " . escapeshellarg($random),
-                "$rrdtool restore $daemon -f " . escapeshellarg($random) . ' ' . escapeshellarg($file),
-                'rm -f ' . escapeshellarg($random),
-            ];
+                $xmlContent = $rrdProcess->run("dump $file");
+                $modifiedXml = $this->modifyXml($xmlContent, $step, $heartbeat);
+                $rrdProcess->run("restore -f - $file\n$modifiedXml\x1A");
 
-            exec(implode(' && ', $commands), $output, $code);
-
-            if ($code === 0) {
                 $this->info('[OK]');
                 $converted++;
-            } else {
-                $this->error('[FAIL]');
+            } catch (Exception $e) {
+                $this->error('[FAIL]: ' . $e->getMessage());
                 $failed++;
             }
         }
 
-        $this->line(__('commands.maintenance:rrd-step.summary', ['converted' => $converted, 'failed' => $failed, 'skipped' => $skipped]));
+        $this->line(__('commands.maintenance:rrd-step.summary', [
+            'converted' => $converted,
+            'failed' => $failed,
+            'skipped' => $skipped,
+        ]));
 
         return $failed > 0 ? 1 : 0;
     }
+
+    /**
+     * @throws RrdException
+     */
+    private function checkRrdFile(RrdProcess $rrdProcess, string $file, int $step, int $heartbeat): void
+    {
+        $rrdInfo = $rrdProcess->run("info $file");
+
+        if (! preg_match('/step = (\d+)/', $rrdInfo, $stepMatches) || $stepMatches[1] != $step) {
+            throw new \RuntimeException(__('commands.maintenance:rrd-step.mismatched_heartbeat', ['file' => $file, 'ds' => 'step', 'hb' => $step]));
+        }
+
+        preg_match_all('/minimal_heartbeat = (\d+)/', $rrdInfo, $heartbeatMatches);
+
+        foreach ($heartbeatMatches[1] as $dsHeartbeat) {
+            if ((int) $dsHeartbeat !== $heartbeat) {
+                throw new \RuntimeException(__('commands.maintenance:rrd-step.mismatched_heartbeat', ['file' => $file, 'ds' => $dsHeartbeat, 'hb' => $heartbeat]));
+            }
+        }
+    }
+
+    private function modifyXml(string $xmlContent, int $step, int $heartbeat): string
+    {
+        $xmlContent = (string) preg_replace('#<step>\d+</step>#', "<step>$step</step>", $xmlContent);
+        $xmlContent = (string) preg_replace('#<minimal_heartbeat>\d+</minimal_heartbeat>#', "<minimal_heartbeat>$heartbeat</minimal_heartbeat>", $xmlContent);
+
+        return $xmlContent;
+    }
+
+    /**
+     * @param  string  $hostname
+     * @param  RrdProcess  $rrdProcess
+     * @return string[]
+     *
+     * @throws RrdException
+     */
+    private function listFiles(string $hostname, RrdProcess $rrdProcess): array
+    {
+        $command = $hostname === 'all' ? ['list -r .', ''] : "list ./$hostname";
+        $output = rtrim($rrdProcess->run($command));
+
+        if (empty($output)) {
+            return [];
+        }
+
+        $files = explode("\n", $output);
+
+        if ($hostname === 'all') {
+            return $files;
+        }
+
+        return array_map(fn ($file) => "$hostname/$file", $files);
+    }
 }
