maintenance:rrd-step update to work with rrdcached

murrant · murrant · commit dd45ed84e9fb · 2025-12-09T01:44:29.000-06:00
No longer uses a temp file or sed
Use RrdProcess and Rrd::getRrdFiles() (fix inconsistencies in getRrdFiles)
diff --git a/LibreNMS/Data/Store/Rrd.php b/LibreNMS/Data/Store/Rrd.php
@@ -485,26 +485,29 @@ public function buildCommand(string $command, string $filename, array $options =
     }
 
     /**
-     * Get array of all rrd files for a device,
-     * via rrdached or localdisk.
+     * Get array of all rrd files for a device via rrdached or localdisk.
+     * If $hostname is empty, it will list all rrd files
      *
      * @param  string  $hostname  hostname of the device
-     * @return string[] array of rrd files for this host
+     * @return string[] array of rrd files for this host relative to rrd_dir
      */
     public function getRrdFiles(string $hostname): array
     {
-        if ($this->rrdcached) {
-            $output = $this->command('list', '/' . self::safeName($hostname));
+        $dir = $this->dirFromHost($hostname);
 
+        if ($this->rrdcached) {
+            $output = $this->command('list', $dir . '/', ['--recursive']);
             $files = explode("\n", trim($output[0] ?? ''));
             array_pop($files); // remove rrdcached status line
-        } else {
-            $files = glob($this->dirFromHost($hostname) . '/*.rrd') ?: [];
+            $prepend = str_replace($this->rrd_dir . '/', '', $dir) . '/';
+
+            return array_map(fn ($file) => $prepend . $file, $files);
         }
 
-        sort($files);
+        $iterator = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($dir));
+        $rrdFiles = new \RegexIterator($iterator, '/\.rrd$/');
 
-        return $files;
+        return array_map(fn (\SplFileInfo $file) => str_replace($this->rrd_dir . '/', '', $file), iterator_to_array($rrdFiles, false));
     }
 
     /**
@@ -522,6 +525,8 @@ public function getRrdApplicationArrays($device, $app_id, $app_name, $category =
         $separator = '-';
 
         $rrdfile_array = $this->getRrdFiles($device['hostname']);
+        sort($rrdfile_array);
+
         if ($category) {
             $pattern = sprintf('%s-%s-%s-%s', 'app', $app_name, $app_id, $category);
         } else {
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
@@ -6,28 +6,38 @@
 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 +50,82 @@ 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') ?: [];
+        $files = Rrd::getRrdFiles($hostname === 'all' ? '' : $hostname);
 
         [$converted, $skipped, $failed] = [0, 0, 0];
 
         foreach ($files as $file) {
-            $random = rtrim($tmpPath, '/') . '/' . mt_rand() . '.xml';
             $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)) ?? '';
+            try {
+                $this->checkRrdFile($rrdProcess, $file, $step, $heartbeat);
 
-            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;
-                }
-
-                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]) . ' ');
-
-            $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),
-            ];
+            try {
+                $this->getOutput()->write(__('commands.maintenance:rrd-step.converting', ['file' => $file]) . ' ');
 
-            exec(implode(' && ', $commands), $output, $code);
+                $xmlContent = $rrdProcess->run("dump $file");
+                $modifiedXml = $this->modifyXml($xmlContent, $step, $heartbeat);
+                $rrdProcess->run("restore -f - $file\n$modifiedXml\x1A");
 
-            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;
+    }
 }
