Correctly determine if server is eligible for removal

Closes #48
jenkinsci · Aug 11, 2022 · 5cf24cc · 5cf24cc
1 parent f7c9ee6
commit 5cf24cc
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 25 deletions.
diff --git a/src/main/java/cloud/dnation/jenkins/plugins/hetzner/Helper.java b/src/main/java/cloud/dnation/jenkins/plugins/hetzner/Helper.java
@@ -15,7 +15,6 @@
  */
 package cloud.dnation.jenkins.plugins.hetzner;
 
-import cloud.dnation.jenkins.plugins.hetzner.client.ServerDetail;
 import com.cloudbees.jenkins.plugins.sshcredentials.impl.BasicSSHUserPrivateKey;
 import com.cloudbees.plugins.credentials.CredentialsMatchers;
 import com.cloudbees.plugins.credentials.CredentialsProvider;
@@ -40,7 +39,9 @@
 import java.nio.charset.StandardCharsets;
 import java.security.KeyPair;
 import java.security.interfaces.RSAPublicKey;
+import java.time.Duration;
 import java.time.LocalDateTime;
+import java.time.ZoneOffset;
 import java.time.format.DateTimeFormatter;
 import java.util.Base64;
 import java.util.Collections;
@@ -168,21 +169,17 @@ public void error(String message, Throwable cause) {
      * you are billed for every hour of existence of server, so it makes sense to keep server running as long as next hour did
      * not start yet.
      *
-     * @param createdStr    RFC3339-formatted instant when server was created. See ServerDetail#getCreated().
-     * @param currentMinute current minute. Kept as argument to allow unit-testing.
+     * @param createdStr  RFC3339-formatted instant when server was created. See ServerDetail#getCreated().
+     * @param currentTime current time. Kept as argument to allow unit-testing.
      * @return <code>true</code> if server should be shut down, <code>false</code> otherwise.
      * Note: we keep small time buffer for corner cases like clock skew or Jenkins's queue manager overload, which could
      * lead to unnecessary 1-hour over-billing.
      */
-    public static boolean canShutdownServer(@Nonnull String createdStr, int currentMinute) {
-        int billingMinute = LocalDateTime.from(DateTimeFormatter.ISO_DATE_TIME.parse(createdStr)).getMinute();
-        if (billingMinute < SHUTDOWN_TIME_BUFFER) {
-            billingMinute += 60;
-        }
-        if (currentMinute < SHUTDOWN_TIME_BUFFER) {
-            currentMinute += 60;
-        }
-        return(currentMinute < billingMinute) && (currentMinute >= (billingMinute - SHUTDOWN_TIME_BUFFER));
+    public static boolean canShutdownServer(@Nonnull String createdStr, LocalDateTime currentTime) {
+        final LocalDateTime created = LocalDateTime.from(DateTimeFormatter.ISO_DATE_TIME.parse(createdStr))
+                .atOffset(ZoneOffset.UTC).toLocalDateTime();
+        long diff = Duration.between(created, currentTime.atOffset(ZoneOffset.UTC).toLocalDateTime()).toMinutes() % 60;
+        return (60 - SHUTDOWN_TIME_BUFFER) <= diff;
     }
 
     /**

diff --git a/src/main/java/cloud/dnation/jenkins/plugins/hetzner/shutdown/BeforeHourWrapsPolicy.java b/src/main/java/cloud/dnation/jenkins/plugins/hetzner/shutdown/BeforeHourWrapsPolicy.java
@@ -55,7 +55,7 @@ public long check(final AbstractCloudComputer c) {
             final HetznerServerAgent agent = (HetznerServerAgent) c.getNode();
             if (c.isIdle() && agent != null && agent.getServerInstance() != null) {
                 if (Helper.canShutdownServer(agent.getServerInstance().getServerDetail().getCreated(),
-                        LocalDateTime.now().getMinute())) {
+                        LocalDateTime.now())) {
                     log.info("Disconnecting {}", c.getName());
                     try {
                         agent.terminate();

diff --git a/src/test/java/cloud/dnation/jenkins/plugins/hetzner/HelperTest.java b/src/test/java/cloud/dnation/jenkins/plugins/hetzner/HelperTest.java
@@ -15,9 +15,12 @@
  */
 package cloud.dnation.jenkins.plugins.hetzner;
 
+import com.google.common.base.Strings;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
 
 import static org.junit.Assert.*;
 
@@ -29,22 +32,28 @@ public void testExtractPublicKey() throws IOException {
         assertEquals(pubKeyStr, Helper.getSSHPublicKeyFromPrivate(privKeyStr, null));
     }
 
+    private static LocalDateTime time(String str) {
+        return LocalDateTime.from(DateTimeFormatter.ISO_DATE_TIME.parse(str + "+02:00"));
+    }
     @Test
     public void testCanShutdownServer() {
         //server started at 10:41 UTC, so it can be shutdown in minutes 36-40
         String str = "2022-05-21T10:41:19+00:00";
-        assertFalse(Helper.canShutdownServer(str, 50));
-        assertTrue(Helper.canShutdownServer(str, 36));
-        assertTrue(Helper.canShutdownServer(str, 40));
-        assertFalse(Helper.canShutdownServer(str, 41));
-        //server started at 10:01 so it can be shutdown in minutes 56-00
+        assertFalse(Helper.canShutdownServer(str, time("2022-05-21T10:50:11")));
+        assertTrue(Helper.canShutdownServer(str, time("2022-05-21T11:36:19")));
+        assertTrue(Helper.canShutdownServer(str, time("2022-05-21T11:40:13")));
+        assertFalse(Helper.canShutdownServer(str, time("2022-05-21T10:41:14")));
+        //server started at 10:01, so it can be shutdown in minutes 56-00
         str = "2022-05-21T10:01:19+00:00";
-        assertFalse(Helper.canShutdownServer(str, 55));
-        assertTrue(Helper.canShutdownServer(str, 56));
-        assertTrue(Helper.canShutdownServer(str, 59));
-        assertTrue(Helper.canShutdownServer(str, 0));
-        assertFalse(Helper.canShutdownServer(str, 1));
-        assertFalse(Helper.canShutdownServer(str, 32));
+        assertFalse(Helper.canShutdownServer(str, time("2022-05-21T10:55:15")));
+        assertTrue(Helper.canShutdownServer(str, time("2022-05-21T10:56:19")));
+        assertTrue(Helper.canShutdownServer(str, time("2022-05-21T10:59:17")));
+        assertFalse(Helper.canShutdownServer(str, time("2022-05-21T10:00:18")));
+        assertTrue(Helper.canShutdownServer(str, time("2022-05-21T11:00:18")));
+        assertFalse(Helper.canShutdownServer(str, time("2022-05-21T10:01:19")));
+        assertFalse(Helper.canShutdownServer(str, time("2022-05-21T10:32:20")));
+        str = "2022-08-08T11:03:55+00:00";
+        assertFalse(Helper.canShutdownServer(str, time("2022-08-08T11:03:02")));
+        assertTrue(Helper.canShutdownServer(str, time("2022-08-08T11:59:02")));
     }
-
 }