[grid] Assigning node to a random free port if not specified explicit…

…ly. Fixes #5783
SeleniumHQ · May 7, 2018 · 2f90373 · 2f90373
1 parent 2e76dd0
commit 2f90373
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 15 deletions.
diff --git a/java/server/src/org/openqa/grid/common/defaults/DefaultNodeWebDriver.json b/java/server/src/org/openqa/grid/common/defaults/DefaultNodeWebDriver.json
@@ -28,7 +28,7 @@
   ],
   "proxy": "org.openqa.grid.selenium.proxy.DefaultRemoteProxy",
   "maxSession": 5,
-  "port": 5555,
+  "port": -1,
   "register": true,
   "registerCycle": 5000,
   "hub": "http://localhost:4444",

diff --git a/java/server/src/org/openqa/grid/internal/utils/configuration/GridNodeConfiguration.java b/java/server/src/org/openqa/grid/internal/utils/configuration/GridNodeConfiguration.java
@@ -55,9 +55,9 @@ public class GridNodeConfiguration extends GridConfiguration {
   static final String DEFAULT_ROLE = "node";
 
   /**
-   * Default hub port
+   * Default node port, -1 means random free port
    */
-  static final Integer DEFAULT_PORT = 5555;
+  static final Integer DEFAULT_PORT = -1;
 
   /**
    * Default node polling

diff --git a/java/server/src/org/openqa/grid/selenium/GridLauncherV3.java b/java/server/src/org/openqa/grid/selenium/GridLauncherV3.java
@@ -36,6 +36,7 @@
 import org.openqa.grid.web.Hub;
 import org.openqa.grid.web.servlet.DisplayHelpServlet;
 import org.openqa.selenium.internal.BuildInfo;
+import org.openqa.selenium.net.PortProber;
 import org.openqa.selenium.remote.server.SeleniumServer;
 import org.openqa.selenium.remote.server.log.LoggingOptions;
 import org.openqa.selenium.remote.server.log.TerseFormatter;
@@ -303,6 +304,9 @@ public CommonCliOptions getOptions() {
           @Override
           public Stoppable launch() throws Exception {
             GridNodeConfiguration configuration = options.toConfiguration();
+            if (configuration.port == null || configuration.port == -1) {
+              configuration.port = PortProber.findFreePort();
+            }
             log.info(String.format(
                 "Launching a Selenium Grid node on port %s", configuration.port));
             SelfRegisteringRemote remote = new SelfRegisteringRemote(configuration);

diff --git a/java/server/test/org/openqa/grid/common/RegistrationRequestTest.java b/java/server/test/org/openqa/grid/common/RegistrationRequestTest.java
@@ -22,7 +22,6 @@
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertTrue;
 
 import org.junit.Assume;
 import org.junit.Test;
@@ -52,7 +51,7 @@ public void getConfigAsTests() throws Exception {
     RegistrationRequest req = new RegistrationRequest(config);
 
     int c = req.getConfiguration().cleanUpCycle;
-    assertTrue(c == 1);
+    assertEquals(1, c);
 
     String url2 = req.getConfiguration().getRemoteHost();
     assertEquals(url2, url.toString());
@@ -104,7 +103,7 @@ public void commandLineParamDefault() {
     assertEquals(4444, req.getConfiguration().getHubPort().longValue());
     // the node defaults to current IP.
     assertNotNull(req.getConfiguration().host);
-    assertEquals(5555, req.getConfiguration().port.longValue());
+    assertEquals(-1, req.getConfiguration().port.longValue());
   }
 
   @Test
@@ -180,7 +179,7 @@ public void testBuildWithConfiguration() {
     actualConfig.host = null;
     // make sure this merge protected value was preserved, then reset it for the final assert
     assertEquals(1234, actualConfig.port.intValue());
-    actualConfig.port = 5555;
+    actualConfig.port = -1;
 
     // merge actualConfig onto it.. which is what build(config) should do
     GridNodeConfiguration expectedConfig = new GridNodeConfiguration();

diff --git a/java/server/test/org/openqa/grid/internal/utils/configuration/GridNodeConfigurationTest.java b/java/server/test/org/openqa/grid/internal/utils/configuration/GridNodeConfigurationTest.java
@@ -73,7 +73,7 @@ public void testLoadFromJson() throws IOException {
     assertEquals(1234, gnc.port.intValue());
     assertEquals(5, gnc.maxSession.intValue());
     assertEquals("dummyhost", gnc.host);
-    assertTrue(gnc.capabilities.size() == 1);
+    assertEquals(1, gnc.capabilities.size());
     assertEquals("firefox", gnc.capabilities.get(0).getBrowserName());
     assertEquals(5L, gnc.capabilities.get(0).getCapability("maxInstances"));
   }
@@ -193,7 +193,7 @@ public void testAsJson() {
         + "\"browserTimeout\":0,"
         + "\"debug\":false,"
         + "\"host\":\"0.0.0.0\","
-        + "\"port\":5555,"
+        + "\"port\":-1,"
         + "\"role\":\"node\","
         + "\"timeout\":1800}",
         MAP_TYPE);
@@ -302,12 +302,6 @@ public void testGetHubPort_forNullConfig() {
     gnc.getHubPort();
   }
 
-  @Test
-  public void testGetRemoteHost_forNullConfig() {
-    GridNodeConfiguration gnc = new GridNodeConfiguration();
-    assertEquals("http://localhost:5555", gnc.getRemoteHost());
-  }
-
   @Test
   public void testMergeWithRealValues() {
     GridNodeConfiguration gnc = new GridNodeConfiguration();