From 7cdae87d004cb9cc796356ee203cb339556c55d3 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 31 Aug 2016 13:06:08 +0200 Subject: Revert "Revert "Revert "Revert "Bratseth/lockdown zk"""" --- .../zookeeper/RestrictedServerCnxnFactory.java | 53 ++++++++++++++++++++++ .../com/yahoo/vespa/zookeeper/ZooKeeperServer.java | 13 ++++++ .../yahoo/vespa/zookeeper/ZooKeeperServerTest.java | 3 +- 3 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java (limited to 'zkfacade') 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..ae7df9ac7cf --- /dev/null +++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java @@ -0,0 +1,53 @@ +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()); + private final Set zooKeeperServerHostnames; + + public RestrictedServerCnxnFactory() throws IOException { + super(); + zooKeeperServerHostnames = toHostnameSet(System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_SERVERS_PROPERTY)); + } + + private Set toHostnameSet(String commaSeparatedString) { + if (commaSeparatedString == null || commaSeparatedString.isEmpty()) + throw new IllegalArgumentException("We have not received the list of ZooKeeper servers in this system"); + + Set hostnames = new HashSet<>(); + for (String hostname : commaSeparatedString.split(",")) + hostnames.add(hostname.trim()); + return hostnames; + } + + @Override + protected NIOServerCnxn createConnection(SocketChannel socket, SelectionKey selection) throws IOException { + String remoteHost = ((InetSocketAddress)socket.getRemoteAddress()).getHostName(); + if ( ! remoteHost.equals("localhost") && ! zooKeeperServerHostnames.contains(remoteHost)) { + String errorMessage = "Rejecting connection to ZooKeeper from " + remoteHost + + ": This cluster only allow connection among its own hosts. " + + "Hosts in this cluster: " + zooKeeperServerHostnames; + log.warning(errorMessage); + throw new IllegalArgumentException(errorMessage); + } + return super.createConnection(socket, selection); + } + +} 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..d4670e97ed8 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java @@ -22,6 +22,7 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { 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"; + static final String ZOOKEEPER_VESPA_SERVERS_PROPERTY = "zookeeper.vespa.servers"; private final Thread zkServerThread; private final ZookeeperServerConfig config; @@ -29,6 +30,10 @@ 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_VESPA_SERVERS_PROPERTY, toHostnameString(config.server())); + System.setProperty("zookeeper.serverCnxnFactory", "com.yahoo.vespa.zookeeper.RestrictedServerCnxnFactory"); + writeConfigToDisk(config); zkServerThread = new Thread(this, "zookeeper server"); if (startServer) { @@ -40,6 +45,14 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { public ZooKeeperServer(ZookeeperServerConfig config) { this(config, true); } + + private String toHostnameString(List servers) { + StringBuilder b = new StringBuilder(); + for (ZookeeperServerConfig.Server server : servers) + b.append(server.hostname()).append(", "); + b.setLength(b.length()-1); // remove the last ", " + return b.toString(); + } private void writeConfigToDisk(ZookeeperServerConfig config) { String cfg = transformConfigToString(config); 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()); -- cgit v1.2.3 From d8b7307f8b60792d8f0028551a555764085b867f Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 31 Aug 2016 16:09:17 +0200 Subject: Allow all application hosts to talk to ZooKeeper --- .../com/yahoo/config/model/provision/Host.java | 21 ++-- .../com/yahoo/config/model/provision/Hosts.java | 108 +++++++++------------ .../model/provision/HostsXmlProvisioner.java | 21 ++-- .../model/provision/InMemoryProvisioner.java | 8 +- .../model/provision/SingleNodeProvisioner.java | 5 +- .../model/provision/HostsXmlProvisionerTest.java | 4 + .../model/provision/ModelProvisioningTest.java | 2 +- .../vespa/model/admin/DedicatedAdminV4Test.java | 2 +- .../yahoo/vespa/model/test/VespaModelTester.java | 10 +- .../vespa/hosted/provision/NodeRepository.java | 24 ++++- .../vespa/hosted/provision/NodeRepositoryTest.java | 64 ++++++++++++ .../zookeeper/RestrictedServerCnxnFactory.java | 31 +++--- .../com/yahoo/vespa/zookeeper/ZooKeeperServer.java | 12 +-- 13 files changed, 184 insertions(+), 128 deletions(-) create mode 100644 node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java (limited to 'zkfacade') 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 hostAliases; + private final ImmutableList aliases; public Host(String hostname) { this.hostname = hostname; - this.hostAliases = new ArrayList<>(); + this.aliases = ImmutableList.of(); } public Host(String hostname, List hostAliases) { this.hostname = hostname; - this.hostAliases = hostAliases; + this.aliases = ImmutableList.copyOf(hostAliases); } - public String getHostname() { - return hostname; - } + public String hostname() { return hostname; } - public List getHostAliases() { - return hostAliases; - } + /** Returns an immutable list of the aliases of this node, which may be empty but never null */ + public List 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 hosts = new LinkedHashMap<>(); - private final Map alias2hostname = new LinkedHashMap<>(); - private final Map alias2host = new LinkedHashMap<>(); + private final ImmutableMap hosts; + public Hosts(Collection hosts) { + validateAliases(hosts); + + ImmutableMap.Builder 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 hosts) { + Set 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 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 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 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 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 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 hostAliases, Host host) { - if (hostAliases.size() < 1) { - throw new RuntimeException("Host '" + host.getHostname() + "' must have at least one 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 getAlias2host() { - return alias2host; - } + /** Returns an immutable collection of the hosts of this */ + public Collection 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 not specified"); + if (hosts.asCollection().size() > 1) { + throw new IllegalArgumentException("More than 1 host specified (" + hosts.asCollection().size() + ") and 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 b4488cef385..c0813f7ab85 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> hosts, boolean failOnOutOfCapacity, int startIndexForClusters, String ... retiredHostNames) { @@ -88,7 +88,7 @@ public class InMemoryProvisioner implements HostProvisioner { List 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 prepare(ClusterSpec cluster, Capacity capacity, int groups, ProvisionLogger logger) { // TODO: This should fail if capacity requested is more than 1 List 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/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 = "\n" + "\n" + " \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 38702d6920f..065d66e9f93 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 { + " " + ""; 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 { + " " + ""; 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 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 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 0533538b016..287f311163e 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; @@ -126,7 +127,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 addedNodes = zkClient.addNodes(nodes); + updateAllowedHosts(); + return addedNodes; } } @@ -244,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 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; } } @@ -342,5 +347,14 @@ 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(); + for (Node node : getNodes(Node.Type.tenant)) + s.append(node.hostname()).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..a8c839a6f44 --- /dev/null +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java @@ -0,0 +1,64 @@ +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(); + NodeRepository nodeRepository = new NodeRepository(nodeFlavors, new MockCurator(), clock); + + assertEquals(0, nodeRepository.getNodes(Node.Type.tenant).size()); + + List 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"), 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))); + } + + private Set 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/zookeeper/RestrictedServerCnxnFactory.java b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java index ae7df9ac7cf..90c68461699 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java @@ -18,36 +18,35 @@ import java.util.logging.Logger; */ @SuppressWarnings("unused") public class RestrictedServerCnxnFactory extends NIOServerCnxnFactory { - + private static final Logger log = Logger.getLogger(RestrictedServerCnxnFactory.class.getName()); - private final Set zooKeeperServerHostnames; public RestrictedServerCnxnFactory() throws IOException { super(); - zooKeeperServerHostnames = toHostnameSet(System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_SERVERS_PROPERTY)); } - private Set toHostnameSet(String commaSeparatedString) { - if (commaSeparatedString == null || commaSeparatedString.isEmpty()) - throw new IllegalArgumentException("We have not received the list of ZooKeeper servers in this system"); - - Set hostnames = new HashSet<>(); - for (String hostname : commaSeparatedString.split(",")) - hostnames.add(hostname.trim()); - return hostnames; - } - @Override protected NIOServerCnxn createConnection(SocketChannel socket, SelectionKey selection) throws IOException { + String zookeeperClients = System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY); + if (zookeeperClients == null || zookeeperClients.isEmpty()) + return super.createConnection(socket, selection); // client checking is not activated + + Set zooKeeperClients = toHostnameSet(zookeeperClients); String remoteHost = ((InetSocketAddress)socket.getRemoteAddress()).getHostName(); - if ( ! remoteHost.equals("localhost") && ! zooKeeperServerHostnames.contains(remoteHost)) { + if ( ! remoteHost.equals("localhost") && ! zooKeeperClients.contains(remoteHost)) { String errorMessage = "Rejecting connection to ZooKeeper from " + remoteHost + - ": This cluster only allow connection among its own hosts. " + - "Hosts in this cluster: " + zooKeeperServerHostnames; + ": This cluster only allow connection from hosts in: " + zooKeeperClients; log.warning(errorMessage); throw new IllegalArgumentException(errorMessage); } return super.createConnection(socket, selection); } + private Set toHostnameSet(String commaSeparatedString) { + Set 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 d4670e97ed8..65c44981fb2 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java @@ -19,10 +19,11 @@ 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"; - static final String ZOOKEEPER_VESPA_SERVERS_PROPERTY = "zookeeper.vespa.servers"; private final Thread zkServerThread; private final ZookeeperServerConfig config; @@ -31,7 +32,6 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { System.setProperty("zookeeper.jmx.log4j.disable", "true"); System.setProperty(ZOOKEEPER_JUTE_MAX_BUFFER, "" + config.juteMaxBuffer()); - System.setProperty(ZOOKEEPER_VESPA_SERVERS_PROPERTY, toHostnameString(config.server())); System.setProperty("zookeeper.serverCnxnFactory", "com.yahoo.vespa.zookeeper.RestrictedServerCnxnFactory"); writeConfigToDisk(config); @@ -46,14 +46,6 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { this(config, true); } - private String toHostnameString(List servers) { - StringBuilder b = new StringBuilder(); - for (ZookeeperServerConfig.Server server : servers) - b.append(server.hostname()).append(", "); - b.setLength(b.length()-1); // remove the last ", " - return b.toString(); - } - private void writeConfigToDisk(ZookeeperServerConfig config) { String cfg = transformConfigToString(config); try (FileWriter writer = new FileWriter(Defaults.getDefaults().underVespaHome(config.zooKeeperConfigFile()))) { -- cgit v1.2.3 From 38af503f93b5d7706aa7b0ace57b53c8c697db9a Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Thu, 1 Sep 2016 09:30:57 +0200 Subject: Whitespace changes --- .../yahoo/vespa/model/container/configserver/ConfigserverCluster.java | 1 + zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) (limited to 'zkfacade') 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/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java index 65c44981fb2..f037e3c9265 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java @@ -31,7 +31,6 @@ 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); -- cgit v1.2.3 From 390b027faf4a632158764fec2f75ce22222134f1 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Thu, 1 Sep 2016 09:54:08 +0200 Subject: Allow config servers to connect to zk --- .../java/com/yahoo/vespa/hosted/provision/NodeRepository.java | 9 +++++++++ zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java | 9 ++++----- .../com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java | 8 ++++++-- 3 files changed, 19 insertions(+), 7 deletions(-) (limited to 'zkfacade') 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 287f311163e..b0e178bea92 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 @@ -56,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. @@ -72,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()) @@ -350,8 +352,15 @@ public class NodeRepository extends AbstractComponent { 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/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java index 4c08924f8de..66734036ce5 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java @@ -54,7 +54,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 +64,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++) { @@ -103,15 +103,14 @@ 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 */ public String connectionSpec() { return connectionSpec; } /** For internal use; prefer creating a {@link CuratorCounter} */ 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 90c68461699..d8561c67767 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java @@ -27,18 +27,22 @@ 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 zookeeperClients = System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY); - if (zookeeperClients == null || zookeeperClients.isEmpty()) + 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 zooKeeperClients = toHostnameSet(zookeeperClients); - String remoteHost = ((InetSocketAddress)socket.getRemoteAddress()).getHostName(); 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); } -- cgit v1.2.3 From 7bc8b35787d320ff0c50fd8036fc83d20316a1a1 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Thu, 1 Sep 2016 13:08:07 +0200 Subject: Include connection spec hosts correctly in permissible zk clients --- .../java/com/yahoo/vespa/hosted/provision/NodeRepository.java | 2 +- .../java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java | 8 +++++--- zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java | 7 ++++++- .../src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java | 7 +++++++ 4 files changed, 19 insertions(+), 5 deletions(-) (limited to 'zkfacade') 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 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 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 ----- -- cgit v1.2.3