[remoteaccess] Track wol failures, avoid those clients

3 files changed, 55 insertions, 8 deletions

diff --git a/modules-available/remoteaccess/hooks/client-update.inc.php b/modules-available/remoteaccess/hooks/client-update.inc.php
index ecf5d91c..579eb25c 100644
--- a/modules-available/remoteaccess/hooks/client-update.inc.php
+++ b/modules-available/remoteaccess/hooks/client-update.inc.php
@@ -1,7 +1,8 @@
 <?php
 
 if ($type === '~poweron') {
-	Database::exec("UPDATE remoteaccess_machine SET password = NULL WHERE machineuuid = :uuid",
+	Database::exec("UPDATE remoteaccess_machine SET password = NULL, wolfails = 0
+			WHERE machineuuid = :uuid",
 		['uuid' => $uuid]);
 } elseif ($type === '~poweroff') {
 	Database::exec("UPDATE remoteaccess_machine SET woltime = 0 WHERE machineuuid = :uuid",
diff --git a/modules-available/remoteaccess/inc/remoteaccess.inc.php b/modules-available/remoteaccess/inc/remoteaccess.inc.php
index 95ca3821..2000894e 100644
--- a/modules-available/remoteaccess/inc/remoteaccess.inc.php
+++ b/modules-available/remoteaccess/inc/remoteaccess.inc.php
@@ -11,6 +11,9 @@ class RemoteAccess
 
 	const PROP_PLUGIN_VERSION = 'remoteaccess.plugin-version';
 
+	/** @var int How long we wait for a client to boot up, i.e. assume it will become online soon */
+	const WOL_TIMEOUT = 120;
+
 	/**
 	 * Get a list of locationIds where remote access is enabled. If $filterOverridden is true,
 	 * the list will not contain any locations where remote access is disabled via location override.
@@ -35,13 +38,18 @@ class RemoteAccess
 		});
 	}
 
-	public static function ensureMachinesRunning()
+	/**
+	 * Tries to ensure that the requested amount of machines is running for every remote access group.
+	 */
+	public static function ensureMachinesRunning(): void
 	{
 		if (!Module::isAvailable('rebootcontrol')) {
 			error_log("Not waking remote access machines: rebootcontrol missing");
 			return;
 		}
 
+		self::updateFailCounters();
+
 		$res = Database::simpleQuery("SELECT rg.groupid, rg.groupname, rg.wolcount,
 			GROUP_CONCAT(rxl.locationid) AS locs
 			FROM remoteaccess_group rg
@@ -49,8 +57,8 @@ class RemoteAccess
 			WHERE rg.active = 1
 			GROUP BY groupid");
 
-		// Consider machines we tried to wake in the past 90 seconds as online
-		$wolDeadline = time() - 90;
+		// Consider machines we tried to wake in the past 120 seconds as online
+		$wolDeadline = time() - self::WOL_TIMEOUT;
 		foreach ($res as $row) {
 			$wantNum = $row['wolcount'];
 			// This can't really be anything but a CSV list, but better be safe
@@ -82,15 +90,24 @@ class RemoteAccess
 		}
 	}
 
+	/**
+	 * Tries to wake machines based on provided locations and a given number limit.
+	 * @param string $locs The locations for which machines need to be woken up.
+	 * @param int $num The maximum number of machines to wake up.
+	 * @return int The remaining number of machines that could not be woken up.
+	 */
 	private static function tryWakeMachines(string $locs, int $num): int
 	{
 		if (empty($locs))
 			return $num;
+		$NOW = time();
 		$res = Database::simpleQuery("SELECT m.machineuuid, m.macaddr, m.clientip, m.locationid FROM machine m
 				LEFT JOIN remoteaccess_machine rm USING (machineuuid)
-				WHERE m.locationid IN ($locs) AND m.state IN ('OFFLINE', 'STANDBY')
-				ORDER BY rm.woltime ASC");
-		$NOW = time();
+				WHERE m.locationid IN ($locs)
+				  AND m.state IN ('OFFLINE', 'STANDBY')
+				  AND rm.woltime < :woldeadline AND rm.wolfails < 10
+				ORDER BY rm.wolfails ASC, rm.woltime ASC
+				LIMIT 100", ['woldeadline' => $NOW - 300]);
 		while ($num > 0) {
 			$list = [];
 			for ($i = 0; $i < $num && ($row = $res->fetch()); ++$i) {
@@ -108,10 +125,29 @@ class RemoteAccess
 				$failIds = ArrayUtil::flattenByKey($fails, 'machineuuid');
 				// Reduce time so they won't be marked as wol_in_progress
 				Database::exec('UPDATE remoteaccess_machine SET woltime = :faketime WHERE machineuuid IN (:fails)',
-					['faketime' => $NOW - 95, 'fails' => $failIds]);
+					['faketime' => $NOW - self::WOL_TIMEOUT, 'fails' => $failIds]);
 			}
 		}
 		return $num;
 	}
 
+	/**
+	 * Update the fail counters for remote access machines that didn't turn on.
+	 * We assume a WoL-attempt failed if it took longer than two minutes and
+	 * the machine is still not up.
+	 */
+	private static function updateFailCounters(): void
+	{
+		$upper = time() - self::WOL_TIMEOUT;
+		$lower = $upper - self::WOL_TIMEOUT;
+		Database::exec("UPDATE remoteaccess_machine rm, machine m
+				SET rm.wolfails = rm.wolfails + 1, rm.woltime = rm.woltime - :woltimeout
+				WHERE (rm.woltime BETWEEN :lower AND :upper) AND rm.machineuuid = m.machineuuid
+				AND m.state IN ('OFFLINE', 'STANDBY') AND m.lastseen < :lower", [
+					'lower' => $lower,
+					'upper' => $upper,
+					'woltimeout' => self::WOL_TIMEOUT,
+		]);
+	}
+
 }
diff --git a/modules-available/remoteaccess/install.inc.php b/modules-available/remoteaccess/install.inc.php
index 2a6fec36..7a78a032 100644
--- a/modules-available/remoteaccess/install.inc.php
+++ b/modules-available/remoteaccess/install.inc.php
@@ -22,6 +22,7 @@ $dbret[] = tableCreate('remoteaccess_machine', "
 	`machineuuid` char(36) CHARACTER SET ascii NOT NULL,
 	`password` char(8) CHARACTER SET ascii NULL DEFAULT NULL,
 	`woltime` int(10) UNSIGNED NOT NULL DEFAULT '0',
+	`wolfails` int(10) UNSIGNED NOT NULL DEFAULT '0',
 	`vncport` smallint(5) UNSIGNED NOT NULL DEFAULT '5900',
 	PRIMARY KEY (`machineuuid`)
 ");
@@ -77,4 +78,13 @@ if (!tableHasColumn('remoteaccess_group', 'unwoken')) {
 	$dbret[] = UPDATE_DONE;
 }
 
+// 2021-03-05: Add wolfails column to machine table
+if (!tableHasColumn('remoteaccess_machine', 'wolfails')) {
+	$ret = Database::exec("ALTER TABLE remoteaccess_machine ADD COLUMN `wolfails` int(10) UNSIGNED NOT NULL DEFAULT '0'");
+	if ($ret === false) {
+		finalResponse(UPDATE_FAILED, Database::lastError());
+	}
+	$dbret[] = UPDATE_DONE;
+}
+
 responseFromArray($dbret);