ZOOKEEPER-4303: Allow configuring client port to 0

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));
+        }
+    }
 }