Add VESPA_CONFIGSERVER_ZOOKEEPER_IDS

5 files changed, 156 insertions, 33 deletions

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 891f68041f1..6c7da668e93 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
@@ -18,6 +18,7 @@ import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions;
 import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions.ConfigServer;
 
 import java.util.Optional;
+import java.util.stream.IntStream;
 
 /**
  * Represents a config server cluster.
@@ -53,17 +54,27 @@ public class ConfigserverCluster extends AbstractConfigProducer
 
     @Override
     public void getConfig(ZookeeperServerConfig.Builder builder) {
+        ConfigServer[] configServers = getConfigServers();
+        int[] zookeeperIds = getConfigServerZookeeperIds();
+
+        if (configServers.length != zookeeperIds.length) {
+            throw new IllegalArgumentException(String.format("Number of provided config server hosts (%d) must be the " +
+                    "same as number of provided config server zookeeper ids (%d)",
+                    configServers.length, zookeeperIds.length));
+        }
+
         String myhostname = HostName.getLocalhost();
-        int myid = 0;
-        int i = 0;
-        for (ConfigServer server : getConfigServers()) {
-            if (server.hostName.equals(myhostname)) {
-                myid = i;
+        for (int i = 0; i < configServers.length; i++) {
+            if (zookeeperIds[i] < 0) {
+                throw new IllegalArgumentException(String.format("Zookeeper ids cannot be negative, was %d for %s",
+                        zookeeperIds[i], configServers[i].hostName));
+            }
+            if (configServers[i].hostName.equals(myhostname)) {
+                builder.myid(zookeeperIds[i]);
             }
-            builder.server(getZkServer(server, i));
-            i++;
+            builder.server(getZkServer(configServers[i], zookeeperIds[i]));
         }
-        builder.myid(myid);
+
         if (options.zookeeperClientPort().isPresent()) {
             builder.clientPort(options.zookeeperClientPort().get());
         }
@@ -150,11 +161,15 @@ public class ConfigserverCluster extends AbstractConfigProducer
     }
 
     private ConfigServer[] getConfigServers() {
-        if (options.allConfigServers().length > 0) {
-            return options.allConfigServers();
-        } else {
-            return new ConfigServer[]{new ConfigServer(HostName.getLocalhost(), Optional.<Integer>empty()) };
-        }
+        return Optional.of(options.allConfigServers())
+                .filter(configServers -> configServers.length > 0)
+                .orElseGet(() -> new ConfigServer[]{new ConfigServer(HostName.getLocalhost(), Optional.empty())});
+    }
+
+    private int[] getConfigServerZookeeperIds() {
+        return Optional.of(options.configServerZookeeperIds())
+                .filter(ids -> ids.length > 0)
+                .orElseGet(() -> IntStream.range(0, getConfigServers().length).toArray());
     }
 
     private ZookeeperServerConfig.Server.Builder getZkServer(ConfigServer server, int id) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/option/CloudConfigOptions.java b/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/option/CloudConfigOptions.java
index c8a39faa1d9..eafff2374cb 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/option/CloudConfigOptions.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/option/CloudConfigOptions.java
@@ -24,6 +24,7 @@ public interface CloudConfigOptions {
     Optional<Boolean> hostedVespa();
 
     ConfigServer[] allConfigServers();
+    int[] configServerZookeeperIds();
     Optional<Integer> zookeeperClientPort();
     String[] configModelPluginDirs();
     Optional<Long> sessionLifeTimeSecs();
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java
index b4ad2ddbd21..708d38d8ebb 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java
@@ -2,6 +2,8 @@
 package com.yahoo.vespa.model.container.configserver;
 
 import com.yahoo.cloud.config.ConfigserverConfig;
+import com.yahoo.cloud.config.ZookeeperServerConfig;
+import com.yahoo.config.ConfigInstance;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.producer.AbstractConfigProducerRoot;
 import com.yahoo.config.model.test.MockRoot;
@@ -11,11 +13,19 @@ import com.yahoo.jdisc.metrics.yamasconsumer.cloud.ScoreBoardConfig;
 import com.yahoo.net.HostName;
 import com.yahoo.text.XML;
 import com.yahoo.vespa.defaults.Defaults;
+import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions;
 import com.yahoo.vespa.model.container.xml.ConfigServerContainerModelBuilder;
-import org.junit.Before;
 import org.junit.Test;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
 import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
@@ -25,33 +35,54 @@ import static org.junit.Assert.assertTrue;
  */
 public class ConfigserverClusterTest {
 
-    private AbstractConfigProducerRoot root;
+    @Test
+    public void zookeeperConfig_default() {
+        ZookeeperServerConfig config = getConfig(ZookeeperServerConfig.class);
+        assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::hostname, "localhost");
+        assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::id, 0);
+        assertEquals(0, config.myid());
+    }
 
-    @Before
-    public void setupCluster() {
-        String services = "<jdisc id='standalone' version='1.0'>"
-                        + "  <http>"
-                        + "    <server port='1337' id='configserver' />"
-                        + "  </http>"
-                        + "</jdisc>";
-        root = new MockRoot();
-        new ConfigServerContainerModelBuilder(new TestOptions().rpcPort(12345).useVespaVersionInRequest(true)
-                                                               .hostedVespa(true).environment("test").region("bar")
-                                                               .numParallelTenantLoaders(99))
-                .build(new DeployState.Builder().build(), null, null, root, XML.getDocument(services).getDocumentElement());
-        root.freezeModelTopology();
+    @Test
+    public void zookeeperConfig_only_config_servers_set() {
+        TestOptions testOptions = createTestOptions(Arrays.asList("cfg1", "localhost", "cfg3"), Collections.emptyList());
+        ZookeeperServerConfig config = getConfig(ZookeeperServerConfig.class, testOptions);
+        assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::hostname, "cfg1", "localhost", "cfg3");
+        assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::id, 0, 1, 2);
+        assertEquals(1, config.myid());
+    }
+
+    @Test
+    public void zookeeperConfig_with_config_servers_and_zk_ids() {
+        TestOptions testOptions = createTestOptions(Arrays.asList("cfg1", "localhost", "cfg3"), Arrays.asList(4, 2, 3));
+        ZookeeperServerConfig config = getConfig(ZookeeperServerConfig.class, testOptions);
+        assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::hostname, "cfg1", "localhost", "cfg3");
+        assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::id, 4, 2, 3);
+        assertEquals(2, config.myid());
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void zookeeperConfig_uneven_number_of_config_servers_and_zk_ids() {
+        TestOptions testOptions = createTestOptions(Arrays.asList("cfg1", "localhost", "cfg3"), Collections.singletonList(1));
+        getConfig(ZookeeperServerConfig.class, testOptions);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void zookeeperConfig_negative_zk_id() {
+        TestOptions testOptions = createTestOptions(Arrays.asList("cfg1", "localhost", "cfg3"), Arrays.asList(1, 2, -1));
+        getConfig(ZookeeperServerConfig.class, testOptions);
     }
 
     @Test
     public void testStatisticsConfig() {
-        StatisticsConfig config = root.getConfig(StatisticsConfig.class, "configserver/standalone");
+        StatisticsConfig config = getConfig(StatisticsConfig.class);
         assertThat((int) config.collectionintervalsec(), is(60));
         assertThat((int) config.loggingintervalsec(), is(60));
     }
 
     @Test
     public void testScoreBoardConfig() {
-        ScoreBoardConfig config = root.getConfig(ScoreBoardConfig.class, "configserver/standalone");
+        ScoreBoardConfig config = getConfig(ScoreBoardConfig.class);
         assertThat(config.applicationName(), is("configserver"));
         assertThat(config.flushTime(), is(60));
         assertThat(config.step(), is(60));
@@ -59,13 +90,13 @@ public class ConfigserverClusterTest {
 
     @Test
     public void testHealthMonitorConfig() {
-        HealthMonitorConfig config = root.getConfig(HealthMonitorConfig.class, "configserver/standalone");
+        HealthMonitorConfig config = getConfig(HealthMonitorConfig.class);
         assertThat(((int) config.snapshot_interval()), is(60));
     }
 
     @Test
     public void testConfigserverConfig() {
-        ConfigserverConfig config = root.getConfig(ConfigserverConfig.class, "configserver/standalone");
+        ConfigserverConfig config = getConfig(ConfigserverConfig.class);
         assertThat(config.configModelPluginDir().size(), is(1));
         assertThat(config.configModelPluginDir().get(0), is(Defaults.getDefaults().underVespaHome("lib/jars/config-models")));
         assertThat(config.rpcport(), is(12345));
@@ -79,4 +110,53 @@ public class ConfigserverClusterTest {
         assertThat(config.region(), is("bar"));
     }
 
+    @SuppressWarnings("varargs")
+    private static <T> void assertZookeeperServerProperty(
+            List<ZookeeperServerConfig.Server> zkServers, Function<ZookeeperServerConfig.Server, T> properyMapper, T... expectedProperties) {
+        List<T> actualPropertyValues = zkServers.stream().map(properyMapper).collect(Collectors.toList());
+        List<T> expectedPropertyValues = Arrays.asList(expectedProperties);
+        assertEquals(expectedPropertyValues, actualPropertyValues);
+    }
+
+    private static TestOptions createTestOptions(List<String> configServerHostnames, List<Integer> configServerZkIds) {
+        TestOptions testOptions = new TestOptions()
+                .rpcPort(12345)
+                .useVespaVersionInRequest(true)
+                .hostedVespa(true)
+                .environment("test")
+                .region("bar")
+                .numParallelTenantLoaders(99);
+
+        Optional.of(configServerHostnames)
+                .filter(hostnames -> !hostnames.isEmpty())
+                .map(hostnames -> hostnames.stream()
+                        .map(hostname -> new CloudConfigOptions.ConfigServer(hostname, Optional.empty()))
+                        .toArray(CloudConfigOptions.ConfigServer[]::new))
+                .ifPresent(testOptions::configServers);
+
+        Optional.of(configServerZkIds)
+                .filter(zkIds -> !zkIds.isEmpty())
+                .map(zkIds -> zkIds.stream().mapToInt(i -> i).toArray())
+                .ifPresent(testOptions::configServerZookeeperIds);
+
+        return testOptions;
+    }
+
+    private static <CONFIGTYPE extends ConfigInstance> CONFIGTYPE getConfig(Class<CONFIGTYPE> clazz) {
+        return getConfig(clazz, createTestOptions(Collections.emptyList(), Collections.emptyList()));
+    }
+
+    private static <CONFIGTYPE extends ConfigInstance> CONFIGTYPE getConfig(Class<CONFIGTYPE> clazz, TestOptions testOptions) {
+        AbstractConfigProducerRoot root = new MockRoot();
+        String services = "<jdisc id='standalone' version='1.0'>"
+                + "  <http>"
+                + "    <server port='1337' id='configserver' />"
+                + "  </http>"
+                + "</jdisc>";
+        new ConfigServerContainerModelBuilder(testOptions)
+                .build(new DeployState.Builder().build(), null, null, root, XML.getDocument(services).getDocumentElement());
+        root.freezeModelTopology();
+
+        return root.getConfig(clazz, "configserver/standalone");
+    }
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/TestOptions.java b/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/TestOptions.java
index 3c2f71fa2e1..07422b1f215 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/TestOptions.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/TestOptions.java
@@ -9,6 +9,9 @@ import java.util.Optional;
  * @author Ulf Lilleengen
  */
 public class TestOptions implements CloudConfigOptions {
+
+    private ConfigServer[] configServers = new ConfigServer[0];
+    private int[] configServerZookeeperIds = new int[0];
     private Optional<Integer> rpcPort = Optional.empty();
     private Optional<String> environment = Optional.empty();
     private Optional<String> region = Optional.empty();
@@ -45,7 +48,12 @@ public class TestOptions implements CloudConfigOptions {
 
     @Override
     public ConfigServer[] allConfigServers() {
-        return new ConfigServer[0];
+        return configServers;
+    }
+
+    @Override
+    public int[] configServerZookeeperIds() {
+        return configServerZookeeperIds;
     }
 
     @Override
@@ -121,6 +129,16 @@ public class TestOptions implements CloudConfigOptions {
         return Optional.empty();
     }
 
+    public TestOptions configServers(ConfigServer[] configServers) {
+        this.configServers = configServers;
+        return this;
+    }
+
+    public TestOptions configServerZookeeperIds(int[] configServerZookeeperIds) {
+        this.configServerZookeeperIds = configServerZookeeperIds;
+        return this;
+    }
+
     public TestOptions numParallelTenantLoaders(int numLoaders) {
         this.numParallelTenantLoaders = Optional.of(numLoaders);
         return this;
diff --git a/standalone-container/src/main/java/com/yahoo/container/standalone/CloudConfigInstallVariables.java b/standalone-container/src/main/java/com/yahoo/container/standalone/CloudConfigInstallVariables.java
index 0be4a55275c..2125fb0e499 100644
--- a/standalone-container/src/main/java/com/yahoo/container/standalone/CloudConfigInstallVariables.java
+++ b/standalone-container/src/main/java/com/yahoo/container/standalone/CloudConfigInstallVariables.java
@@ -33,6 +33,15 @@ public class CloudConfigInstallVariables implements CloudConfigOptions {
     }
 
     @Override
+    public int[] configServerZookeeperIds() {
+        return Optional.ofNullable(System.getenv("VESPA_CONFIGSERVER_ZOOKEEPER_IDS"))
+                .map(CloudConfigInstallVariables::multiValueParameterStream)
+                .orElseGet(Stream::empty)
+                .mapToInt(Integer::valueOf)
+                .toArray();
+    }
+
+    @Override
     public Optional<Long> zookeeperBarrierTimeout() {
         return getInstallVariable("zookeeper_barrier_timeout", Long::parseLong);
     }