Generate correct zookeeper server config and fix setup of components

Copmonents need to created so that they will subscribe with a config id
that is unique per container (since config is different for each container)

Also make sure to to only generate needed ´myid´ in config for each container

5 files changed, 38 insertions, 12 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java
index a52b4d915c9..23c27bb1778 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java
@@ -81,8 +81,6 @@ public final class ApplicationContainer extends Container implements
         AbstractConfigProducer<?> parent = getParent();
         if (parent == null) return;
 
-        if (parent instanceof ApplicationContainerCluster)
-            ((ApplicationContainerCluster) this.parent).getConfig(builder);
         builder.myid(index());
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
index 5593fba4ef2..813247d1ae9 100755
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
@@ -557,7 +557,7 @@ public abstract class ContainerCluster<CONTAINER extends Container>
 
     /**
      * Returns a config server config containing the right zone settings (and defaults for the rest).
-     * This is useful to allow applications to find out in which zone they are runnung by having the Zone
+     * This is useful to allow applications to find out in which zone they are running by having the Zone
      * object (which is constructed from this config) injected.
      */
     @Override
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
index 9a2814fbf03..63426979649 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
@@ -30,6 +30,7 @@ import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.Zone;
+import com.yahoo.osgi.provider.model.ComponentModel;
 import com.yahoo.search.rendering.RendererRegistry;
 import com.yahoo.searchdefinition.derived.RankProfileList;
 import com.yahoo.text.XML;
@@ -60,6 +61,7 @@ import com.yahoo.vespa.model.container.SecretStore;
 import com.yahoo.vespa.model.container.component.BindingPattern;
 import com.yahoo.vespa.model.container.component.FileStatusHandlerComponent;
 import com.yahoo.vespa.model.container.component.Handler;
+import com.yahoo.vespa.model.container.component.SimpleComponent;
 import com.yahoo.vespa.model.container.component.SystemBindingPattern;
 import com.yahoo.vespa.model.container.component.UserBindingPattern;
 import com.yahoo.vespa.model.container.component.chain.ProcessingHandler;
@@ -218,8 +220,18 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
                                                MIN_ZOOKEEPER_NODE_COUNT + " and " + MAX_ZOOKEEPER_NODE_COUNT);
         }
         cluster.addSimpleComponent("com.yahoo.vespa.curator.Curator", null, "zkfacade");
-        cluster.addSimpleComponent("com.yahoo.vespa.zookeeper.ReconfigurableVespaZooKeeperServer", null, "zookeeper-server");
-        cluster.addSimpleComponent("com.yahoo.vespa.zookeeper.Reconfigurer", null, "zookeeper-server");
+
+        // These need to be setup so that they will use the container's config id, since each container
+        // have different config (id of zookeeper server)
+        cluster.getContainers().forEach(container -> {
+            container.addComponent(zookeeperComponent("com.yahoo.vespa.zookeeper.ReconfigurableVespaZooKeeperServer", container));
+            container.addComponent(zookeeperComponent("com.yahoo.vespa.zookeeper.Reconfigurer", container));
+        });
+    }
+
+    private SimpleComponent zookeeperComponent(String idSpec, Container container) {
+        String configId = container.getConfigId();
+        return new SimpleComponent(new ComponentModel(idSpec, null, "zookeeper-server", configId));
     }
 
     private void addSecretStore(ApplicationContainerCluster cluster, Element spec) {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java
index 2503f3795b0..d151d34b6a7 100755
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java
@@ -347,12 +347,17 @@ public class ContainerClusterTest {
         addContainer(root.deployLogger(), cluster, "c2", "host-c2");
         addContainer(root.deployLogger(), cluster, "c3", "host-c3");
 
+        // Only myid is set for container
         ZookeeperServerConfig.Builder configBuilder = new ZookeeperServerConfig.Builder();
         cluster.getContainers().get(0).getConfig(configBuilder);
-        ZookeeperServerConfig config = configBuilder.build();
+        assertEquals(0, configBuilder.build().myid());
+
+        // the rest (e.g. servers) is set for cluster
+        cluster.getConfig(configBuilder);
+        assertEquals(0, configBuilder.build().myid());
         assertEquals(List.of("host-c1", "host-c2", "host-c3"),
-                     config.server().stream().map(ZookeeperServerConfig.Server::hostname).collect(Collectors.toList()));
-        assertEquals(0, config.myid());
+                     configBuilder.build().server().stream().map(ZookeeperServerConfig.Server::hostname).collect(Collectors.toList()));
+
     }
 
     private void verifyTesterApplicationInstalledBundles(Zone zone, List<String> expectedBundleNames) {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java
index 06378c2ea6e..9f3e0390168 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.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.model.container.xml;
 
+import com.yahoo.cloud.config.ZookeeperServerConfig;
 import com.yahoo.component.ComponentId;
 import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.model.NullConfigModelRegistry;
@@ -887,8 +888,14 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase {
             ApplicationContainerCluster cluster = model.getContainerClusters().get("default");
             assertNotNull(cluster);
             assertComponentConfigured(cluster,"com.yahoo.vespa.curator.Curator");
-            assertComponentConfigured(cluster,"com.yahoo.vespa.zookeeper.ReconfigurableVespaZooKeeperServer");
-            assertComponentConfigured(cluster,"com.yahoo.vespa.zookeeper.Reconfigurer");
+            cluster.getContainers().forEach(container -> {
+                assertComponentConfigured(container, "com.yahoo.vespa.zookeeper.ReconfigurableVespaZooKeeperServer");
+                assertComponentConfigured(container, "com.yahoo.vespa.zookeeper.Reconfigurer");
+
+                ZookeeperServerConfig container0Config = model.getConfig(ZookeeperServerConfig.class, container.getConfigId());
+                assertEquals(container.index(), container0Config.myid());
+                assertEquals(3, container0Config.server().size());
+            });
         }
         {
             try {
@@ -916,8 +923,12 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase {
     }
 
     private void assertComponentConfigured(ApplicationContainerCluster cluster, String componentId) {
-        Component<?, ?> curatorComponent = cluster.getComponentsMap().get(ComponentId.fromString(componentId));
-        assertNotNull(curatorComponent);
+        Component<?, ?> component = cluster.getComponentsMap().get(ComponentId.fromString(componentId));
+        assertNotNull(component);
+    }
+
+    private void assertComponentConfigured(ApplicationContainer container, String id) {
+        assertTrue(container.getComponents().getComponents().stream().anyMatch(component -> id.equals(component.getComponentId().getName())));
     }
 
     private Element generateContainerElementWithRenderer(String rendererId) {