summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJon Bratseth <jonbratseth@yahoo.com>2016-09-04 18:22:33 +0200
committerGitHub <noreply@github.com>2016-09-04 18:22:33 +0200
commite0dc287f3ee28e59310f964ef31c76c92f7c7ec5 (patch)
tree232447ae5c0d9ac4b951690e69b5e03640708ba3
parentede80119d58a3c4a4eb25c120cf32ac5dff09e84 (diff)
Revert "Revert "Revert "Revert "Revert "Revert "Bratseth/lockdown zk""""""
-rw-r--r--config-model/src/main/java/com/yahoo/config/model/provision/Host.java21
-rw-r--r--config-model/src/main/java/com/yahoo/config/model/provision/Hosts.java108
-rw-r--r--config-model/src/main/java/com/yahoo/config/model/provision/HostsXmlProvisioner.java21
-rw-r--r--config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java8
-rw-r--r--config-model/src/main/java/com/yahoo/config/model/provision/SingleNodeProvisioner.java5
-rw-r--r--config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java1
-rw-r--r--config-model/src/test/java/com/yahoo/config/model/provision/HostsXmlProvisionerTest.java4
-rw-r--r--config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java2
-rw-r--r--config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java2
-rw-r--r--config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java10
-rw-r--r--node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java33
-rw-r--r--node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java66
-rw-r--r--zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java14
-rw-r--r--zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java7
-rw-r--r--zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java56
-rw-r--r--zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java6
-rw-r--r--zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java3
17 files changed, 257 insertions, 110 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/provision/Host.java b/config-model/src/main/java/com/yahoo/config/model/provision/Host.java
index afe4de7ef0d..360853d0f79 100644
--- a/config-model/src/main/java/com/yahoo/config/model/provision/Host.java
+++ b/config-model/src/main/java/com/yahoo/config/model/provision/Host.java
@@ -1,40 +1,39 @@
// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.config.model.provision;
+import com.google.common.collect.ImmutableList;
+
import java.util.ArrayList;
import java.util.List;
/**
- * A hostname with zero or more aliases.
+ * A hostname with zero or more aliases. This is immutable.
*
* @author hmusum
*/
public class Host {
private final String hostname;
- private final List<String> hostAliases;
+ private final ImmutableList<String> aliases;
public Host(String hostname) {
this.hostname = hostname;
- this.hostAliases = new ArrayList<>();
+ this.aliases = ImmutableList.of();
}
public Host(String hostname, List<String> hostAliases) {
this.hostname = hostname;
- this.hostAliases = hostAliases;
+ this.aliases = ImmutableList.copyOf(hostAliases);
}
- public String getHostname() {
- return hostname;
- }
+ public String hostname() { return hostname; }
- public List<String> getHostAliases() {
- return hostAliases;
- }
+ /** Returns an immutable list of the aliases of this node, which may be empty but never null */
+ public List<String> aliases() { return aliases; }
@Override
public String toString() {
- return hostname + " (aliases: " + hostAliases + ")";
+ return hostname + (aliases.size() > 0 ? " (aliases: " + aliases + ")" : "" );
}
}
diff --git a/config-model/src/main/java/com/yahoo/config/model/provision/Hosts.java b/config-model/src/main/java/com/yahoo/config/model/provision/Hosts.java
index bfb77612d31..1a145e6c83e 100644
--- a/config-model/src/main/java/com/yahoo/config/model/provision/Hosts.java
+++ b/config-model/src/main/java/com/yahoo/config/model/provision/Hosts.java
@@ -1,8 +1,9 @@
// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.config.model.provision;
+import com.google.common.collect.ImmutableMap;
+import com.yahoo.cloud.config.ConfigserverConfig;
import com.yahoo.config.model.builder.xml.XmlHelper;
-import com.yahoo.log.LogLevel;
import com.yahoo.net.HostName;
import com.yahoo.text.XML;
import com.yahoo.vespa.model.builder.xml.dom.VespaDomBuilder;
@@ -17,26 +18,60 @@ import java.util.*;
import java.util.logging.Logger;
/**
- * TODO: What is this?
+ * A collection of hosts
*
- * @author hmusum
+ * @author bratseth
*/
public class Hosts {
public static final Logger log = Logger.getLogger(Hosts.class.getPackage().toString());
- private final HashMap<String, Host> hosts = new LinkedHashMap<>();
- private final Map<String, String> alias2hostname = new LinkedHashMap<>();
- private final Map<String, Host> alias2host = new LinkedHashMap<>();
+ private final ImmutableMap<String, Host> hosts;
+ public Hosts(Collection<Host> hosts) {
+ validateAliases(hosts);
+
+ ImmutableMap.Builder<String, Host> hostsBuilder = new ImmutableMap.Builder<>();
+ for (Host host : hosts)
+ hostsBuilder.put(host.hostname(), host);
+ this.hosts = hostsBuilder.build();
+
+ System.setProperty("zookeeper.vespa.clients", toHostnameString(hosts)); // See com.yahoo.vespa.zookeeper.ZooKeeperServer
+ }
+
+ /** Throw IllegalArgumentException if host aliases breaks invariants */
+ private void validateAliases(Collection<Host> hosts) {
+ Set<String> aliases = new HashSet<>();
+ for (Host host : hosts) {
+ if (host.aliases().size() > 0) {
+ if (host.aliases().size() < 1)
+ throw new IllegalArgumentException("Host '" + host.hostname() + "' must have at least one <alias> tag.");
+ for (String alias : host.aliases()) {
+ if (aliases.contains(alias))
+ throw new IllegalArgumentException("Alias '" + alias + "' is used by multiple hosts.");
+ aliases.add(alias);
+ }
+ }
+ }
+ }
+
+ private String toHostnameString(Collection<Host> hosts) {
+ StringBuilder b = new StringBuilder();
+ for (Host host : hosts)
+ b.append(host.hostname()).append(",");
+ if (b.length() > 0)
+ b.setLength(b.length() - 1); // remove last comma
+ return b.toString();
+ }
+
/**
* Builds host system from a hosts.xml file
*
* @param hostsFile a reader for host from application package
* @return the HostSystem for this application package
*/
- public static Hosts getHosts(Reader hostsFile) {
- Hosts hosts = new Hosts();
+ public static Hosts readFrom(Reader hostsFile) {
+ List<Host> hosts = new ArrayList<>();
Document doc;
try {
doc = XmlHelper.getDocumentBuilder().parse(new InputSource(hostsFile));
@@ -55,62 +90,13 @@ public class Hosts {
if (hostAliases.isEmpty()) {
throw new IllegalArgumentException("No host aliases defined for host '" + name + "'");
}
- Host host = new Host(name, hostAliases);
- hosts.addHost(host, hostAliases);
- }
- log.log(LogLevel.DEBUG, "Created hosts:" + hosts);
- return hosts;
- }
-
- public Collection<Host> getHosts() {
- return hosts.values();
- }
-
- /**
- * Adds one host to this host system.
- *
- * @param host The host to add
- * @param aliases The aliases for this host.
- */
- public void addHost(Host host, List<String> aliases) {
- hosts.put(host.getHostname(), host);
- if ((aliases != null) && (aliases.size() > 0)) {
- addHostAliases(aliases, host);
+ hosts.add(new Host(name, hostAliases));
}
+ return new Hosts(hosts);
}
- /**
- * Add all aliases for one host
- *
- * @param hostAliases A list of host aliases
- * @param host The Host instance to add the alias for
- */
- private void addHostAliases(List<String> hostAliases, Host host) {
- if (hostAliases.size() < 1) {
- throw new RuntimeException("Host '" + host.getHostname() + "' must have at least one <alias> tag.");
- }
- for (String alias : hostAliases) {
- addHostAlias(alias, host);
- }
- }
-
- /**
- * Adds an alias for the given host
- *
- * @param alias alias (string) for a Host
- * @param host the {@link Host} to add the alias for
- */
- protected void addHostAlias(String alias, Host host) {
- if (alias2hostname.containsKey(alias)) {
- throw new RuntimeException("Alias '" + alias + "' must be used for only one host!");
- }
- alias2hostname.put(alias, host.getHostname());
- alias2host.put(alias, host);
- }
-
- public Map<String, Host> getAlias2host() {
- return alias2host;
- }
+ /** Returns an immutable collection of the hosts of this */
+ public Collection<Host> asCollection() { return hosts.values(); }
@Override
public String toString() {
diff --git a/config-model/src/main/java/com/yahoo/config/model/provision/HostsXmlProvisioner.java b/config-model/src/main/java/com/yahoo/config/model/provision/HostsXmlProvisioner.java
index bf630b74272..32a7e79d278 100644
--- a/config-model/src/main/java/com/yahoo/config/model/provision/HostsXmlProvisioner.java
+++ b/config-model/src/main/java/com/yahoo/config/model/provision/HostsXmlProvisioner.java
@@ -22,27 +22,24 @@ public class HostsXmlProvisioner implements HostProvisioner {
public static final String IMPLICIT_ADMIN_HOSTALIAS = "INTERNAL_VESPA_IMPLICIT_ADMIN";
public HostsXmlProvisioner(Reader hosts) {
- this.hosts = Hosts.getHosts(hosts);
+ this.hosts = Hosts.readFrom(hosts);
}
@Override
public HostSpec allocateHost(String alias) {
- /**
- * Some special rules to allow no admin elements as well
- * as jdisc element without nodes.
- */
+ // Some special rules to allow no admin elements as well as jdisc element without nodes.
if (alias.equals(IMPLICIT_ADMIN_HOSTALIAS)) {
- if (hosts.getHosts().size() > 1) {
- throw new IllegalArgumentException("More than 1 host specified (" + hosts.getHosts().size() + ") and <admin> not specified");
+ if (hosts.asCollection().size() > 1) {
+ throw new IllegalArgumentException("More than 1 host specified (" + hosts.asCollection().size() + ") and <admin> not specified");
} else {
return host2HostSpec(getFirstHost());
}
} else if (alias.equals(Container.SINGLENODE_CONTAINER_SERVICESPEC)) {
return host2HostSpec(getFirstHost());
}
- for (Host host : hosts.getHosts()) {
- if (host.getHostAliases().contains(alias)) {
- return new HostSpec(host.getHostname(), host.getHostAliases());
+ for (Host host : hosts.asCollection()) {
+ if (host.aliases().contains(alias)) {
+ return new HostSpec(host.hostname(), host.aliases());
}
}
throw new IllegalArgumentException("Unable to find host for alias '" + alias + "'");
@@ -54,11 +51,11 @@ public class HostsXmlProvisioner implements HostProvisioner {
}
private HostSpec host2HostSpec(Host host) {
- return new HostSpec(host.getHostname(), host.getHostAliases());
+ return new HostSpec(host.hostname(), host.aliases());
}
private Host getFirstHost() {
- return hosts.getHosts().iterator().next();
+ return hosts.asCollection().iterator().next();
}
}
diff --git a/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java b/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java
index 69054cb7ae6..5c9d03b434f 100644
--- a/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java
+++ b/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java
@@ -49,12 +49,12 @@ public class InMemoryProvisioner implements HostProvisioner {
/** Creates this with a set of hosts of the flavor 'default' */
public InMemoryProvisioner(Hosts hosts, boolean failOnOutOfCapacity, String ... retiredHostNames) {
- this(Collections.singletonMap("default", hosts.getHosts()), failOnOutOfCapacity, 0, retiredHostNames);
+ this(Collections.singletonMap("default", hosts.asCollection()), failOnOutOfCapacity, 0, retiredHostNames);
}
/** Creates this with a set of hosts of the flavor 'default' */
public InMemoryProvisioner(Hosts hosts, boolean failOnOutOfCapacity, int startIndexForClusters, String ... retiredHostNames) {
- this(Collections.singletonMap("default", hosts.getHosts()), failOnOutOfCapacity, startIndexForClusters, retiredHostNames);
+ this(Collections.singletonMap("default", hosts.asCollection()), failOnOutOfCapacity, startIndexForClusters, retiredHostNames);
}
public InMemoryProvisioner(Map<String, Collection<Host>> hosts, boolean failOnOutOfCapacity, int startIndexForClusters, String ... retiredHostNames) {
@@ -88,7 +88,7 @@ public class InMemoryProvisioner implements HostProvisioner {
List<Host> defaultHosts = freeNodes.get("default");
if (defaultHosts.isEmpty()) throw new IllegalArgumentException("No more hosts of default flavor available");
Host newHost = freeNodes.removeValue("default", 0);
- HostSpec hostSpec = new HostSpec(newHost.getHostname(), newHost.getHostAliases());
+ HostSpec hostSpec = new HostSpec(newHost.hostname(), newHost.aliases());
legacyMapping.put(alias, hostSpec);
return hostSpec;
}
@@ -141,7 +141,7 @@ public class InMemoryProvisioner implements HostProvisioner {
if (freeNodes.get(flavor).isEmpty()) throw new IllegalArgumentException("No nodes of flavor '" + flavor + "' available");
Host newHost = freeNodes.removeValue(flavor, 0);
ClusterMembership membership = ClusterMembership.from(clusterGroup, nextIndex++);
- allocation.add(new HostSpec(newHost.getHostname(), newHost.getHostAliases(), membership));
+ allocation.add(new HostSpec(newHost.hostname(), newHost.aliases(), membership));
}
nextIndexInCluster.put(new Pair<>(clusterGroup.type(), clusterGroup.id()), nextIndex);
diff --git a/config-model/src/main/java/com/yahoo/config/model/provision/SingleNodeProvisioner.java b/config-model/src/main/java/com/yahoo/config/model/provision/SingleNodeProvisioner.java
index 67e80ec95d6..1d5544873d9 100644
--- a/config-model/src/main/java/com/yahoo/config/model/provision/SingleNodeProvisioner.java
+++ b/config-model/src/main/java/com/yahoo/config/model/provision/SingleNodeProvisioner.java
@@ -1,7 +1,6 @@
// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.config.model.provision;
-import com.yahoo.config.application.api.DeployLogger;
import com.yahoo.config.model.api.HostProvisioner;
import com.yahoo.config.provision.*;
import com.yahoo.net.HostName;
@@ -31,7 +30,7 @@ public class SingleNodeProvisioner implements HostProvisioner {
} catch (UnknownHostException e) {
throw new RuntimeException(e);
}
- this.hostSpec = new HostSpec(host.getHostname(), host.getHostAliases());
+ this.hostSpec = new HostSpec(host.hostname(), host.aliases());
}
@Override
@@ -42,7 +41,7 @@ public class SingleNodeProvisioner implements HostProvisioner {
@Override
public List<HostSpec> prepare(ClusterSpec cluster, Capacity capacity, int groups, ProvisionLogger logger) { // TODO: This should fail if capacity requested is more than 1
List<HostSpec> hosts = new ArrayList<>();
- hosts.add(new HostSpec(host.getHostname(), host.getHostAliases(), ClusterMembership.from(cluster, counter++)));
+ hosts.add(new HostSpec(host.hostname(), host.aliases(), ClusterMembership.from(cluster, counter++)));
return hosts;
}
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java
index c52a7375528..ef0f4882f1c 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java
@@ -178,4 +178,5 @@ public class ConfigserverCluster extends AbstractConfigProducer
public void getConfig(HealthMonitorConfig.Builder builder) {
builder.snapshot_interval(60.0);
}
+
}
diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/HostsXmlProvisionerTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/HostsXmlProvisionerTest.java
index 6d047c66a5a..ba38c9a3223 100644
--- a/config-model/src/test/java/com/yahoo/config/model/provision/HostsXmlProvisionerTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/provision/HostsXmlProvisionerTest.java
@@ -11,11 +11,13 @@ import java.util.*;
import static junit.framework.TestCase.assertTrue;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
/**
* @author hmusum
*/
public class HostsXmlProvisionerTest {
+
private static final String oneHost = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n" +
"<hosts>\n" +
" <host name=\"test1.yahoo.com\">\n" +
@@ -71,6 +73,8 @@ public class HostsXmlProvisionerTest {
assertThat(map.size(), is(3));
assertCorrectNumberOfHosts(map, 3);
assertTrue(map.keySet().containsAll(aliases));
+
+ assertEquals("test1.yahoo.com,test2.yahoo.com,test3.yahoo.com", System.getProperty("zookeeper.vespa.clients"));
}
@Test(expected = IllegalArgumentException.class)
diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
index 34f75d46a1c..f2b235a29da 100644
--- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
@@ -81,7 +81,7 @@ public class ModelProvisioningTest {
+ " </host>"
+ "</hosts>";
VespaModelCreatorWithMockPkg creator = new VespaModelCreatorWithMockPkg(null, services);
- VespaModel model = creator.create(new DeployState.Builder().modelHostProvisioner(new InMemoryProvisioner(Hosts.getHosts(new StringReader(hosts)), true)));
+ VespaModel model = creator.create(new DeployState.Builder().modelHostProvisioner(new InMemoryProvisioner(Hosts.readFrom(new StringReader(hosts)), true)));
assertThat(model.getContainerClusters().get("mydisc").getContainers().size(), is(3));
assertThat(model.getContainerClusters().get("mydisc").getContainers().get(0).getConfigId(), is("mydisc/container.0"));
assertTrue(model.getContainerClusters().get("mydisc").getContainers().get(0).isInitialized());
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java
index b4e366c1609..05f5145cfa7 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java
@@ -47,7 +47,7 @@ public class DedicatedAdminV4Test {
+ " </host>"
+ "</hosts>";
ApplicationPackage app = new MockApplicationPackage.Builder().withHosts(hosts).withServices(services).build();
- VespaModel model = new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder().applicationPackage(app).modelHostProvisioner(new InMemoryProvisioner(Hosts.getHosts(app.getHosts()), true)).build());
+ VespaModel model = new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder().applicationPackage(app).modelHostProvisioner(new InMemoryProvisioner(Hosts.readFrom(app.getHosts()), true)).build());
assertEquals(3, model.getHosts().size());
Set<String> serviceNames0 = serviceNames(model.getConfig(SentinelConfig.class, "hosts/myhost0"));
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
index 6eb38dd6f66..0c57d036998 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
@@ -13,9 +13,11 @@ import com.yahoo.vespa.model.VespaModel;
import com.yahoo.vespa.model.test.utils.ApplicationPackageUtils;
import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
/**
@@ -48,11 +50,11 @@ public class VespaModelTester {
public Hosts addHosts(int count) { return addHosts("default", count); }
/** Adds some hosts to this system */
public Hosts addHosts(String flavor, int count) {
- Hosts hosts = new Hosts();
+ List<Host> hosts = new ArrayList<>();
for (int i = 0; i < count; i++)
- hosts.addHost(new com.yahoo.config.model.provision.Host(flavor + i), Collections.emptyList());
- this.hosts.put(flavor.isEmpty() ? "default" : flavor, hosts.getHosts());
- return hosts;
+ hosts.add(new com.yahoo.config.model.provision.Host(flavor + i));
+ this.hosts.put(flavor.isEmpty() ? "default" : flavor, hosts);
+ return new Hosts(hosts);
}
/** Creates a model which uses 0 as start index and fails on out of capacity */
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 7751abb1d4b..327246e3099 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,6 +14,7 @@ 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;
@@ -55,6 +56,7 @@ 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.
@@ -71,6 +73,7 @@ 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())
@@ -126,7 +129,9 @@ public class NodeRepository extends AbstractComponent {
throw new IllegalArgumentException("Cannot add " + node.hostname() + ": A node with this name already exists");
}
try (Mutex lock = lockUnallocated()) {
- return zkClient.addNodes(nodes);
+ List<Node> addedNodes = zkClient.addNodes(nodes);
+ updateAllowedHosts();
+ return addedNodes;
}
}
@@ -242,14 +247,16 @@ public class NodeRepository extends AbstractComponent {
/**
* Removes a node. A node must be in the failed or parked state before it can be removed.
*
- * @return true if the node was removed, false if it was not found
+ * @return true if the node was removed, false if it was not found in one of these states
*/
public boolean remove(String hostname) {
Optional<Node> nodeToRemove = getNode(hostname, Node.State.failed, Node.State.parked);
- if ( ! nodeToRemove.isPresent())
- return false;
+ if ( ! nodeToRemove.isPresent()) return false;
+
try (Mutex lock = lock(nodeToRemove.get())) {
- return zkClient.removeNode(nodeToRemove.get().state(), hostname);
+ boolean removed = zkClient.removeNode(nodeToRemove.get().state(), hostname);
+ updateAllowedHosts();
+ return removed;
}
}
@@ -340,5 +347,21 @@ public class NodeRepository extends AbstractComponent {
private Mutex lock(Node node) {
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/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
new file mode 100644
index 00000000000..dc4744c9eaf
--- /dev/null
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
@@ -0,0 +1,66 @@
+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 com.yahoo.vespa.zookeeper.ZooKeeperServer;
+import org.junit.Test;
+import java.time.Clock;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * tests basic operation of the node repository
+ *
+ * @author bratseth
+ */
+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);
+
+ assertEquals(0, nodeRepository.getNodes(Node.Type.tenant).size());
+
+ 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)));
+
+ nodeRepository.move("host2", Node.State.parked);
+ assertTrue(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)));
+ }
+
+ 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/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java
index 4c08924f8de..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;
@@ -54,7 +55,7 @@ public class Curator {
private final String connectionSpec;
private final int serverCount;
- /** Creates a curator instance from a comma-separated string of ZooKeeper host names */
+ /** Creates a curator instance from a comma-separated string of ZooKeeper host:port strings */
public static Curator create(String connectionSpec) {
return new Curator(connectionSpec);
}
@@ -64,7 +65,7 @@ public class Curator {
public Curator(ConfigserverConfig configserverConfig, ZooKeeperServer server) {
this(createConnectionSpec(configserverConfig));
}
-
+
private static String createConnectionSpec(ConfigserverConfig config) {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < config.zookeeperserver().size(); i++) {
@@ -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);
@@ -103,15 +105,17 @@ public class Curator {
}
private static void validateConnectionSpec(String connectionSpec) {
- if (connectionSpec == null || connectionSpec.isEmpty()) {
+ if (connectionSpec == null || connectionSpec.isEmpty())
throw new IllegalArgumentException(String.format("Connections spec '%s' is not valid", connectionSpec));
- }
}
/** Returns the number of zooKeeper servers in this cluster */
public int serverCount() { return serverCount; }
- /** Returns a comma-separated list of the zookeeper servers in this cluster */
+ /**
+ * 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 -----
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
new file mode 100644
index 00000000000..d8561c67767
--- /dev/null
+++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
@@ -0,0 +1,56 @@
+package com.yahoo.vespa.zookeeper;
+
+import org.apache.zookeeper.server.NIOServerCnxn;
+import org.apache.zookeeper.server.NIOServerCnxnFactory;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.nio.channels.SelectionKey;
+import java.nio.channels.SocketChannel;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.logging.Logger;
+
+/**
+ * This class is created by zookeeper by reflection, see the ZooKeeperServer constructor.
+ *
+ * @author bratseth
+ */
+@SuppressWarnings("unused")
+public class RestrictedServerCnxnFactory extends NIOServerCnxnFactory {
+
+ private static final Logger log = Logger.getLogger(RestrictedServerCnxnFactory.class.getName());
+
+ public RestrictedServerCnxnFactory() throws IOException {
+ super();
+ }
+
+ @Override
+ protected NIOServerCnxn createConnection(SocketChannel socket, SelectionKey selection) throws IOException {
+ String remoteHost = ((InetSocketAddress)socket.getRemoteAddress()).getHostName(); // TODO: Move this line down
+
+ 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
+ return super.createConnection(socket, selection); // client checking is not activated
+ }
+
+ Set<String> zooKeeperClients = toHostnameSet(zookeeperClients);
+ 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("Allowing connection to ZooKeeper from " + remoteHost + ", as it is in " + zookeeperClients); // TODO: Remove this line
+ return super.createConnection(socket, selection);
+ }
+
+ private Set<String> toHostnameSet(String commaSeparatedString) {
+ Set<String> hostnames = new HashSet<>();
+ for (String hostname : commaSeparatedString.split(","))
+ hostnames.add(hostname.trim());
+ return hostnames;
+ }
+
+}
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
index ffae797561c..f037e3c9265 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
@@ -19,6 +19,8 @@ import java.util.List;
*/
public class ZooKeeperServer extends AbstractComponent implements Runnable {
+ public static final String ZOOKEEPER_VESPA_CLIENTS_PROPERTY = "zookeeper.vespa.clients";
+
private static final java.util.logging.Logger log = java.util.logging.Logger.getLogger(ZooKeeperServer.class.getName());
private static final String ZOOKEEPER_JMX_LOG4J_DISABLE = "zookeeper.jmx.log4j.disable";
static final String ZOOKEEPER_JUTE_MAX_BUFFER = "jute.maxbuffer";
@@ -29,6 +31,8 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable {
this.config = config;
System.setProperty("zookeeper.jmx.log4j.disable", "true");
System.setProperty(ZOOKEEPER_JUTE_MAX_BUFFER, "" + config.juteMaxBuffer());
+ System.setProperty("zookeeper.serverCnxnFactory", "com.yahoo.vespa.zookeeper.RestrictedServerCnxnFactory");
+
writeConfigToDisk(config);
zkServerThread = new Thread(this, "zookeeper server");
if (startServer) {
@@ -40,7 +44,7 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable {
public ZooKeeperServer(ZookeeperServerConfig config) {
this(config, true);
}
-
+
private void writeConfigToDisk(ZookeeperServerConfig config) {
String cfg = transformConfigToString(config);
try (FileWriter writer = new FileWriter(Defaults.getDefaults().underVespaHome(config.zooKeeperConfigFile()))) {
diff --git a/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java b/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java
index 4e231631f28..8c4db5dc8e4 100644
--- a/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java
+++ b/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java
@@ -3,7 +3,6 @@ package com.yahoo.vespa.zookeeper;
import com.yahoo.cloud.config.ZookeeperServerConfig;
import com.yahoo.io.IOUtils;
-import com.yahoo.vespa.curator.Curator;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
@@ -12,7 +11,6 @@ import java.io.File;
import java.io.IOException;
import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import com.yahoo.vespa.defaults.Defaults;
@@ -74,6 +72,7 @@ public class ZooKeeperServerTest {
File idFile = folder.newFile();
File cfgFile = folder.newFile();
+ builder.server(new ZookeeperServerConfig.Server.Builder().id(0).hostname("testhost"));
builder.zooKeeperConfigFile(cfgFile.getAbsolutePath());
builder.myidFile(idFile.getAbsolutePath());