[rebootcontrol] Simplify some logic, improve scheduler

3 files changed, 83 insertions, 96 deletions

diff --git a/modules-available/rebootcontrol/hooks/cron.inc.php b/modules-available/rebootcontrol/hooks/cron.inc.php
index 5ae3b1c8..56446c49 100644
--- a/modules-available/rebootcontrol/hooks/cron.inc.php
+++ b/modules-available/rebootcontrol/hooks/cron.inc.php
@@ -12,25 +12,21 @@ if (in_array((int)date('G'), [6, 7, 9, 12, 15]) && in_array(date('i'), ['00', '0
 
 // CRON for Scheduler
 $now = time();
-$res = Database::simpleQuery("SELECT * FROM reboot_scheduler WHERE nextexecution < :now", ['now' => $now]);
+$res = Database::simpleQuery("SELECT s.locationid, s.action, s.nextexecution, s.options, l.openingtime
+	FROM reboot_scheduler s
+	INNER JOIN location l USING (locationid)
+	WHERE s.nextexecution <= :now", ['now' => $now]);
 while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
 	$options = json_decode($row['options'], true);
 
 	// Calculate next_execution for the event.
-	$location = Database::queryFirst("SELECT openingtime FROM `location`
-		WHERE locationid = :lid", ['lid' => $row['locationid']]);
-	Scheduler::updateSchedule($row['locationid'], $options, $location['openingtime']);
+	Scheduler::updateSchedule($row['locationid'], $options, $row['openingtime']);
 
 	if ($row['nextexecution'] + 1200 < $now)
 		continue;
 
-	$machinedb = Database::simpleQuery("SELECT machineuuid, clientip, macaddr, locationid FROM machine
+	$machines = Database::queryAll("SELECT machineuuid, clientip, macaddr, locationid FROM machine
 		WHERE locationid = :locid", ['locid' => $row['locationid']]);
-	$machines = [];
-	while ($machine = $machinedb->fetch(PDO::FETCH_ASSOC)) {
-		settype($machine['locationid'], 'int');
-		$machines[] = $machine;
-	}
 	if ($row['action'] === 'sd') {
 		RebootControl::execute($machines, RebootControl::SHUTDOWN, 0);
 	} elseif ($row['action'] === 'wol') {
diff --git a/modules-available/rebootcontrol/inc/scheduler.inc.php b/modules-available/rebootcontrol/inc/scheduler.inc.php
index 7a5c08dc..613fbbee 100644
--- a/modules-available/rebootcontrol/inc/scheduler.inc.php
+++ b/modules-available/rebootcontrol/inc/scheduler.inc.php
@@ -5,14 +5,22 @@ class Scheduler
 
 	public static function updateSchedule($locationid, $options, $openingTimes)
 	{
-		if ($openingTimes == '') {
+		if (empty($openingTimes)) {
 			self::deleteSchedule($locationid);
 			return false;
 		}
 		$nextexec = self::calculateNext($options, $openingTimes);
-		$json_options = json_encode($options);
 		if ($nextexec !== false) {
-			self::upsert($locationid, $nextexec['action'], $nextexec['time'], $json_options);
+			$json_options = json_encode($options);
+			Database::exec("INSERT INTO `reboot_scheduler` (locationid, action, nextexecution, options) 
+				VALUES (:lid, :act, :next, :opt)
+				ON DUPLICATE KEY UPDATE
+					action = VALUES(action), nextexecution = VALUES(nextexecution), options = VALUES(options)", [
+				'lid' => $locationid,
+				'act' => $nextexec['action'],
+				'next' => $nextexec['time'],
+				'opt' => $json_options,
+			]);
 		} else {
 			// All times are getting ignored because they are within 5 minutes of each other, delete possible db entries.
 			self::deleteSchedule($locationid);
@@ -26,106 +34,86 @@ class Scheduler
 			WHERE locationid = :lid", ['lid' => $locationid]);
 	}
 
+	private static function calculateTimestamp($now, $day, $time)
+	{
+		$ts = strtotime("$day $time");
+		if ($ts < $now) {
+			$ts = strtotime("next $day $time");
+		}
+		if ($ts < $now) {
+			EventLog::warning("Invalid params to calculateTimestamp(): 'next $day $time'");
+			$ts = $now + 864000;
+		}
+		return $ts;
+	}
+
 	private static function calculateNext($options, $openingTimes)
 	{
+		if (!$options['wol'] && !$options['sd'])
+			return false;
 		$openingTimes = json_decode($openingTimes, true);
 		$now = time();
-		$openTimes = [];
-		$closeTimes = [];
+		$events = [];
 		foreach ($openingTimes as $row) {
-			if (!$options['wol'] && !$options['sd'])
-				continue;
-			if ($options['wol']) {
-				$open = explode(':', $row['openingtime']);
-				$openTime = $open[0] * 60 + $open[1] - $options['wol-offset'];
-			}
-			if ($options['sd']) {
-				$close = explode(':', $row['closingtime']);
-				$closeTime = $close[0] * 60 + $close[1] + $options['sd-offset'];
-			}
 			foreach ($row['days'] as $day) {
 				if ($options['wol']) {
-					$nextOpen = strtotime(date('Y-m-d H:i', strtotime($day . ' ' . $openTime . ' minutes')));
-					if ($nextOpen < $now) {
-						$openTimes[] = strtotime(date('Y-m-d H:i', strtotime('next ' . $day . ' ' . $openTime . ' minutes')));
-					} else {
-						$openTimes[] = $nextOpen;
-					}
+					$events[] = ['action' => 'wol',
+						'time' => self::calculateTimestamp($now, $day, $row['openingtime'])];
 				}
 				if ($options['sd']) {
-					$nextClose = strtotime(date('Y-m-d H:i', strtotime($day . ' ' . $closeTime . ' minutes')));
-					if ($nextClose < $now) {
-						$closeTimes[] = strtotime(date('Y-m-d H:i', strtotime('next ' . $day . ' ' . $closeTime . ' minutes')));
-					} else {
-						$closeTimes[] = $nextClose;
-					}
+					$events[] = ['action' => 'sd',
+						'time' => self::calculateTimestamp($now, $day, $row['closingtime'])];
 				}
 			}
 		}
+		$tmp = ArrayUtil::flattenByKey($events, 'time');
+		array_multisort($tmp, SORT_NUMERIC | SORT_ASC, $events);
 
-		sort($openTimes);
-		sort($closeTimes);
-		$res = [];
-
-		if ($options['wol'] && !$options['sd']) {
-			$res['action'] = 'wol';
-			$res['time'] = $openTimes[0];
-			return $res;
-		} else if ($options['sd'] && !$options['wol']) {
-			$res['action'] = 'sd';
-			$res['time'] = $closeTimes[0];
-			return $res;
+		// Only apply offsets now, so we can detect nonsensical overlap
+		$wolOffset = $options['wol-offset'] * 60;
+		$sdOffset = $options['sd-offset'] * 60;
+		$prev = PHP_INT_MAX;
+		for ($i = count($events) - 1; $i >= 0; --$i) {
+			$event =& $events[$i];
+			if ($event['action'] === 'wol') {
+				$event['time'] -= $wolOffset;
+			} elseif ($event['action'] === 'sd') {
+				$event['time'] += $sdOffset;
+			} else {
+				error_log('BUG Unhandled event type ' . $event['action']);
+			}
+			if ($event['time'] >= $prev || $event['time'] < $now) {
+				// Overlap, delete this event
+				unset($events[$i]);
+			} else {
+				$prev = $event['time'];
+			}
 		}
+		unset($event);
+		// Reset array keys
+		$events = array_values($events);
 
-		for ($i = 0; $i <= sizeof($openTimes); $i++) {
-			if (abs($openTimes[$i] - $closeTimes[$i]) < 300) {
-				// If difference is < 5 min, ignore both events.
+		// See which is the next suitable event to act upon
+		$lastEvent = count($events) - 1;
+		for ($i = 0; $i <= $lastEvent; $i++) {
+			$event =& $events[$i];
+			$diff = ($i === $lastEvent ? PHP_INT_MAX : $events[$i + 1]['time'] - $event['time']);
+			if ($diff < 300 && $event['action'] !== $events[$i + 1]['action']) {
+				// If difference to next event is < 5 min, ignore.
 				continue;
-			} elseif (abs($openTimes[$i] - $closeTimes[$i]) < 900) {
-				// If difference is < 15 min, reboot at the earlier time.
+			}
+			if ($diff < 900 && $event['action'] === 'sd' && $events[$i + 1]['action'] === 'wol') {
+				// If difference to next WOL is < 15 min and this is a shutdown, reboot instead.
 				$res['action'] = 'rb';
-				$res['time'] = $openTimes[$i] < $closeTimes[$i] ? $openTimes[$i] : $closeTimes[$i];
+				$res['time'] = $event['time'];
 			} else {
 				// Use first event.
-				if ($openTimes[$i] < $closeTimes[$i]) {
-					$res['action'] = 'wol';
-					$res['time'] = $openTimes[$i];
-				} else {
-					$res['action'] = 'sd';
-					$res['time'] = $closeTimes[$i];
-				}
+				$res = $event;
 			}
 			return $res;
 		}
+		unset($event);
 		return false;
 	}
 
-	private static function upsert($locationid, $action, $nextexec, $options)
-	{
-		$schedule = Database::queryFirst("SELECT locationid 
-			FROM `reboot_scheduler`
-			WHERE locationid = :lid", [
-			'lid' => $locationid
-		]);
-		if ($schedule === false) {
-			Database::exec("INSERT INTO `reboot_scheduler` (locationid, action, nextexecution, options) 
-				VALUES (:lid, :act, :next, :opt)", [
-				'lid' => $locationid,
-				'act' => $action,
-				'next' => $nextexec,
-				'opt' => $options
-			]);
-		} else {
-			Database::exec("UPDATE `reboot_scheduler` 
-				SET action = :act, nextexecution = :next, options = :opt
-				WHERE locationid = :lid", [
-				'act' => $action,
-				'next' => $nextexec,
-				'opt' => $options,
-				'lid' => $locationid
-			]);
-		}
-		return true;
-	}
-
 }
\ No newline at end of file
diff --git a/modules-available/rebootcontrol/install.inc.php b/modules-available/rebootcontrol/install.inc.php
index 008d26aa..f400129e 100644
--- a/modules-available/rebootcontrol/install.inc.php
+++ b/modules-available/rebootcontrol/install.inc.php
@@ -39,10 +39,10 @@ $output[] = tableCreate('reboot_subnet_x_subnet', "
 
 $output[] = tableCreate('reboot_scheduler', "
 	`locationid` INT(11) NOT NULL,
-	`action` ENUM('wol', 'sd'),
+	`action` ENUM('wol', 'sd', 'rb'),
 	`nextexecution` INT(10) UNSIGNED NOT NULL DEFAULT 0,
 	`options` BLOB,
-	PRIMARY KEY (`locationid`, `action`)");
+	PRIMARY KEY (`locationid`)");
 
 $output[] = tableAddConstraint('reboot_jumphost_x_subnet', 'hostid', 'reboot_jumphost', 'hostid',
 	'ON UPDATE CASCADE ON DELETE CASCADE');
@@ -55,11 +55,14 @@ $output[] = tableAddConstraint('reboot_subnet_x_subnet', 'dstid', 'reboot_subnet
 $output[] = tableAddConstraint('reboot_scheduler', 'locationid', 'location', 'locationid',
 	'ON UPDATE CASCADE ON DELETE CASCADE');
 
-if (tableExists('reboot_scheduler')) {
-	Database::exec("ALTER TABLE `reboot_scheduler` DROP PRIMARY KEY , ADD PRIMARY KEY (`locationid`)");
+if (tableColumnKeyType('reboot_scheduler', 'action') === 'PRI') {
+	Database::exec("DELETE FROM reboot_scheduler WHERE action <> 'wol'");
+	$res = Database::exec("ALTER TABLE `reboot_scheduler` DROP PRIMARY KEY, ADD PRIMARY KEY (`locationid`)");
+	$output[] = $res !== false ? UPDATE_DONE : UPDATE_FAILED;
 }
-if (tableHasColumn('reboot_scheduler', 'action')) {
-	Database::exec("ALTER TABLE `reboot_scheduler` MODIFY COLUMN `action` ENUM('wol', 'sd', 'rb')");
+if (strpos(tableColumnType('reboot_scheduler', 'action'), 'rb') === false) {
+	$res = Database::exec("ALTER TABLE `reboot_scheduler` MODIFY COLUMN `action` ENUM('wol', 'sd', 'rb')");
+	$output[] = $res !== false ? UPDATE_DONE : UPDATE_FAILED;
 }