Allocate ports dynamically

7 files changed, 46 insertions, 52 deletions

diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/rpc/RpcConnector.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/rpc/RpcConnector.java
index e7feab9926d..edb9fba5307 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/rpc/RpcConnector.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/rpc/RpcConnector.java
@@ -33,7 +33,7 @@ public class RpcConnector extends AbstractComponent {
         Spec spec = new Spec(config.port());
         try {
             acceptor = supervisor.listen(spec);
-            log.log(DEBUG, "Listening on " + spec.host() + ":" + spec.port());
+            log.log(DEBUG, "Listening on " + spec.host() + ":" + acceptor.port());
         } catch (ListenFailedException e) {
             stop();
             log.log(INFO, "Failed listening at " + spec.host() + ":" + spec.port());
@@ -41,6 +41,10 @@ public class RpcConnector extends AbstractComponent {
         }
     }
 
+    public int port() {
+        return acceptor.port();
+    }
+
     /**
      * Adds a method. If a method with the same name already exists, it will be replaced.
      * @param method The method to add.
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/IntegrationTester.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/IntegrationTester.java
index df1ef9e5035..997c02f7768 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/IntegrationTester.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/IntegrationTester.java
@@ -44,8 +44,6 @@ public class IntegrationTester implements  AutoCloseable {
     static final String SERVICE_1_CONFIG_ID = "container/qrserver.0";
     static final String SERVICE_2_CONFIG_ID = "storage/cluster.storage/storage/0";
 
-    private final int httpPort;
-    private final int rpcPort;
     private final RpcConnector connector;
     private final MockHttpServer mockHttpServer;
     private final VespaServices vespaServices;
@@ -54,16 +52,11 @@ public class IntegrationTester implements  AutoCloseable {
         HttpMetricFetcher.CONNECTION_TIMEOUT = 60000; // 60 secs in unit tests
     }
 
-    IntegrationTester(int httpPort, int rpcPort) {
-        if (httpPort == 0 || rpcPort == 0) {
-            throw new IllegalArgumentException("http port and rpc port must be defined");
-        }
-        this.httpPort = httpPort;
-        this.rpcPort = rpcPort;
+    IntegrationTester() {
         try {
-            mockHttpServer = new MockHttpServer(httpPort, null, STATE_PATH);
+            mockHttpServer = new MockHttpServer(null, STATE_PATH);
         } catch (IOException e) {
-            throw new RuntimeException("Unable to start web server on port:" + httpPort);
+            throw new RuntimeException("Unable to start web server");
         }
 
         vespaServices = new VespaServices(servicesConfig(), monitoringConfig(), null);
@@ -74,7 +67,8 @@ public class IntegrationTester implements  AutoCloseable {
         NodeDimensions nodeDimensions = new NodeDimensions(nodeDimensionsConfig());
 
         connector = new RpcConnector(rpcConnectorConfig());
-        RpcServer server = new RpcServer(connector, vespaServices, new MetricsManager(vespaServices, vespaMetrics, externalMetrics,                                                                                     appDimensions, nodeDimensions));
+        RpcServer server = new RpcServer(connector, vespaServices,
+                new MetricsManager(vespaServices, vespaMetrics, externalMetrics, appDimensions, nodeDimensions));
     }
 
     MockHttpServer httpServer() {
@@ -91,7 +85,7 @@ public class IntegrationTester implements  AutoCloseable {
 
     private RpcConnectorConfig rpcConnectorConfig() {
         return new RpcConnectorConfig.Builder()
-                .port(rpcPort)
+                .port(0)
                 .build();
     }
 
@@ -113,8 +107,8 @@ public class IntegrationTester implements  AutoCloseable {
 
     private VespaServicesConfig servicesConfig() {
         return new VespaServicesConfig.Builder()
-                .service(createService(toServiceId("qrserver"), SERVICE_1_CONFIG_ID, httpPort))
-                .service(createService(toServiceId("storagenode"), SERVICE_2_CONFIG_ID, httpPort))
+                .service(createService(toServiceId("qrserver"), SERVICE_1_CONFIG_ID, httpPort()))
+                .service(createService(toServiceId("storagenode"), SERVICE_2_CONFIG_ID, httpPort()))
                 .build();
     }
 
@@ -140,4 +134,12 @@ public class IntegrationTester implements  AutoCloseable {
         return new NodeDimensionsConfig.Builder().build();
     }
 
+    public int rpcPort() {
+        return connector.port();
+    }
+
+    public int httpPort() {
+        return mockHttpServer.port();
+    }
+
 }
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcHealthMetricsTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcHealthMetricsTest.java
index c882de349c8..55e14381183 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcHealthMetricsTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcHealthMetricsTest.java
@@ -36,14 +36,9 @@ public class RpcHealthMetricsTest {
     private static final String WANTED_RPC_RESPONSE =
             getFileContents("rpc-json-output-check.json").trim();
 
-    // see factory/doc/port-ranges.txt
-    private static final int httpPort = 18635;
-    private static final int rpcPort = 18636;
-
     @Test
     public void expected_response_is_returned() {
-        try (IntegrationTester tester = new IntegrationTester(httpPort, rpcPort)) {
-
+        try (IntegrationTester tester = new IntegrationTester()) {
             MockHttpServer mockHttpServer = tester.httpServer();
             mockHttpServer.setResponse(HEALTH_OK_RESPONSE);
             List<VespaService> services = tester.vespaServices().getInstancesById(SERVICE_1_CONFIG_ID);
@@ -61,22 +56,22 @@ public class RpcHealthMetricsTest {
             assertThat("Status should be failed" + h.getMessage(), h.isOk(), is(false));
             assertThat(h.getMessage(), is("SOMETHING FAILED"));
 
-            String jsonRPCMessage = getHealthMetrics(qrserver.getMonitoringName());
+            String jsonRPCMessage = getHealthMetrics(tester, qrserver.getMonitoringName());
             assertThat(jsonRPCMessage, is(WANTED_RPC_RESPONSE));
         }
     }
 
     @Test
     public void non_existent_service_name_returns_an_error_message() {
-        try (IntegrationTester tester = new IntegrationTester(httpPort, rpcPort)) {
-            String jsonRPCMessage = getHealthMetrics("non-existing service");
+        try (IntegrationTester tester = new IntegrationTester()) {
+            String jsonRPCMessage = getHealthMetrics(tester, "non-existing service");
             assertThat(jsonRPCMessage, is("105: No service with name 'non-existing service'"));
         }
     }
 
-    private String getHealthMetrics(String service) {
+    private String getHealthMetrics(IntegrationTester tester, String service) {
         Supervisor supervisor = new Supervisor(new Transport());
-        Target target = supervisor.connect(new Spec("localhost", rpcPort));
+        Target target = supervisor.connect(new Spec("localhost", tester.rpcPort()));
         Request req = new Request("getHealthMetricsForYamas");
         req.parameters().add(new StringValue(service));
         String returnValue;
@@ -93,5 +88,4 @@ public class RpcHealthMetricsTest {
 
         return returnValue;
     }
-
 }
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcMetricsTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcMetricsTest.java
index 6d1b4f3d3b7..d4777618546 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcMetricsTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcMetricsTest.java
@@ -44,13 +44,9 @@ public class RpcMetricsTest {
     private static final String METRICS_RESPONSE_CCL =
             getFileContents("metrics-storage-simple.json").trim();
 
-    // see factory/doc/port-ranges.txt
-    private static final int httpPort = 18633;
-    private static final int rpcPort = 18634;
-
     @Test
     public void testGetMetrics() throws Exception {
-        try (IntegrationTester tester = new IntegrationTester(httpPort, rpcPort)) {
+        try (IntegrationTester tester = new IntegrationTester()) {
             tester.httpServer().setResponse(METRICS_RESPONSE_CCL);
             List<VespaService> services = tester.vespaServices().getInstancesById(SERVICE_1_CONFIG_ID);
 
@@ -68,7 +64,7 @@ public class RpcMetricsTest {
 
             // Setup RPC client
             Supervisor supervisor = new Supervisor(new Transport());
-            Target target = supervisor.connect(new Spec("localhost", rpcPort));
+            Target target = supervisor.connect(new Spec("localhost", tester.rpcPort()));
 
             verifyMetricsFromRpcRequest(qrserver, target);
 
@@ -137,7 +133,7 @@ public class RpcMetricsTest {
 
     @Test
     public void testGetAllMetricNames() {
-        try (IntegrationTester tester = new IntegrationTester(httpPort, rpcPort)) {
+        try (IntegrationTester tester = new IntegrationTester()) {
 
             tester.httpServer().setResponse(METRICS_RESPONSE_CCL);
             List<VespaService> services = tester.vespaServices().getInstancesById(SERVICE_1_CONFIG_ID);
@@ -154,7 +150,7 @@ public class RpcMetricsTest {
 
             // Setup RPC
             Supervisor supervisor = new Supervisor(new Transport());
-            Target target = supervisor.connect(new Spec("localhost", rpcPort));
+            Target target = supervisor.connect(new Spec("localhost", tester.rpcPort()));
 
             String response = getAllMetricNamesForService(services.get(0).getMonitoringName(), VESPA_CONSUMER_ID, target);
             assertThat(response, is("foo.count=ON;output-name=foo_count,bar.count=OFF,"));
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ContainerServiceTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ContainerServiceTest.java
index 725501aacaa..5a174412729 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ContainerServiceTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ContainerServiceTest.java
@@ -22,8 +22,7 @@ import static org.junit.Assert.assertThat;
  */
 public class ContainerServiceTest {
 
-    private MockHttpServer service;
-    private int csPort;
+    private MockHttpServer httpServer;
 
     @BeforeClass
     public static void init() {
@@ -32,10 +31,9 @@ public class ContainerServiceTest {
 
     @Before
     public void setupHTTPServer() {
-        csPort = 18637; // see factory/doc/port-ranges.txt
         try {
             String response = getFileContents("metrics-container-state-multi-chain.json");
-            service = new MockHttpServer(csPort, response, METRICS_PATH);
+            httpServer = new MockHttpServer(response, METRICS_PATH);
         } catch (Exception e) {
             e.printStackTrace();
         }
@@ -44,7 +42,7 @@ public class ContainerServiceTest {
     @Test
     public void testMultipleQueryDimensions() throws JSONException {
         int count = 0;
-        VespaService service = VespaService.create("service1", "id", csPort);
+        VespaService service = VespaService.create("service1", "id", httpServer.port());
         for (Metric m : service.getMetrics().getMetrics()) {
             if (m.getName().equals("queries.rate")) {
                 count++;
@@ -63,6 +61,6 @@ public class ContainerServiceTest {
 
     @After
     public void shutdown() {
-        this.service.close();
+        this.httpServer.close();
     }
 }
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MockHttpServer.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MockHttpServer.java
index fdf2fae3081..c1cfd4c170c 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MockHttpServer.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MockHttpServer.java
@@ -23,23 +23,25 @@ public class MockHttpServer {
     /**
      * Mock http server that will return response as body
      *
-     * @param port     the port to listen to
-     * @param response the response to return along with 200 OK
      * @param path     the file path that the server will accept requests for. E.g /state/v1/metrics
      */
-    public MockHttpServer(int port, String response, String path) throws IOException {
+    public MockHttpServer(String response, String path) throws IOException {
         this.response = response;
-        this.server = HttpServer.create(new InetSocketAddress(port), 10);
+        this.server = HttpServer.create(new InetSocketAddress(0), 10);
         this.server.createContext(path, new MyHandler());
         this.server.setExecutor(null); // creates a default executor
         this.server.start();
-        System.out.println("Started web server on port " + port);
+        System.out.println("Started web server on port " + port());
     }
 
     public synchronized void setResponse(String r) {
         this.response = r;
     }
 
+    public int port() {
+        return server.getAddress().getPort();
+    }
+
     public void close() {
         this.server.stop(0);
     }
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServiceTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServiceTest.java
index a0c6b5333cc..ac839e595f9 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServiceTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServiceTest.java
@@ -18,8 +18,7 @@ import static org.junit.Assert.assertThat;
  * @author Unknown
  */
 public class VespaServiceTest {
-    private MockHttpServer service;
-    private int csPort;
+    private MockHttpServer httpServer;
     private static final String response;
 
     static {
@@ -29,9 +28,8 @@ public class VespaServiceTest {
 
     @Before
     public void setupHTTPServer() {
-        csPort = 18632; // see factory/doc/port-ranges.txt
         try {
-            service = new MockHttpServer(csPort, response, METRICS_PATH);
+            httpServer = new MockHttpServer(response, METRICS_PATH);
         } catch (Exception e) {
             e.printStackTrace();
         }
@@ -56,7 +54,7 @@ public class VespaServiceTest {
     @Test
     // TODO: Make it possible to test this without running a HTTP server to create the response
     public void testMetricsFetching() {
-        VespaService service = VespaService.create("service1", "id", csPort);
+        VespaService service = VespaService.create("service1", "id", httpServer.port());
         Metrics metrics = service.getMetrics();
         assertThat(metrics.getMetric("queries.count").getValue().intValue(), is(28));
 
@@ -70,7 +68,7 @@ public class VespaServiceTest {
 
     @After
     public void shutdown() {
-        this.service.close();
+        this.httpServer.close();
     }
 
 }