Fix public path + fixes to tests

LEDfan · LEDfan · commit e8e79cf1c78b · 2024-03-07T17:23:58.000+01:00
diff --git a/src/main/java/eu/openanalytics/containerproxy/api/ProxyController.java b/src/main/java/eu/openanalytics/containerproxy/api/ProxyController.java
@@ -269,7 +269,7 @@ public ResponseEntity<ApiResponse<Proxy>> startProxy(@PathVariable String proxyS
     }
 
     private String getPublicPath(String proxyId) {
-        return contextPathHelper.withEndingSlash() + "api/route/" + proxyId;
+        return contextPathHelper.withEndingSlash() + "api/route/" + proxyId + "/";
     }
 
     public static class ParametersBody {
diff --git a/src/main/java/eu/openanalytics/containerproxy/util/Retrying.java b/src/main/java/eu/openanalytics/containerproxy/util/Retrying.java
@@ -37,7 +37,7 @@ public static boolean retry(Attempt job, int maxDelay, boolean retryOnException)
 
     public static boolean retry(Attempt job, int maxDelay, String logMessage, int logAfterAttmepts, boolean retryOnException) {
         boolean retVal = false;
-        RuntimeException exception = null;
+        Exception exception = null;
         int maxAttempts = numberOfAttempts(maxDelay);
         for (int currentAttempt = 0; currentAttempt < maxAttempts; currentAttempt++) {
             delay(currentAttempt); // delay here so that we don't delay for the last iteration
@@ -47,16 +47,16 @@ public static boolean retry(Attempt job, int maxDelay, String logMessage, int lo
                     exception = null;
                     break;
                 }
-            } catch (RuntimeException e) {
+            } catch (Exception e) {
                 if (retryOnException) exception = e;
-                else throw e;
+                else throw new RuntimeException(e);
             }
             if (currentAttempt > logAfterAttmepts && logMessage != null) {
                 log.info(String.format("Retry: %s (%d/%d)", logMessage, currentAttempt, maxAttempts));
             }
         }
         if (exception == null) return retVal;
-        else throw exception;
+        else throw new RuntimeException(exception);
     }
 
     public static void delay(Integer attempt) {
@@ -84,7 +84,7 @@ public static int numberOfAttempts(Integer maxDelay) {
 
     @FunctionalInterface
     public interface Attempt {
-        boolean attempt(int currentAttempt, int maxAttempts);
+        boolean attempt(int currentAttempt, int maxAttempts) throws Exception;
     }
 
 }
diff --git a/src/test/java/eu/openanalytics/containerproxy/test/helpers/NotInternalOnlyTestStrategy.java b/src/test/java/eu/openanalytics/containerproxy/test/helpers/NotInternalOnlyTestStrategy.java
@@ -22,7 +22,6 @@
 
 import eu.openanalytics.containerproxy.backend.strategy.IProxyTestStrategy;
 import eu.openanalytics.containerproxy.model.runtime.Proxy;
-import eu.openanalytics.containerproxy.service.StructuredLogger;
 import eu.openanalytics.containerproxy.util.Retrying;
 
 import java.net.HttpURLConnection;
@@ -32,31 +31,24 @@
 
 public class NotInternalOnlyTestStrategy implements IProxyTestStrategy {
 
-    private final StructuredLogger log = StructuredLogger.create(getClass());
-
     @Override
     public boolean testProxy(Proxy proxy) {
         URI targetURI = proxy.getTargets().get("");
 
         return Retrying.retry((currentAttempt, maxAttempts) -> {
-            try {
-                if (proxy.getStatus().isUnavailable()) {
-                    // proxy got stopped while loading -> no need to try to connect it since the container will already be deleted
-                    return true;
-                }
-                URL testURL = new URL(targetURI.toString() + "/");
-                HttpURLConnection connection = ((HttpURLConnection) testURL.openConnection());
-                connection.setInstanceFollowRedirects(false);
-                int responseCode = connection.getResponseCode();
-                if (Arrays.asList(200, 301, 302, 303, 307, 308).contains(responseCode)) {
-                    return true;
-                }
-                log.warn(proxy, "Error during test strategy test: status code: " + responseCode);
-            } catch (Exception e) {
-                return false;
+            if (proxy.getStatus().isUnavailable()) {
+                // proxy got stopped while loading -> no need to try to connect it since the container will already be deleted
+                return true;
+            }
+            URL testURL = new URL(targetURI.toString() + "/");
+            HttpURLConnection connection = ((HttpURLConnection) testURL.openConnection());
+            connection.setInstanceFollowRedirects(false);
+            int responseCode = connection.getResponseCode();
+            if (Arrays.asList(200, 301, 302, 303, 307, 308).contains(responseCode)) {
+                return true;
             }
             return false;
-        }, 60_000);
+        }, 60_000, "Container unresponsive", 10, true);
     }
 
 }
diff --git a/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnEcs.java b/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnEcs.java
@@ -116,7 +116,7 @@ public void launchProxy() {
                 Assertions.assertEquals(3, environment.size());
                 Assertions.assertEquals("demo", environment.get("SHINYPROXY_USERNAME"));
                 Assertions.assertEquals("", environment.get("SHINYPROXY_USERGROUPS"));
-                Assertions.assertEquals("/api/route/" + proxy.getId(), environment.get("SHINYPROXY_PUBLIC_PATH")); // TODO no ending slash?
+                Assertions.assertEquals("/api/route/" + proxy.getId() + "/", environment.get("SHINYPROXY_PUBLIC_PATH"));
                 Assertions.assertEquals("openanalytics/shinyproxy-integration-test-app", containerDefinition.image());
                 Assertions.assertNull(containerDefinition.privileged()); // fargate does not support privileged
                 Assertions.assertNull(containerDefinition.hostname());
diff --git a/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnKube.java b/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnKube.java
@@ -190,6 +190,7 @@ public void launchProxyWithEnv() {
             Assertions.assertEquals("VALUE2", env.get("VAR2").getValue());
             Assertions.assertTrue(env.containsKey("VAR3"));
             Assertions.assertEquals("VALUE3", env.get("VAR3").getValue());
+            Assertions.assertEquals("/api/route/" + proxy.getId() + "/", env.get("SHINYPROXY_PUBLIC_PATH").getValue());
 
             inst.proxyService.stopProxy(null, proxy, true).run();
 
diff --git a/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationProxySharing.java b/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationProxySharing.java
@@ -345,7 +345,8 @@ public void testCleanupPendingDelegateProxies(String backend, Map<String, String
 
                     // wait for scale-up to finish
                     // pending proxy should be marked as ToRemove
-                    waitUntilNumberOfDelegateProxies(inst, 2, 1, 0, 1);
+                    // wait for long time, leader election can take long
+                    waitUntilNumberOfDelegateProxies(inst, 2, 1, 0, 1, 120_000);
 
                     // cleanup should remove old proxy
                     proxySharingScaler.enableCleanup();
@@ -622,14 +623,18 @@ private void waitUntilNumberOfDelegateProxies(ShinyProxyInstance inst, int numDe
     }
 
     private void waitUntilNumberOfDelegateProxies(ShinyProxyInstance inst, int numDelegateProxies, int numAvailable, int numPending, int numRemove) {
+        waitUntilNumberOfDelegateProxies(inst, numDelegateProxies, numAvailable, numPending, numRemove, 60_000);
+    }
+
+    private void waitUntilNumberOfDelegateProxies(ShinyProxyInstance inst, int numDelegateProxies, int numAvailable, int numPending, int numRemove, int delay) {
         TestProxySharingScaler proxySharingScaler = inst.getBean("proxySharingScaler_myApp", TestProxySharingScaler.class);
         IDelegateProxyStore delegateProxyStore = proxySharingScaler.getDelegateProxyStore();
         boolean noPendingSeats = Retrying.retry((c, m) -> {
             return delegateProxyStore.getAllDelegateProxies().size() == numDelegateProxies
                 && delegateProxyStore.getAllDelegateProxies(DelegateProxyStatus.Available).count() == numAvailable
                 && delegateProxyStore.getAllDelegateProxies(DelegateProxyStatus.Pending).count() == numPending
                 && delegateProxyStore.getAllDelegateProxies(DelegateProxyStatus.ToRemove).count() == numRemove;
-        }, 90_000, "assert number delegated proxies", 1, true);
+        }, 120_000, "assert number delegated proxies", 1, true);
         Assertions.assertTrue(noPendingSeats,
             String.format("Total: %s, Available: %s, Pending: %s, ToRemove: %s",
                 delegateProxyStore.getAllDelegateProxies().size(),

Original file line number	Diff line number	Diff line change
`@@ -269,7 +269,7 @@ public ResponseEntity<ApiResponse<Proxy>> startProxy(@PathVariable String proxyS`
`269`	`269`	`}`
`270`	`270`
`271`	`271`	`private String getPublicPath(String proxyId) {`
`272`		`- return contextPathHelper.withEndingSlash() + "api/route/" + proxyId;`
	`272`	`+ return contextPathHelper.withEndingSlash() + "api/route/" + proxyId + "/";`
`273`	`273`	`}`
`274`	`274`
`275`	`275`	`public static class ParametersBody {`
Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ public static boolean retry(Attempt job, int maxDelay, boolean retryOnException)`
`37`	`37`
`38`	`38`	`public static boolean retry(Attempt job, int maxDelay, String logMessage, int logAfterAttmepts, boolean retryOnException) {`
`39`	`39`	`boolean retVal = false;`
`40`		`- RuntimeException exception = null;`
	`40`	`+ Exception exception = null;`
`41`	`41`	`int maxAttempts = numberOfAttempts(maxDelay);`
`42`	`42`	`for (int currentAttempt = 0; currentAttempt < maxAttempts; currentAttempt++) {`
`43`	`43`	`delay(currentAttempt); // delay here so that we don't delay for the last iteration`
`@@ -47,16 +47,16 @@ public static boolean retry(Attempt job, int maxDelay, String logMessage, int lo`
`47`	`47`	`exception = null;`
`48`	`48`	`break;`
`49`	`49`	`}`
`50`		`- } catch (RuntimeException e) {`
	`50`	`+ } catch (Exception e) {`
`51`	`51`	`if (retryOnException) exception = e;`
`52`		`- else throw e;`
	`52`	`+ else throw new RuntimeException(e);`
`53`	`53`	`}`
`54`	`54`	`if (currentAttempt > logAfterAttmepts && logMessage != null) {`
`55`	`55`	`log.info(String.format("Retry: %s (%d/%d)", logMessage, currentAttempt, maxAttempts));`
`56`	`56`	`}`
`57`	`57`	`}`
`58`	`58`	`if (exception == null) return retVal;`
`59`		`- else throw exception;`
	`59`	`+ else throw new RuntimeException(exception);`
`60`	`60`	`}`
`61`	`61`
`62`	`62`	`public static void delay(Integer attempt) {`
`@@ -84,7 +84,7 @@ public static int numberOfAttempts(Integer maxDelay) {`
`84`	`84`
`85`	`85`	`@FunctionalInterface`
`86`	`86`	`public interface Attempt {`
`87`		`- boolean attempt(int currentAttempt, int maxAttempts);`
	`87`	`+ boolean attempt(int currentAttempt, int maxAttempts) throws Exception;`
`88`	`88`	`}`
`89`	`89`
`90`	`90`	`}`