Merge pull request #574 from yahoo/bratseth/protect-zk-access-part-2

Bratseth/protect zk access part 2

12 files changed, 210 insertions, 71 deletions

diff --git a/filedistributionmanager/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionManager.java b/filedistributionmanager/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionManager.java
index 63e93f1a358..23721919620 100644
--- a/filedistributionmanager/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionManager.java
+++ b/filedistributionmanager/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionManager.java
@@ -14,8 +14,6 @@ public class FileDistributionManager {
 
     private final static boolean available;
 
-    private long nativeFileDistributionManager;
-
     private final File applicationDirectory;
     private final String appId;
     private final Lock lock;
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java
index fa6d598d776..acc2848a782 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java
@@ -259,11 +259,14 @@ public final class Node {
 
     public enum Type {
         
-        /** A node to be assigned to a tenant to run application workloads */
-        tenant,
-        
         /** A host of a set of (docker) tenant nodes */
-        host;
+        host,
+        
+        /** Nodes running the shared proxy layer */
+        proxy,
+
+        /** A node to be assigned to a tenant to run application workloads */
+        tenant
 
     }
 
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 327246e3099..1936050d5f2 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
@@ -14,7 +14,6 @@ import com.yahoo.vespa.hosted.provision.node.filter.NodeFilter;
 import com.yahoo.vespa.hosted.provision.node.filter.NodeListFilter;
 import com.yahoo.vespa.hosted.provision.node.filter.StateFilter;
 import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient;
-import com.yahoo.vespa.zookeeper.ZooKeeperServer;
 
 import java.time.Clock;
 import java.util.ArrayList;
@@ -56,7 +55,6 @@ import java.util.stream.Collectors;
 public class NodeRepository extends AbstractComponent {
 
     private final CuratorDatabaseClient zkClient;
-    private final Curator curator;
 
     /**
      * Creates a node repository form a zookeeper provider.
@@ -73,7 +71,6 @@ public class NodeRepository extends AbstractComponent {
      */
     public NodeRepository(NodeFlavors flavors, Curator curator, Clock clock) {
         this.zkClient = new CuratorDatabaseClient(flavors, curator, clock);
-        this.curator = curator;
 
         // read and write all nodes to make sure they are stored in the latest version of the serialized format
         for (Node.State state : Node.State.values())
@@ -129,9 +126,7 @@ public class NodeRepository extends AbstractComponent {
                 throw new IllegalArgumentException("Cannot add " + node.hostname() + ": A node with this name already exists");
         }
         try (Mutex lock = lockUnallocated()) {
-            List<Node> addedNodes = zkClient.addNodes(nodes);
-            updateAllowedHosts();
-            return addedNodes;
+            return zkClient.addNodes(nodes);
         }
     }
 
@@ -254,9 +249,7 @@ public class NodeRepository extends AbstractComponent {
         if ( ! nodeToRemove.isPresent()) return false;
 
         try (Mutex lock = lock(nodeToRemove.get())) {
-            boolean removed = zkClient.removeNode(nodeToRemove.get().state(), hostname);
-            updateAllowedHosts();
-            return removed;
+            return zkClient.removeNode(nodeToRemove.get().state(), hostname);
         }
     }
 
@@ -348,20 +341,4 @@ public class NodeRepository extends AbstractComponent {
         return node.allocation().isPresent() ? lock(node.allocation().get().owner()) : lockUnallocated();
     }
     
-    private void updateAllowedHosts() {
-        StringBuilder s = new StringBuilder();
-        
-        // Add tenant hosts
-        for (Node node : getNodes(Node.Type.tenant))
-            s.append(node.hostname()).append(",");
-
-        // Add the zooKeeper servers
-        for (String hostPort : curator.connectionSpec().split(","))
-            s.append(hostPort.split(":")[0]).append(",");
-
-        if (s.length() > 0)
-            s.setLength(s.length()-1); // remove last comma
-        System.setProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY, s.toString());
-    }
-
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java
index 2bc826adc30..3e6881b912b 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java
@@ -7,6 +7,7 @@ import com.yahoo.config.provision.Deployer;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostLivenessTracker;
 import com.yahoo.config.provision.Zone;
+import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.orchestrator.Orchestrator;
 import com.yahoo.vespa.service.monitor.ServiceMonitor;
@@ -24,6 +25,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
 
     private final NodeFailer nodeFailer;
     private final ApplicationMaintainer applicationMaintainer;
+    private final ZooKeeperAccessMaintainer zooKeeperAccessMaintainer;
     private final ReservationExpirer reservationExpirer;
     private final InactiveExpirer inactiveExpirer;
     private final RetiredExpirer retiredExpirer;
@@ -31,18 +33,19 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
     private final DirtyExpirer dirtyExpirer;
 
     @Inject
-    public NodeRepositoryMaintenance(NodeRepository nodeRepository, Deployer deployer, 
+    public NodeRepositoryMaintenance(NodeRepository nodeRepository, Deployer deployer, Curator curator,
                                      HostLivenessTracker hostLivenessTracker, ServiceMonitor serviceMonitor, 
                                      Zone zone, Orchestrator orchestrator) {
-        this(nodeRepository, deployer, hostLivenessTracker, serviceMonitor, zone, Clock.systemUTC(), orchestrator);
+        this(nodeRepository, deployer, curator, hostLivenessTracker, serviceMonitor, zone, Clock.systemUTC(), orchestrator);
     }
 
-    public NodeRepositoryMaintenance(NodeRepository nodeRepository, Deployer deployer, 
+    public NodeRepositoryMaintenance(NodeRepository nodeRepository, Deployer deployer, Curator curator,
                                      HostLivenessTracker hostLivenessTracker, ServiceMonitor serviceMonitor, 
                                      Zone zone, Clock clock, Orchestrator orchestrator) {
         DefaultTimes defaults = new DefaultTimes(zone.environment());
         nodeFailer = new NodeFailer(deployer, hostLivenessTracker, serviceMonitor, nodeRepository, fromEnv("fail_grace").orElse(defaults.failGrace), clock, orchestrator);
         applicationMaintainer = new ApplicationMaintainer(deployer, nodeRepository, fromEnv("redeploy_frequency").orElse(defaults.redeployFrequency));
+        zooKeeperAccessMaintainer = new ZooKeeperAccessMaintainer(nodeRepository, curator, fromEnv("zookeeper_access_maintenance_interval").orElse(defaults.zooKeeperAccessMaintenanceInterval));
         reservationExpirer = new ReservationExpirer(nodeRepository, clock, fromEnv("reservation_expiry").orElse(defaults.reservationExpiry));
         retiredExpirer = new RetiredExpirer(nodeRepository, deployer, clock, fromEnv("retired_expiry").orElse(defaults.retiredExpiry));
         inactiveExpirer = new InactiveExpirer(nodeRepository, clock, fromEnv("inactive_expiry").orElse(defaults.inactiveExpiry));
@@ -59,6 +62,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
     public void deconstruct() {
         nodeFailer.deconstruct();
         applicationMaintainer.deconstruct();
+        zooKeeperAccessMaintainer.deconstruct();
         reservationExpirer.deconstruct();
         inactiveExpirer.deconstruct();
         retiredExpirer.deconstruct();
@@ -73,6 +77,8 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
 
         /** The time a node must be continuously nonresponsive before it is failed */
         private final Duration failGrace;
+        
+        private final Duration zooKeeperAccessMaintenanceInterval;
 
         private final Duration reservationExpiry;
         private final Duration inactiveExpiry;
@@ -84,18 +90,20 @@ public class NodeRepositoryMaintenance extends AbstractComponent {
             if (environment.equals(Environment.prod)) {
                 // These values are to avoid losing data (retired), and to be able to return an application
                 // back to a previous state fast (inactive)
-                redeployFrequency = Duration.ofMinutes(30);
                 failGrace = Duration.ofMinutes(60);
+                redeployFrequency = Duration.ofMinutes(30);
+                zooKeeperAccessMaintenanceInterval = Duration.ofMinutes(1);
                 reservationExpiry = Duration.ofMinutes(15);
                 inactiveExpiry = Duration.ofHours(4); // enough time for the application owner to discover and redeploy
                 retiredExpiry = Duration.ofDays(4); // enough time to migrate data
                 failedExpiry = Duration.ofDays(4); // enough time to recover data even if it happens friday night
                 dirtyExpiry = Duration.ofHours(2); // enough time to clean the node
             } else {
-                redeployFrequency = Duration.ofMinutes(30);
-                failGrace = Duration.ofMinutes(60);
                 // These values ensure tests and development is not delayed due to nodes staying around
                 // Use non-null values as these also determine the maintenance interval
+                failGrace = Duration.ofMinutes(60);
+                redeployFrequency = Duration.ofMinutes(30);
+                zooKeeperAccessMaintenanceInterval = Duration.ofSeconds(10);
                 reservationExpiry = Duration.ofMinutes(10); // Need to be long enough for deployment to be finished for all config model versions
                 inactiveExpiry = Duration.ofMinutes(1);
                 retiredExpiry = Duration.ofMinutes(1);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainer.java
new file mode 100644
index 00000000000..085e3e4dac8
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainer.java
@@ -0,0 +1,52 @@
+package com.yahoo.vespa.hosted.provision.maintenance;
+
+import com.yahoo.vespa.curator.Curator;
+import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.NodeRepository;
+import com.yahoo.vespa.zookeeper.ZooKeeperServer;
+
+import java.time.Duration;
+
+/**
+ * Maintains the system property which tells ZooKeeper which nodes should have access to it.
+ * These are the zokeeper servers and all tenant and proxy nodes. This is maintained in the background because
+ * nodes could be added or removed on another server. 
+ * 
+ * We could limit access to the <i>active</i> subset of nodes, but that 
+ * does not seem to have any particular operational or security benefits and might make it more problematic
+ * for this job to be behind actual changes to the active set of nodes.
+ * 
+ * @author bratseth
+ */
+public class ZooKeeperAccessMaintainer extends Maintainer {
+
+    private final Curator curator;
+    
+    public ZooKeeperAccessMaintainer(NodeRepository nodeRepository, Curator curator, Duration maintenanceInterval) {
+        super(nodeRepository, maintenanceInterval);
+        this.curator = curator;
+    }
+    
+    @Override
+    protected void maintain() {
+        StringBuilder hostList = new StringBuilder();
+
+        for (Node node : nodeRepository().getNodes(Node.Type.tenant))
+            hostList.append(node.hostname()).append(",");
+        for (Node node : nodeRepository().getNodes(Node.Type.proxy))
+            hostList.append(node.hostname()).append(",");
+        for (String hostPort : curator.connectionSpec().split(","))
+            hostList.append(hostPort.split(":")[0]).append(",");
+
+        if (hostList.length() > 0)
+            hostList.setLength(hostList.length()-1); // remove last comma
+
+        System.setProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY, hostList.toString());
+    }
+
+    @Override
+    public String toString() {
+        return "ZooKeeper access maintainer";
+    }
+
+}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java
index 82475538c04..53379ff7413 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java
@@ -286,6 +286,7 @@ public class NodeSerializer {
         switch (typeString) {
             case "tenant" : return Node.Type.tenant;
             case "host" : return Node.Type.host;
+            case "proxy" : return Node.Type.proxy;
             default : throw new IllegalArgumentException("Unknown node type '" + typeString + "'");
         }
     }
@@ -293,6 +294,7 @@ public class NodeSerializer {
         switch (type) {
             case tenant: return "tenant";
             case host: return "host";
+            case proxy: return "proxy";
         }
         throw new IllegalArgumentException("Serialized form of '" + type + "' not defined");
     }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java
index c8bed1b9b9a..52327a578b6 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java
@@ -204,6 +204,7 @@ public class NodesApiHandler extends LoggingRequestHandler {
         switch (object.asString()) {
             case "tenant" : return Node.Type.tenant;
             case "host" : return Node.Type.host;
+            case "proxy" : return Node.Type.proxy;
             default: throw new IllegalArgumentException("Unknown node type '" + object.asString() + "'");
         }
     }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
index a8b3cced1a1..864b2a357cc 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
@@ -188,6 +188,7 @@ class NodesResponse extends HttpResponse {
         switch(type) {
             case tenant: return "tenant";
             case host: return "host";
+            case proxy: return "proxy";
             default:
                 throw new RuntimeException("New type added to enum, not implemented in NodesResponse: " + type.name());
         }
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 dc4744c9eaf..805de69e081 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
@@ -28,39 +28,19 @@ public class NodeRepositoryTest {
 
     @Test
     public void nodeRepositoryTest() {
-        NodeFlavors nodeFlavors = new NodeFlavors(createConfig());
-        Clock clock = new ManualClock();
-        MockCurator curator = new MockCurator();
-        curator.setConnectionSpec("server1:1234,server2:5678");
-        NodeRepository nodeRepository = new NodeRepository(nodeFlavors, curator, clock);
+        NodeRepositoryTester tester = new NodeRepositoryTester();
+        assertEquals(0, tester.getNodes(Node.Type.tenant).size());
 
-        assertEquals(0, nodeRepository.getNodes(Node.Type.tenant).size());
+        tester.addNode("id1", "host1", "default", Node.Type.tenant);
+        tester.addNode("id2", "host2", "default", Node.Type.tenant);
+        tester.addNode("id3", "host3", "default", Node.Type.tenant);
 
-        List<Node> nodes = new ArrayList<>();
-        nodes.add(nodeRepository.createNode("id1", "host1", Optional.empty(), new Configuration(nodeFlavors.getFlavorOrThrow("default")), Node.Type.tenant));
-        nodes.add(nodeRepository.createNode("id2", "host2", Optional.empty(), new Configuration(nodeFlavors.getFlavorOrThrow("default")), Node.Type.tenant));
-        nodes.add(nodeRepository.createNode("id3", "host3", Optional.empty(), new Configuration(nodeFlavors.getFlavorOrThrow("default")), Node.Type.tenant));
-        nodeRepository.addNodes(nodes);
-
-        assertEquals(3, nodeRepository.getNodes(Node.Type.tenant).size());
-        assertEquals(asSet("host1,host2,host3,server1,server2"), asSet(System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY)));
+        assertEquals(3, tester.getNodes(Node.Type.tenant).size());
         
-        nodeRepository.move("host2", Node.State.parked);
-        assertTrue(nodeRepository.remove("host2"));
+        tester.nodeRepository().move("host2", Node.State.parked);
+        assertTrue(tester.nodeRepository().remove("host2"));
 
-        assertEquals(2, nodeRepository.getNodes(Node.Type.tenant).size());
-        assertEquals(asSet("host1,host3,server1,server2"), asSet(System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY)));
+        assertEquals(2, tester.getNodes(Node.Type.tenant).size());
     }
     
-    private Set<String> asSet(String s) {
-        return new HashSet<>(Arrays.asList(s.split(",")));
-    }
-
-    private NodeRepositoryConfig createConfig() {
-        FlavorConfigBuilder b = new FlavorConfigBuilder();
-        b.addFlavor("default", 2., 4., 100, "BARE_METAL").cost(3);
-        b.addFlavor("small", 1., 2., 50, "BARE_METAL").cost(2);
-        return b.build();
-    }
-
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java
new file mode 100644
index 00000000000..fa757047134
--- /dev/null
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java
@@ -0,0 +1,54 @@
+package com.yahoo.vespa.hosted.provision;
+
+import com.yahoo.test.ManualClock;
+import com.yahoo.vespa.config.nodes.NodeRepositoryConfig;
+import com.yahoo.vespa.curator.mock.MockCurator;
+import com.yahoo.vespa.hosted.provision.node.Configuration;
+import com.yahoo.vespa.hosted.provision.node.NodeFlavors;
+import com.yahoo.vespa.hosted.provision.testutils.FlavorConfigBuilder;
+
+import java.time.Clock;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * @author bratseth
+ */
+public class NodeRepositoryTester {
+
+    private final NodeFlavors nodeFlavors;
+    private final NodeRepository nodeRepository;
+    private final Clock clock;
+    private final MockCurator curator;
+    
+    
+    public NodeRepositoryTester() {
+        nodeFlavors = new NodeFlavors(createConfig());
+        clock = new ManualClock();
+        curator = new MockCurator();
+        curator.setConnectionSpec("server1:1234,server2:5678");
+        nodeRepository = new NodeRepository(nodeFlavors, curator, clock);
+    }
+    
+    public NodeRepository nodeRepository() { return nodeRepository; }
+    public MockCurator curator() { return curator; }
+    
+    public List<Node> getNodes(Node.Type type, Node.State ... inState) {
+        return nodeRepository.getNodes(type, inState);
+    }
+    
+    public Node addNode(String id, String hostname, String flavor, Node.Type type) {
+        Node node = nodeRepository.createNode(id, hostname, Optional.empty(), 
+                                              new Configuration(nodeFlavors.getFlavorOrThrow(flavor)), type);
+        return nodeRepository.addNodes(Collections.singletonList(node)).get(0);
+    }
+
+    private NodeRepositoryConfig createConfig() {
+        FlavorConfigBuilder b = new FlavorConfigBuilder();
+        b.addFlavor("default", 2., 4., 100, "BARE_METAL").cost(3);
+        b.addFlavor("small", 1., 2., 50, "BARE_METAL").cost(2);
+        return b.build();
+    }
+
+}
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainerTest.java
new file mode 100644
index 00000000000..4d57980fff1
--- /dev/null
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainerTest.java
@@ -0,0 +1,62 @@
+package com.yahoo.vespa.hosted.provision.maintenance;
+
+import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.NodeRepositoryTester;
+import com.yahoo.vespa.zookeeper.ZooKeeperServer;
+import org.junit.Test;
+
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertNull;
+
+/**
+ * @author bratseth
+ */
+public class ZooKeeperAccessMaintainerTest {
+
+    @Test
+    public void test() {
+        NodeRepositoryTester tester = new NodeRepositoryTester();
+        tester.curator().setConnectionSpec("server1:1234,server2:5678");
+        ZooKeeperAccessMaintainer maintainer = new ZooKeeperAccessMaintainer(tester.nodeRepository(), 
+                                                                             tester.curator(), Duration.ofHours(1));
+        assertNull(System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY));
+        maintainer.maintain();
+        assertEquals(asSet("server1,server2"), asSet(System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY)));
+
+        tester.addNode("id1", "host1", "default", Node.Type.tenant);
+        tester.addNode("id2", "host2", "default", Node.Type.tenant);
+        tester.addNode("id3", "host3", "default", Node.Type.tenant);
+        maintainer.maintain();
+
+        assertEquals(3, tester.getNodes(Node.Type.tenant).size());
+        assertEquals(0, tester.getNodes(Node.Type.proxy).size());
+        assertEquals(asSet("host1,host2,host3,server1,server2"), asSet(System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY)));
+
+        tester.addNode("proxy1", "host4", "default", Node.Type.proxy);
+        tester.addNode("proxy2", "host5", "default", Node.Type.proxy);
+        maintainer.maintain();
+
+        assertEquals(3, tester.getNodes(Node.Type.tenant).size());
+        assertEquals(2, tester.getNodes(Node.Type.proxy).size());
+        assertEquals(asSet("host1,host2,host3,host4,host5,server1,server2"), asSet(System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY)));
+
+        tester.nodeRepository().move("host2", Node.State.parked);
+        assertTrue(tester.nodeRepository().remove("host2"));
+        maintainer.maintain();
+
+        assertEquals(2, tester.getNodes(Node.Type.tenant).size());
+        assertEquals(2, tester.getNodes(Node.Type.proxy).size());
+        assertEquals(asSet("host1,host3,host4,host5,server1,server2"), asSet(System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY)));
+    }
+
+    private Set<String> asSet(String s) {
+        return new HashSet<>(Arrays.asList(s.split(",")));
+    }
+
+}
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
index d8561c67767..b52f755bdc4 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
@@ -27,11 +27,11 @@ public class RestrictedServerCnxnFactory extends NIOServerCnxnFactory {
     
     @Override
     protected NIOServerCnxn createConnection(SocketChannel socket, SelectionKey selection) throws IOException {
-        String remoteHost = ((InetSocketAddress)socket.getRemoteAddress()).getHostName(); // TODO: Move this line down
+        String remoteHost = ((InetSocketAddress)socket.getRemoteAddress()).getHostName();
 
         String zookeeperClients = System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY);
         if (zookeeperClients == null || zookeeperClients.isEmpty()) {
-            log.info("Allowing connection to ZooKeeper from " + remoteHost + ", as " + ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY + " is not set"); // TODO: Remove this line
+            log.fine("Allowing connection to ZooKeeper from " + remoteHost + ", as " + ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY + " is not set");
             return super.createConnection(socket, selection); // client checking is not activated
         }
 
@@ -39,10 +39,11 @@ public class RestrictedServerCnxnFactory extends NIOServerCnxnFactory {
         if ( ! remoteHost.equals("localhost") && ! zooKeeperClients.contains(remoteHost)) {
             String errorMessage = "Rejecting connection to ZooKeeper from " + remoteHost +
                                   ": This cluster only allow connection from hosts in: " + zooKeeperClients;
-            log.warning(errorMessage);
-            throw new IllegalArgumentException(errorMessage);
+            // log.info(errorMessage);
+            // throw new IllegalArgumentException(errorMessage);
+            log.fine("Would reject if activated: " + errorMessage);
         }
-        log.info("Allowing connection to ZooKeeper from " + remoteHost + ", as it is in " + zookeeperClients); // TODO: Remove this line
+        log.fine("Allowing connection to ZooKeeper from " + remoteHost + ", as it is in " + zookeeperClients);
         return super.createConnection(socket, selection);
     }