From 981cbe274456f3c674a94093ae85ec9cb1aed1f0 Mon Sep 17 00:00:00 2001
From: "Michael N. Lipp" <mnl@mnl.de>
Date: Sun, 26 Jan 2025 14:52:26 +0100
Subject: [PATCH] Improve readability.

---
 .../vmoperator/common/VmDefinition.java       | 18 ++++++++
 .../vmoperator/manager/PoolMonitor.java       | 22 ++++-----
 .../jdrupes/vmoperator/manager/VmMonitor.java | 45 +++++++++++++++----
 3 files changed, 65 insertions(+), 20 deletions(-)

diff --git a/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmDefinition.java b/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmDefinition.java
index 2742b88..da639f4 100644
--- a/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmDefinition.java
+++ b/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmDefinition.java
@@ -353,6 +353,24 @@ public class VmDefinition {
                 .map("True"::equals).orElse(false));
     }
 
+    /**
+     * Return true if the console is in use.
+     *
+     * @return true, if successful
+     */
+    public boolean consoleConnected() {
+        return conditionStatus("ConsoleConnected").orElse(false);
+    }
+
+    /**
+     * Return the last known console user.
+     *
+     * @return the optional
+     */
+    public Optional<String> consoleUser() {
+        return this.<String> fromStatus("consoleUser");
+    }
+
     /**
      * Set extra data (locally used, unknown to kubernetes).
      *
diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PoolMonitor.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PoolMonitor.java
index 4a7a1cb..0606650 100644
--- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PoolMonitor.java
+++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/PoolMonitor.java
@@ -23,7 +23,6 @@ import io.kubernetes.client.apimachinery.GroupVersionKind;
 import io.kubernetes.client.openapi.ApiException;
 import io.kubernetes.client.util.Watch;
 import java.io.IOException;
-import java.time.Instant;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -171,17 +170,18 @@ public class PoolMonitor extends
         }
 
         // Sync last usage to console state change if user matches
-        var assignedTo = vmDef.assignedTo().orElse(null);
-        if (assignedTo == null || !assignedTo
-            .equals(vmDef.<String> fromStatus("consoleUser").orElse(null))) {
+        if (vmDef.assignedTo()
+            .map(at -> at.equals(vmDef.consoleUser().orElse(null)))
+            .orElse(true)) {
             return;
         }
-        var lastUsed
-            = vmDef.assignmentLastUsed().orElse(Instant.ofEpochSecond(0));
-        var conChange = vmDef.condition("ConsoleConnected")
-            .map(c -> c.getLastTransitionTime().toInstant())
-            .orElse(Instant.ofEpochSecond(0));
-        if (!conChange.isAfter(lastUsed)) {
+
+        var ccChange = vmDef.condition("ConsoleConnected")
+            .map(cc -> cc.getLastTransitionTime().toInstant());
+        if (ccChange
+            .map(tt -> vmDef.assignmentLastUsed().map(alu -> alu.isAfter(tt))
+                .orElse(true))
+            .orElse(true)) {
             return;
         }
         var vmStub = VmDefinitionStub.get(client(),
@@ -190,7 +190,7 @@ public class PoolMonitor extends
         vmStub.updateStatus(from -> {
             JsonObject status = from.status();
             var assignment = GsonPtr.to(status).to("assignment");
-            assignment.set("lastUsed", conChange.toString());
+            assignment.set("lastUsed", ccChange.get().toString());
             return status;
         });
     }
diff --git a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/VmMonitor.java b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/VmMonitor.java
index 9b6f75f..102a6c9 100644
--- a/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/VmMonitor.java
+++ b/org.jdrupes.vmoperator.manager/src/org/jdrupes/vmoperator/manager/VmMonitor.java
@@ -283,15 +283,7 @@ public class VmMonitor extends
             // Find available VM.
             var pool = vmPool;
             assignedVm = channelManager.channels().stream()
-                .filter(c -> c.vmDefinition().pools()
-                    .contains(event.fromPool()))
-                .filter(c -> !c.vmDefinition()
-                    .conditionStatus("ConsoleConnected").orElse(false))
-                .filter(c -> c.vmDefinition().assignedTo().isEmpty()
-                    || pool.retainUntil(c.vmDefinition()
-                        .<String> fromStatus("assignment", "lastUsed")
-                        .map(Instant::parse).orElse(Instant.ofEpochSecond(0)))
-                        .isBefore(Instant.now()))
+                .filter(c -> isAssignable(pool, c.vmDefinition()))
                 .sorted(Comparator.comparing(c -> c.vmDefinition()
                     .assignmentLastUsed().orElse(Instant.ofEpochSecond(0))))
                 .findFirst();
@@ -320,4 +312,39 @@ public class VmMonitor extends
                 assignedVm.get()));
         }
     }
+
+    @SuppressWarnings("PMD.SimplifyBooleanReturns")
+    private boolean isAssignable(VmPool pool, VmDefinition vmDef) {
+        // Check if the VM is in the pool
+        if (!vmDef.pools().contains(pool.name())) {
+            return false;
+        }
+
+        // Check if the VM is not in use
+        if (vmDef.consoleConnected()) {
+            return false;
+        }
+
+        // If not assigned, it's usable
+        if (vmDef.assignedTo().isEmpty()) {
+            return true;
+        }
+
+        // Check if it is to be retained
+        if (vmDef.assignmentLastUsed()
+            .map(lu -> pool.retainUntil(lu))
+            .map(ru -> Instant.now().isBefore(ru)).orElse(false)) {
+            return false;
+        }
+
+        // Additional check in case lastUsed has not been updated
+        // by PoolMonitor#onVmDefChanged() yet ("race condition")
+        if (vmDef.condition("ConsoleConnected")
+            .map(cc -> cc.getLastTransitionTime().toInstant())
+            .map(t -> pool.retainUntil(t))
+            .map(ru -> Instant.now().isBefore(ru)).orElse(false)) {
+            return false;
+        }
+        return true;
+    }
 }