Include connection spec hosts correctly in permissible zk clients

4 files changed, 19 insertions, 5 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
index b0e178bea92..9edf1eeaf01 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
@@ -358,7 +358,7 @@ public class NodeRepository extends AbstractComponent {
             s.append(node.hostname()).append(",");
 
         // Add the zooKeeper servers
-        for (String hostPort : curator.connectionSpec().split("/"))
+        for (String hostPort : curator.connectionSpec().split(","))
             s.append(hostPort.split(":")[0]).append(",");
 
         if (s.length() > 0)
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
index a8c839a6f44..dc4744c9eaf 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
@@ -30,7 +30,9 @@ public class NodeRepositoryTest {
     public void nodeRepositoryTest() {
         NodeFlavors nodeFlavors = new NodeFlavors(createConfig());
         Clock clock = new ManualClock();
-        NodeRepository nodeRepository = new NodeRepository(nodeFlavors, new MockCurator(), clock);
+        MockCurator curator = new MockCurator();
+        curator.setConnectionSpec("server1:1234,server2:5678");
+        NodeRepository nodeRepository = new NodeRepository(nodeFlavors, curator, clock);
 
         assertEquals(0, nodeRepository.getNodes(Node.Type.tenant).size());
 
@@ -41,13 +43,13 @@ public class NodeRepositoryTest {
         nodeRepository.addNodes(nodes);
 
         assertEquals(3, nodeRepository.getNodes(Node.Type.tenant).size());
-        assertEquals(asSet("host1,host2,host3"), asSet(System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY)));
+        assertEquals(asSet("host1,host2,host3,server1,server2"), asSet(System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY)));
         
         nodeRepository.move("host2", Node.State.parked);
         assertTrue(nodeRepository.remove("host2"));
 
         assertEquals(2, nodeRepository.getNodes(Node.Type.tenant).size());
-        assertEquals(asSet("host1,host3"), asSet(System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY)));
+        assertEquals(asSet("host1,host3,server1,server2"), asSet(System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY)));
     }
     
     private Set<String> asSet(String s) {
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java
index 66734036ce5..135154bba92 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java
@@ -25,6 +25,7 @@ import java.time.Duration;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.TimeUnit;
@@ -80,6 +81,7 @@ public class Curator {
     }
 
     private Curator(String connectionSpec) {
+        Objects.requireNonNull(connectionSpec, "The curator connection spec cannot be null");
         this.connectionSpec = connectionSpec;
         this.serverCount = connectionSpec.split(",").length;
         validateConnectionSpec(connectionSpec);
@@ -110,7 +112,10 @@ public class Curator {
     /** Returns the number of zooKeeper servers in this cluster */
     public int serverCount() { return serverCount; }
 
-    /** Returns the servers in this cluster as a comma-separated list of host:port strings */
+    /** 
+     * Returns the servers in this cluster as a comma-separated list of host:port strings. 
+     * This may be empty but never null 
+     */
     public String connectionSpec() { return connectionSpec; }
 
     /** For internal use; prefer creating a {@link CuratorCounter} */
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java
index 92db255a67f..4615eb4e34a 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java
@@ -101,6 +101,7 @@ public class MockCurator extends Curator {
     private boolean shouldTimeoutOnEnter = false;
     private int monotonicallyIncreasingNumber = 0;
     private final boolean stableOrdering;
+    private String connectionSpec = "";
 
     /** The file system used by this mock to store zookeeper files and directories */
     private final MemoryFileSystem fileSystem = new MemoryFileSystem();
@@ -143,6 +144,12 @@ public class MockCurator extends Curator {
     public Optional<DistributedAtomicLong> counter(String path) {
         return Optional.ofNullable(atomicCounters.get(path));
     }
+    
+    /** Assigns the connection string, which must be on the form host1:port,host2:port ... */
+    public void setConnectionSpec(String connectionSpec) { this.connectionSpec = connectionSpec; }
+
+    @Override
+    public String connectionSpec() { return connectionSpec; }
 
     // ----- Start of adaptor methods from Curator to the mock file system -----