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 class NIOServerCnxnFactory extends ServerCnxnFactory {
         } 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 @@ public class ZooKeeperServerMain {
         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 @@ class ZooKeeperServerEmbeddedImpl implements ZooKeeperServerEmbedded {
                         @Override
                         public void start() {
                             super.start();
+                            boundClientPort = getClientPort();
+                            boundSecureClientPort = getSecureClientPort();
                             LOG.info("ZK Server {} started", this);
                             started.complete(null);
                         }
@@ -142,6 +148,8 @@ class ZooKeeperServerEmbeddedImpl implements ZooKeeperServerEmbedded {
                 @Override
                 public void serverStarted() {
                     LOG.info("ZK Server started");
+                    boundClientPort = getClientPort();
+                    boundSecureClientPort = getSecureClientPort();
                     started.complete(null);
                 }
             };
@@ -184,22 +192,22 @@ class ZooKeeperServerEmbeddedImpl implements ZooKeeperServerEmbedded {
 
     @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 class QuorumPeer extends ZooKeeperThread implements QuorumStats.Provider
         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 class QuorumPeerConfig {
      * @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 class QuorumPeerConfig {
             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 class QuorumPeerConfig {
             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 class ZookeeperServerEmbeddedTest {
 
     }
 
+    @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));
+        }
+    }
 }