ZOOKEEPER-4303: Allow configuring client port to 0

https://issues.apache.org/jira/browse/ZOOKEEPER-4303

Allows specifying an explicit port 0 for the client port or secure client port, which will default to operating system behavior of finding an unbound port. Modified ZKServerEmbedded to report the actual port instead of the configured port.

Author: Mike Drob <mdrob@apple.com>

Reviewers: Kezhu Wang <kezhuw@gmail.com>, Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>

Closes #1868 from madrob/zookeeper-4303

zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxnFactory.java

@@ -663,6 +663,10 @@ public void configure(InetSocketAddress addr, int maxcc, int backlog, boolean se
         } else {
             ss.socket().bind(addr, listenBacklog);
         }
+        if (addr.getPort() == 0) {
+            // We're likely bound to a different port than was requested, so log that too
+            LOG.info("bound to port {}", ss.getLocalAddress());
+        }
         ss.configureBlocking(false);
         acceptThread = new AcceptThread(ss, addr, selectorThreads);
     }

zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMain.java

@@ -244,6 +244,23 @@ ServerCnxnFactory getSecureCnxnFactory() {
         return secureCnxnFactory;
     }
 
+    // VisibleForTesting
+    public int getClientPort() {
+        if (cnxnFactory != null) {
+            return cnxnFactory.getLocalPort();
+        }
+        return 0;
+    }
+
+    // VisibleForTesting
+    public int getSecureClientPort() {
+        if (secureCnxnFactory != null) {
+            return secureCnxnFactory.getLocalPort();
+        }
+        return 0;
+    }
+
+
     /**
      * Shutdowns properly the service, this method is not a public API.
      */

zookeeper-server/src/main/java/org/apache/zookeeper/server/embedded/ZooKeeperServerEmbeddedImpl.java

@@ -1,6 +1,7 @@
 package org.apache.zookeeper.server.embedded;
 
 import java.io.OutputStream;
+import java.net.InetSocketAddress;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Map;
@@ -45,6 +46,9 @@ class ZooKeeperServerEmbeddedImpl implements ZooKeeperServerEmbedded {
     private final ExitHandler exitHandler;
     private volatile boolean stopping;
 
+    private int boundClientPort;
+    private int boundSecureClientPort;
+
     ZooKeeperServerEmbeddedImpl(Properties p, Path baseDir, ExitHandler exitHandler) throws Exception {
         if (!p.containsKey("dataDir")) {
             p.put("dataDir", baseDir.resolve("data").toAbsolutePath().toString());
@@ -103,6 +107,8 @@ protected QuorumPeer getQuorumPeer() throws SaslException {
                         @Override
                         public void start() {
                             super.start();
+                            boundClientPort = getClientPort();
+                            boundSecureClientPort = getSecureClientPort();
                             LOG.info("ZK Server {} started", this);
                             started.complete(null);
                         }
@@ -142,6 +148,8 @@ public void run() {
                 @Override
                 public void serverStarted() {
                     LOG.info("ZK Server started");
+                    boundClientPort = getClientPort();
+                    boundSecureClientPort = getSecureClientPort();
                     started.complete(null);
                 }
             };
@@ -184,22 +192,22 @@ public void run() {
 
     @Override
     public String getConnectionString() {
-        if (config.getClientPortAddress() != null) {
-            String raw = config.getClientPortAddress().getHostString() + ":" + config.getClientPortAddress().getPort();
-            return raw.replace("0.0.0.0", "localhost");
-        } else {
-            throw new IllegalStateException("No client address is configured");
-        }
+        return prettifyConnectionString(config.getClientPortAddress(), boundClientPort);
     }
 
     @Override
     public String getSecureConnectionString() {
-        if (config.getSecureClientPortAddress() != null) {
-            String raw = config.getSecureClientPortAddress().getHostString() + ":" + config.getSecureClientPortAddress().getPort();
-            return raw.replace("0.0.0.0", "localhost");
-        } else {
-            throw new IllegalStateException("No client address is configured");
+        return prettifyConnectionString(config.getSecureClientPortAddress(), boundSecureClientPort);
+    }
+
+    private String prettifyConnectionString(InetSocketAddress confAddress, int boundPort) {
+        if (confAddress != null) {
+            return confAddress.getHostString()
+                .replace("0.0.0.0", "localhost")
+                .replace("0:0:0:0:0:0:0:0", "localhost")
+                + ":" + boundPort;
         }
+        throw new IllegalStateException("No client address is configured");
     }
 
     @Override

zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java

@@ -2139,6 +2139,13 @@ public int getClientPort() {
         return -1;
     }
 
+    public int getSecureClientPort() {
+        if (secureCnxnFactory != null) {
+            return secureCnxnFactory.getLocalPort();
+        }
+        return -1;
+    }
+
     public void setTxnFactory(FileTxnSnapLog factory) {
         this.logFactory = factory;
     }

zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java

@@ -271,8 +271,8 @@ public static String getVersionFromFilename(String filename) {
      * @throws ConfigException
      */
     public void parseProperties(Properties zkProp) throws IOException, ConfigException {
-        int clientPort = 0;
-        int secureClientPort = 0;
+        Integer clientPort = null;
+        Integer secureClientPort = null;
         int observerMasterPort = 0;
         String clientPortAddress = null;
         String secureClientPortAddress = null;
@@ -427,7 +427,7 @@ public void parseProperties(Properties zkProp) throws IOException, ConfigExcepti
             dataLogDir = dataDir;
         }
 
-        if (clientPort == 0) {
+        if (clientPort == null) {
             LOG.info("clientPort is not set");
             if (clientPortAddress != null) {
                 throw new IllegalArgumentException("clientPortAddress is set but clientPort is not set");
@@ -440,7 +440,7 @@ public void parseProperties(Properties zkProp) throws IOException, ConfigExcepti
             LOG.info("clientPortAddress is {}", formatInetAddr(this.clientPortAddress));
         }
 
-        if (secureClientPort == 0) {
+        if (secureClientPort == null) {
             LOG.info("secureClientPort is not set");
             if (secureClientPortAddress != null) {
                 throw new IllegalArgumentException("secureClientPortAddress is set but secureClientPort is not set");

zookeeper-server/src/test/java/org/apache/zookeeper/server/embedded/ZookeeperServerEmbeddedTest.java

@@ -17,12 +17,17 @@
  */
 package org.apache.zookeeper.server.embedded;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.endsWith;
+import static org.hamcrest.Matchers.not;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.Properties;
 import org.apache.zookeeper.PortAssignment;
 import org.apache.zookeeper.test.ClientBase;
+import org.junit.function.ThrowingRunnable;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
@@ -95,4 +100,31 @@ public void testStart() throws Exception {
 
     }
 
+    @Test
+    public void testBindPortZero() throws Exception {
+        final Properties configZookeeper = new Properties();
+        final ZooKeeperServerEmbedded.ZookKeeperServerEmbeddedBuilder builder = ZooKeeperServerEmbedded.builder()
+            .baseDir(baseDir)
+            .configuration(configZookeeper)
+            .exitHandler(ExitHandler.LOG_ONLY);
+
+        // Unconfigured client port will still fail
+        try (ZooKeeperServerEmbedded zkServer = builder.build()) {
+            zkServer.start();
+            assertThrows(IllegalStateException.class, new ThrowingRunnable() {
+                @Override
+                public void run() throws Throwable {
+                    zkServer.getConnectionString();
+                }
+            });
+        }
+
+        // Explicit port zero should work
+        configZookeeper.put("clientPort", "0");
+        try (ZooKeeperServerEmbedded zkServer = builder.build()) {
+            zkServer.start();
+            assertThat(zkServer.getConnectionString(), not(endsWith(":0")));
+            assertTrue(ClientBase.waitForServerUp(zkServer.getConnectionString(), 60000));
+        }
+    }
 }