diff options
8 files changed, 29 insertions, 146 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ConfigCurator.java b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ConfigCurator.java index c7a79556397..faf25a16321 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ConfigCurator.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ConfigCurator.java @@ -88,7 +88,7 @@ public class ConfigCurator { @Inject public ConfigCurator(Curator curator, ZooKeeperServer server) { - this(curator, server.getConfig().juteMaxBuffer()); + this(curator, server.getZookeeperServerConfig().juteMaxBuffer()); } private ConfigCurator(Curator curator, int maxNodeSize) { 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 7b0606b809b..7b191538ad8 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 @@ -38,7 +38,6 @@ public class NodeRepositoryMaintenance extends AbstractComponent { private final NodeFailer nodeFailer; private final PeriodicApplicationMaintainer periodicApplicationMaintainer; private final OperatorChangeApplicationMaintainer operatorChangeApplicationMaintainer; - private final ZooKeeperAccessMaintainer zooKeeperAccessMaintainer; private final ReservationExpirer reservationExpirer; private final InactiveExpirer inactiveExpirer; private final RetiredExpirer retiredExpirer; @@ -70,7 +69,6 @@ public class NodeRepositoryMaintenance extends AbstractComponent { nodeFailer = new NodeFailer(deployer, hostLivenessTracker, serviceMonitor, nodeRepository, durationFromEnv("fail_grace").orElse(defaults.failGrace), clock, orchestrator, throttlePolicyFromEnv("throttle_policy").orElse(defaults.throttlePolicy), metric, jobControl, configserverConfig); periodicApplicationMaintainer = new PeriodicApplicationMaintainer(deployer, nodeRepository, durationFromEnv("periodic_redeploy_interval").orElse(defaults.periodicRedeployInterval), jobControl); operatorChangeApplicationMaintainer = new OperatorChangeApplicationMaintainer(deployer, nodeRepository, clock, durationFromEnv("operator_change_redeploy_interval").orElse(defaults.operatorChangeRedeployInterval), jobControl); - zooKeeperAccessMaintainer = new ZooKeeperAccessMaintainer(nodeRepository, curator, durationFromEnv("zookeeper_access_maintenance_interval").orElse(defaults.zooKeeperAccessMaintenanceInterval), jobControl); reservationExpirer = new ReservationExpirer(nodeRepository, clock, durationFromEnv("reservation_expiry").orElse(defaults.reservationExpiry), jobControl); retiredExpirer = new RetiredExpirer(nodeRepository, orchestrator, deployer, clock, durationFromEnv("retired_interval").orElse(defaults.retiredInterval), durationFromEnv("retired_expiry").orElse(defaults.retiredExpiry), jobControl); inactiveExpirer = new InactiveExpirer(nodeRepository, clock, durationFromEnv("inactive_expiry").orElse(defaults.inactiveExpiry), jobControl); @@ -91,7 +89,6 @@ public class NodeRepositoryMaintenance extends AbstractComponent { nodeFailer.deconstruct(); periodicApplicationMaintainer.deconstruct(); operatorChangeApplicationMaintainer.deconstruct(); - zooKeeperAccessMaintainer.deconstruct(); reservationExpirer.deconstruct(); inactiveExpirer.deconstruct(); retiredExpirer.deconstruct(); 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 deleted file mode 100644 index b392d670a77..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainer.java +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -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; -import java.util.HashSet; -import java.util.Set; - -/** - * Maintains the list of hosts that should be allowed to access ZooKeeper in this runtime. - * These are the zookeeper servers and all nodes in node repository. 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, - JobControl jobControl) { - super(nodeRepository, maintenanceInterval, jobControl); - this.curator = curator; - } - - @Override - protected void maintain() { - Set<String> hosts = new HashSet<>(); - - for (Node node : nodeRepository().getNodes()) - hosts.add(node.hostname()); - - if ( ! hosts.isEmpty()) { // no nodes -> not a hosted instance: Pass an empty list to deactivate restriction - for (String hostPort : curator.zooKeeperEnsembleConnectionSpec().split(",")) - hosts.add(hostPort.split(":")[0]); - } - - ZooKeeperServer.setAllowedClientHostnames(hosts); - } - -} 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 deleted file mode 100644 index 93cf19f5450..00000000000 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ZooKeeperAccessMaintainerTest.java +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.maintenance; - -import com.yahoo.config.provision.NodeType; -import com.yahoo.vespa.hosted.provision.NodeRepositoryTester; -import com.yahoo.vespa.hosted.provision.node.Agent; -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; - -/** - * @author bratseth - */ -public class ZooKeeperAccessMaintainerTest { - - @Test - public void test() { - NodeRepositoryTester tester = new NodeRepositoryTester(); - tester.curator().setZooKeeperEnsembleConnectionSpec("server1:1234,server2:5678"); - ZooKeeperAccessMaintainer maintainer = new ZooKeeperAccessMaintainer(tester.nodeRepository(), - tester.curator(), Duration.ofHours(1), new JobControl(tester.nodeRepository().database())); - assertTrue(ZooKeeperServer.getAllowedClientHostnames().isEmpty()); - maintainer.maintain(); - assertTrue("We don't restrict to only config servers", ZooKeeperServer.getAllowedClientHostnames().isEmpty()); - - tester.addNode("id1", "host1", "default", NodeType.tenant); - tester.addNode("id2", "host2", "default", NodeType.tenant); - tester.addNode("id3", "host3", "default", NodeType.tenant); - maintainer.maintain(); - - assertEquals(3, tester.getNodes(NodeType.tenant).size()); - assertEquals(0, tester.getNodes(NodeType.proxy).size()); - assertEquals(asSet("host1,host2,host3,server1,server2"), ZooKeeperServer.getAllowedClientHostnames()); - - tester.addNode("proxy1", "host4", "default", NodeType.proxy); - tester.addNode("proxy2", "host5", "default", NodeType.proxy); - maintainer.maintain(); - - assertEquals(3, tester.getNodes(NodeType.tenant).size()); - assertEquals(2, tester.getNodes(NodeType.proxy).size()); - assertEquals(asSet("host1,host2,host3,host4,host5,server1,server2"), ZooKeeperServer.getAllowedClientHostnames()); - - tester.nodeRepository().park("host2", Agent.system, "Parking to unit test"); - tester.nodeRepository().removeRecursively("host2"); - maintainer.maintain(); - - assertEquals(2, tester.getNodes(NodeType.tenant).size()); - assertEquals(2, tester.getNodes(NodeType.proxy).size()); - assertEquals(asSet("host1,host3,host4,host5,server1,server2"), ZooKeeperServer.getAllowedClientHostnames()); - - tester.addNode("docker-host-1", "host6", "default", NodeType.host); - tester.addNode("docker-host-2", "host7", "default", NodeType.host); - maintainer.maintain(); - assertEquals(asSet("host1,host3,host4,host5,host6,host7,server1,server2"), ZooKeeperServer.getAllowedClientHostnames()); - } - - private Set<String> asSet(String s) { - return new HashSet<>(Arrays.asList(s.split(","))); - } - -} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/maintenance.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/maintenance.json index 979d846704e..28e28f9678e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/maintenance.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/responses/maintenance.json @@ -34,9 +34,6 @@ "name":"MetricsReporter" }, { - "name":"ZooKeeperAccessMaintainer" - }, - { "name":"NodeFailer" } ], 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 a0c8b845aca..d7f42c7e6e9 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java @@ -71,9 +71,9 @@ public class RestrictedServerCnxnFactory extends NIOServerCnxnFactory { return ZooKeeperServer.getAllowedClientHostnames(); } - private Set<String> toHostnameSet(String hosatnamesString) { + private Set<String> toHostnameSet(String hostnamesString) { Set<String> hostnames = new HashSet<>(); - for (String hostname : StringUtilities.split(hosatnamesString)) { + for (String hostname : StringUtilities.split(hostnamesString)) { if ( ! hostname.trim().isEmpty()) hostnames.add(hostname.trim()); } 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 74f9d01b833..aff798729a8 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.zookeeper; import com.google.common.collect.ImmutableSet; import com.google.inject.Inject; +import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.cloud.config.ZookeeperServerConfig; import com.yahoo.component.AbstractComponent; import com.yahoo.log.LogLevel; @@ -10,16 +11,14 @@ import static com.yahoo.vespa.defaults.Defaults.getDefaults; import java.io.FileWriter; import java.io.IOException; -import java.util.Collection; import java.util.List; -import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; /** * Writes zookeeper config and starts zookeeper server. * - * @author lulf - * @since 5.3 + * @author Ulf Lilleengen */ public class ZooKeeperServer extends AbstractComponent implements Runnable { @@ -35,15 +34,16 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { private static final String ZOOKEEPER_JMX_LOG4J_DISABLE = "zookeeper.jmx.log4j.disable"; static final String ZOOKEEPER_JUTE_MAX_BUFFER = "jute.maxbuffer"; private final Thread zkServerThread; - private final ZookeeperServerConfig config; + private final ZookeeperServerConfig zookeeperServerConfig; - ZooKeeperServer(ZookeeperServerConfig config, boolean startServer) { - this.config = config; + ZooKeeperServer(ZookeeperServerConfig zookeeperServerConfig, ConfigserverConfig configserverConfig, boolean startServer) { + this.zookeeperServerConfig = zookeeperServerConfig; System.setProperty("zookeeper.jmx.log4j.disable", "true"); - System.setProperty(ZOOKEEPER_JUTE_MAX_BUFFER, "" + config.juteMaxBuffer()); + System.setProperty(ZOOKEEPER_JUTE_MAX_BUFFER, "" + zookeeperServerConfig.juteMaxBuffer()); System.setProperty("zookeeper.serverCnxnFactory", "com.yahoo.vespa.zookeeper.RestrictedServerCnxnFactory"); - writeConfigToDisk(config); + setAllowedClientHostnames(zookeeperServerConfig, configserverConfig); + writeConfigToDisk(zookeeperServerConfig); zkServerThread = new Thread(this, "zookeeper server"); if (startServer) { zkServerThread.start(); @@ -51,13 +51,15 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { } @Inject - public ZooKeeperServer(ZookeeperServerConfig config) { - this(config, true); + public ZooKeeperServer(ZookeeperServerConfig zookeeperServerConfig, ConfigserverConfig configserverConfig) { + this(zookeeperServerConfig, configserverConfig, true); } - + /** Restrict access to this ZooKeeper server to the given client hosts */ - public static void setAllowedClientHostnames(Collection<String> hostnames) { - allowedClientHostnames = ImmutableSet.copyOf(hostnames); + private static void setAllowedClientHostnames(ZookeeperServerConfig zookeeperServerConfig, ConfigserverConfig configserverConfig) { + if (configserverConfig.hostedVespa()) + allowedClientHostnames = ImmutableSet.copyOf(zookeeperServerHostnames(zookeeperServerConfig)); + // empty set if not hosted Vespa => allow all access } /** Returns the hosts which are allowed to access this ZooKeeper server, or empty to allow access from anywhere */ @@ -130,10 +132,9 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { @Override public void run() { System.setProperty(ZOOKEEPER_JMX_LOG4J_DISABLE, "true"); - String[] args = new String[]{getDefaults().underVespaHome(config.zooKeeperConfigFile())}; + String[] args = new String[]{getDefaults().underVespaHome(zookeeperServerConfig.zooKeeperConfigFile())}; log.log(LogLevel.DEBUG, "Starting ZooKeeper server with config: " + args[0]); - log.log(LogLevel.INFO, "Trying to establish ZooKeeper quorum (from " + - config.server().stream().map(ZookeeperServerConfig.Server::hostname).collect(Collectors.toList()) + ")"); + log.log(LogLevel.INFO, "Trying to establish ZooKeeper quorum (from " + zookeeperServerHostnames(zookeeperServerConfig) + ")"); org.apache.zookeeper.server.quorum.QuorumPeerMain.main(args); } @@ -143,6 +144,10 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { super.deconstruct(); } - public ZookeeperServerConfig getConfig() { return config; } + public ZookeeperServerConfig getZookeeperServerConfig() { return zookeeperServerConfig; } + + private static Set<String> zookeeperServerHostnames(ZookeeperServerConfig zookeeperServerConfig) { + return zookeeperServerConfig.server().stream().map(ZookeeperServerConfig.Server::hostname).collect(Collectors.toSet()); + } } 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 8dd33f3d744..626e5bf0627 100644 --- a/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java +++ b/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.zookeeper; +import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.cloud.config.ZookeeperServerConfig; import com.yahoo.io.IOUtils; import org.junit.Rule; @@ -53,11 +54,11 @@ public class ZooKeeperServerTest { } private void createServer(ZookeeperServerConfig.Builder builder) { - new ZooKeeperServer(new ZookeeperServerConfig(builder), false); + new ZooKeeperServer(new ZookeeperServerConfig(builder), new ConfigserverConfig(new ConfigserverConfig.Builder()), false); } @Test(expected = RuntimeException.class) - public void require_that_this_id_must_be_present_amongst_servers() throws IOException { + public void require_that_this_id_must_be_present_amongst_servers() { ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder(); builder.server(newServer(2, "bar", 234, 432)); builder.server(newServer(3, "baz", 345, 543)); |