From 3e99b87cdf2ad2227ebde3296bfbc4a588c844f0 Mon Sep 17 00:00:00 2001
From: Ad Schellevis <ad@opnsense.org>
Date: Sun, 11 Aug 2024 12:56:49 +0200
Subject: [PATCH] Reporting / rrd - start using cron for rrd collection.

This is a temporary solution, but a first step into cleaning up rrd stat collection.

In this first milestone, we keep using the generated script, but remove the loop and sleep construct out of it, offering the posibility to handle service control to cron. To prevent the script being written at the same time we're collecting, we always lock the script before use.

should fix: https://github.com/opnsense/core/issues/7753
---
 src/etc/inc/plugins.inc.d/core.inc |  8 ++++++++
 src/etc/inc/rrd.inc                | 25 ++++++++-----------------
 src/www/reporting_settings.php     |  6 ++++--
 3 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/src/etc/inc/plugins.inc.d/core.inc b/src/etc/inc/plugins.inc.d/core.inc
index 347cff55f..e5328209d 100644
--- a/src/etc/inc/plugins.inc.d/core.inc
+++ b/src/etc/inc/plugins.inc.d/core.inc
@@ -295,6 +295,14 @@ function core_cron()
     $jobs[]['autocron'] = array('/usr/local/sbin/ping_hosts.sh', '*/4');
     $jobs[]['autocron'] = array('/usr/local/sbin/configctl -d firmware changelog cron', '0', '22');
 
+    /**
+     * rrd graph collector, only execute when script is present and not locked by rrd_configure()
+     */
+    $jobs[]['autocron'] = [
+        'test -e /var/db/rrd/updaterrd.sh && /usr/local/bin/flock -n -E 1 /var/db/rrd/updaterrd.sh /var/db/rrd/updaterrd.sh',
+        '*'
+    ];
+
     if (!empty($config['system']['rrdbackup']) && $config['system']['rrdbackup'] > 0) {
         $jobs[]['autocron'] = array(
             '/usr/local/etc/rc.syshook.d/backup/20-rrd',
diff --git a/src/etc/inc/rrd.inc b/src/etc/inc/rrd.inc
index 6005d8379..7f04333bb 100644
--- a/src/etc/inc/rrd.inc
+++ b/src/etc/inc/rrd.inc
@@ -100,7 +100,7 @@ function rrd_configure($verbose = false, $bootup = false)
     $downstream = 2500000000;
     $upstream = 2500000000;
 
-    /* kill off traffic collectors */
+    /* XXX: kill off traffic collectors, remove when we know the update required a reboot */
     killbypid('/var/run/updaterrd.pid');
 
     if (isset($config['rrd']['enable'])) {
@@ -109,16 +109,14 @@ function rrd_configure($verbose = false, $bootup = false)
             mkdir($rrddbpath, 0775);
         }
         chown($rrddbpath, "nobody");
+        /* open file handle to update script and lock it in exclusive mode */
+        $fobj = new \OPNsense\Core\FileObject('/var/db/rrd/updaterrd.sh', 'r+', 0755, LOCK_EX);
 
         /* db update script */
         $rrdupdatesh = "#!/bin/sh\n";
         $rrdupdatesh .= "\n";
         $rrdupdatesh .= "export TERM=dumb\n";
         $rrdupdatesh .= "\n";
-        $rrdupdatesh .= "counter=1\n";
-        $rrdupdatesh .= "while [ \"\$counter\" -ne 0 ]\n";
-        $rrdupdatesh .= "do\n";
-        $rrdupdatesh .= "";
 
         $ifdescrs = get_configured_interface_with_descr();
         /* IPsec counters */
@@ -541,19 +539,12 @@ function rrd_configure($verbose = false, $bootup = false)
 
 EOD;
 
-        $rrdupdatesh .= "\n";
-        $rrdupdatesh .= "     for UNUSED in \$(seq 1 60); do sleep 1; done\n";
-        $rrdupdatesh .= "done\n";
-
         /* write the rrd update script */
-        $updaterrdscript = '/var/db/rrd/updaterrd.sh';
-        $fd = fopen($updaterrdscript, 'w');
-        fwrite($fd, $rrdupdatesh);
-        fclose($fd);
-        chmod($updaterrdscript, 0755);
-
-        /* start traffic collector */
-        mwexecf('/usr/sbin/daemon -f -p %s %s', ['/var/run/updaterrd.pid', $updaterrdscript]);
+        $fobj->seek(0)->truncate(0)->write($rrdupdatesh);
+        unset($fobj);
+    } elseif (file_exists('/var/db/rrd/updaterrd.sh')) {
+        /* remove script, stop collecting. cron checks for existence if this file */
+        unlink('/var/db/rrd/updaterrd.sh');
     }
 
     $databases = glob("{$rrddbpath}/*.rrd");
diff --git a/src/www/reporting_settings.php b/src/www/reporting_settings.php
index 334653c00..5a4d37973 100644
--- a/src/www/reporting_settings.php
+++ b/src/www/reporting_settings.php
@@ -75,8 +75,10 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
     if ($configure_unbound) {
         unbound_configure_do();
     } else {
-        plugins_configure('monitor');
-        rrd_configure();
+      plugins_configure('monitor');
+      rrd_configure();
+      /* XXX: rrd graphs depend on a cronjob, we can probably remove this in a future version when the job is unconditional */
+      system_cron_configure();
     }
 
 }