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

6 files changed, 83 insertions, 15 deletions

diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxnFactory.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxnFactory.java
index 57495c10a..f081ab24b 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxnFactory.java
+++ b/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);
     }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMain.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMain.java
index 7bd30baa6..d6cbbe4a1 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMain.java
+++ b/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.
      */
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/embedded/ZooKeeperServerEmbeddedImpl.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/embedded/ZooKeeperServerEmbeddedImpl.java
index ec6ae637a..ad41a3228 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/embedded/ZooKeeperServerEmbeddedImpl.java
+++ b/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
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java
index 18e97bb2f..220d813f4 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java
+++ b/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;
     }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
index 1b37f291f..d478c409c 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
+++ b/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");
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/embedded/ZookeeperServerEmbeddedTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/embedded/ZookeeperServerEmbeddedTest.java
index d9868b235..00bfced0c 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/embedded/ZookeeperServerEmbeddedTest.java
+++ b/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));
+        }
+    }
 }