Revert "Support configuring ZooKeeper service in container"

11 files changed, 60 insertions, 111 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java
index 916c65e5f82..0a61b5c91c6 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java
@@ -14,7 +14,7 @@ import com.yahoo.vespa.model.container.component.SystemBindingPattern;
 public class LogserverContainerCluster extends ContainerCluster<LogserverContainer> {
 
     public LogserverContainerCluster(AbstractConfigProducer<?> parent, String name, DeployState deployState) {
-        super(parent, name, name, deployState, true);
+        super(parent, name, name, deployState);
 
         addDefaultHandlersWithVip();
         addLogHandler();
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java
index c11a5017f13..a44f14e858c 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.model.admin.clustercontroller;
 
 import com.google.common.base.Joiner;
+import com.yahoo.cloud.config.CuratorConfig;
 import com.yahoo.cloud.config.ZookeeperServerConfig;
 import com.yahoo.cloud.config.ZookeepersConfig;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
@@ -19,10 +20,11 @@ import java.util.Collection;
  * @author Ulf Lilleengen
  */
 public class ClusterControllerCluster extends AbstractConfigProducer<ClusterControllerContainerCluster> implements
+        CuratorConfig.Producer,
         ZookeeperServerConfig.Producer,
         ZookeepersConfig.Producer {
 
-    private static final int ZK_CLIENT_PORT = 2181; // Must match the default in CuratorConfig
+    private static final int ZK_CLIENT_PORT = 2181;
     private ClusterControllerContainerCluster containerCluster = null;
 
     public ClusterControllerCluster(AbstractConfigProducer<?> parent, String subId) {
@@ -73,5 +75,15 @@ public class ClusterControllerCluster extends AbstractConfigProducer<ClusterCont
         }
     }
 
+    @Override
+    public void getConfig(CuratorConfig.Builder builder) {
+        for (ClusterControllerContainer container : containerCluster.getContainers()) {
+            CuratorConfig.Server.Builder serverBuilder = new CuratorConfig.Server.Builder();
+            serverBuilder.hostname(container.getHostName()).port(ZK_CLIENT_PORT);
+            builder.server(serverBuilder);
+            builder.zookeeperLocalhostAffinity(false);
+        }
+    }
+
 }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java
index d88c665578c..9e15db348a2 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.model.admin.clustercontroller;
 
 import com.yahoo.cloud.config.ZookeeperServerConfig;
 import com.yahoo.component.ComponentSpecification;
+import com.yahoo.config.model.api.Reindexing;
 import com.yahoo.config.model.api.container.ContainerServiceType;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.container.bundle.BundleInstantiationSpecification;
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java
index ed4c50999fe..3ee2a840f20 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java
@@ -13,7 +13,7 @@ import com.yahoo.vespa.model.container.ContainerCluster;
 public class ClusterControllerContainerCluster extends ContainerCluster<ClusterControllerContainer>
 {
     public ClusterControllerContainerCluster(AbstractConfigProducer<?> parent, String subId, String name, DeployState deployState) {
-        super(parent, subId, name, deployState, false);
+        super(parent, subId, name, deployState);
         addDefaultHandlersWithVip();
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java
index 3e2adeaacc9..4b9e1c302b7 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java
@@ -88,7 +88,7 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC
 
 
     public MetricsProxyContainerCluster(AbstractConfigProducer<?> parent, String name, DeployState deployState) {
-        super(parent, name, name, deployState, true);
+        super(parent, name, name, deployState);
         this.parent = parent;
         applicationId = deployState.getProperties().applicationId();
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
index 4e7405a7d99..93717f0f532 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
@@ -8,6 +8,7 @@ import com.yahoo.config.FileReference;
 import com.yahoo.config.application.api.ComponentInfo;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
+import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.container.bundle.BundleInstantiationSpecification;
 import com.yahoo.container.di.config.ApplicationBundlesConfig;
 import com.yahoo.container.handler.metrics.MetricsProxyApiConfig;
@@ -82,7 +83,7 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat
     private Integer memoryPercentage = null;
 
     public ApplicationContainerCluster(AbstractConfigProducer<?> parent, String configSubId, String clusterId, DeployState deployState) {
-        super(parent, configSubId, clusterId, deployState, true);
+        super(parent, configSubId, clusterId, deployState);
         this.tlsClientAuthority = deployState.tlsClientAuthority();
         restApiGroup = new ConfigProducerGroup<>(this, "rest-api");
         servletGroup = new ConfigProducerGroup<>(this, "servlet");
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 1db735995a1..0909376cb18 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
@@ -3,7 +3,6 @@ package com.yahoo.vespa.model.container;
 
 import com.yahoo.cloud.config.ClusterInfoConfig;
 import com.yahoo.cloud.config.ConfigserverConfig;
-import com.yahoo.cloud.config.CuratorConfig;
 import com.yahoo.cloud.config.RoutingProviderConfig;
 import com.yahoo.component.ComponentId;
 import com.yahoo.config.application.api.ApplicationMetaData;
@@ -35,7 +34,6 @@ import com.yahoo.search.config.QrStartConfig;
 import com.yahoo.search.pagetemplates.PageTemplatesConfig;
 import com.yahoo.search.query.profile.config.QueryProfilesConfig;
 import com.yahoo.vespa.configdefinition.IlscriptsConfig;
-import com.yahoo.vespa.model.HostResource;
 import com.yahoo.vespa.model.PortsMeta;
 import com.yahoo.vespa.model.Service;
 import com.yahoo.vespa.model.admin.monitoring.Monitoring;
@@ -102,8 +100,7 @@ public abstract class ContainerCluster<CONTAINER extends Container>
         DocprocConfig.Producer,
         ClusterInfoConfig.Producer,
         RoutingProviderConfig.Producer,
-        ConfigserverConfig.Producer,
-        CuratorConfig.Producer
+        ConfigserverConfig.Producer
 {
 
     /**
@@ -150,7 +147,6 @@ public abstract class ContainerCluster<CONTAINER extends Container>
     private final List<String> endpointAliases = new ArrayList<>();
     private final ComponentGroup<Component<?, ?>> componentGroup;
     private final boolean isHostedVespa;
-    private final boolean zooKeeperLocalhostAffinity;
 
     private final Map<String, String> concreteDocumentTypes = new LinkedHashMap<>();
 
@@ -165,12 +161,11 @@ public abstract class ContainerCluster<CONTAINER extends Container>
 
     private boolean deferChangesUntilRestart = false;
 
-    public ContainerCluster(AbstractConfigProducer<?> parent, String configSubId, String clusterId, DeployState deployState, boolean zooKeeperLocalhostAffinity) {
+    public ContainerCluster(AbstractConfigProducer<?> parent, String configSubId, String clusterId, DeployState deployState) {
         super(parent, configSubId);
         this.name = clusterId;
         this.isHostedVespa = stateIsHosted(deployState);
         this.zone = (deployState != null) ? deployState.zone() : Zone.defaultZone();
-        this.zooKeeperLocalhostAffinity = zooKeeperLocalhostAffinity;
 
         componentGroup = new ComponentGroup<>(this, "component");
 
@@ -567,16 +562,6 @@ public abstract class ContainerCluster<CONTAINER extends Container>
         builder.region(zone.region().value());
     }
 
-    @Override
-    public void getConfig(CuratorConfig.Builder builder) {
-        for (var container : containers) {
-            HostResource hostResource = container.getHostResource();
-            if (hostResource == null) continue;
-            builder.server(new CuratorConfig.Server.Builder().hostname(hostResource.getHostname()));
-        }
-        builder.zookeeperLocalhostAffinity(zooKeeperLocalhostAffinity);
-    }
-
     private List<ClusterInfoConfig.Services.Ports.Builder> getPorts(Service service) {
         List<ClusterInfoConfig.Services.Ports.Builder> builders = new ArrayList<>();
         PortsMeta portsMeta = service.getPortsMeta();
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 b7d64483e57..7eea5d8496f 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
@@ -85,6 +85,7 @@ import org.w3c.dom.Node;
 
 import java.net.URI;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -112,10 +113,6 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
     private static final String DEPRECATED_CONTAINER_TAG = "jdisc";
     private static final String ENVIRONMENT_VARIABLES_ELEMENT = "environment-variables";
 
-    // The node count to enforce in a cluster running ZooKeeper
-    private static final int MIN_ZOOKEEPER_NODE_COUNT = 3;
-    private static final int MAX_ZOOKEEPER_NODE_COUNT = 7;
-
     public enum Networking { disable, enable }
 
     private ApplicationPackage app;
@@ -199,26 +196,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         addClientProviders(deployState, spec, cluster);
         addServerProviders(deployState, spec, cluster);
 
-        // Must be added after nodes:
-        addAthensCopperArgos(cluster, context);
-        addZooKeeper(cluster, spec);
-    }
-
-    private void addZooKeeper(ApplicationContainerCluster cluster, Element spec) {
-        Element zkElement = XML.getChild(spec, "zookeeper");
-        if (zkElement == null) return;
-        Element nodesElement = XML.getChild(spec, "nodes");
-        boolean isCombined = nodesElement != null && nodesElement.hasAttribute("of");
-        if (isCombined) {
-            throw new IllegalArgumentException("A combined cluster cannot run ZooKeeper");
-        }
-        int nodeCount = cluster.getContainers().size();
-        if (nodeCount < MIN_ZOOKEEPER_NODE_COUNT || nodeCount > MAX_ZOOKEEPER_NODE_COUNT || nodeCount % 2 == 0) {
-            throw new IllegalArgumentException("Clusters running ZooKeeper must have an odd number of nodes, between " +
-                                               MIN_ZOOKEEPER_NODE_COUNT + " and " + MAX_ZOOKEEPER_NODE_COUNT);
-        }
-        cluster.addSimpleComponent("com.yahoo.vespa.curator.Curator", null, "zkfacade");
-        // TODO: Add server component
+        addAthensCopperArgos(cluster, context);  // Must be added after nodes.
     }
 
     private void addSecretStore(ApplicationContainerCluster cluster, Element spec) {
@@ -746,6 +724,43 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         return createNodesFromHosts(context.getDeployLogger(), hosts, cluster);
     }
 
+    /**
+     * This is used in case we are on hosted Vespa and no nodes tag is supplied:
+     * If there are content clusters this will pick the first host in the first cluster as the container node.
+     * If there are no content clusters this will return empty (such that the node can be created by the container here).
+     */
+    private Optional<HostResource> getHostResourceFromContentClusters(ApplicationContainerCluster cluster, Element containersElement, ConfigModelContext context) {
+        Optional<Element> services = servicesRootOf(containersElement);
+        if ( ! services.isPresent())
+            return Optional.empty();
+        List<Element> contentServices = XML.getChildren(services.get(), "content");
+        if ( contentServices.isEmpty() ) return Optional.empty();
+        Element contentNodesElementOrNull = XML.getChild(contentServices.get(0), "nodes");
+        
+        NodesSpecification nodesSpec;
+        if (contentNodesElementOrNull == null)
+            nodesSpec = NodesSpecification.nonDedicated(1, context);
+        else
+            nodesSpec = NodesSpecification.from(new ModelElement(contentNodesElementOrNull), context);
+
+        Map<HostResource, ClusterMembership> hosts =
+                StorageGroup.provisionHosts(nodesSpec,
+                                            contentServices.get(0).getAttribute("id"),
+                                            cluster.getRoot().hostSystem(),
+                                            context.getDeployLogger());
+        return Optional.of(hosts.keySet().iterator().next());
+    }
+
+    /** Returns the services element above the given Element, or empty if there is no services element */
+    private Optional<Element> servicesRootOf(Element element) {
+        Node parent = element.getParentNode();
+        if (parent == null) return Optional.empty();
+        if ( ! (parent instanceof Element)) return Optional.empty();
+        Element parentElement = (Element)parent;
+        if (parentElement.getTagName().equals("services")) return Optional.of(parentElement);
+        return servicesRootOf(parentElement);
+    }
+    
     private List<ApplicationContainer> createNodesFromHosts(DeployLogger deployLogger, Map<HostResource, ClusterMembership> hosts, ApplicationContainerCluster cluster) {
         List<ApplicationContainer> nodes = new ArrayList<>();
         for (Map.Entry<HostResource, ClusterMembership> entry : hosts.entrySet()) {
diff --git a/config-model/src/main/resources/schema/containercluster.rnc b/config-model/src/main/resources/schema/containercluster.rnc
index ca7efd5a938..25d10e0d9b3 100644
--- a/config-model/src/main/resources/schema/containercluster.rnc
+++ b/config-model/src/main/resources/schema/containercluster.rnc
@@ -27,7 +27,6 @@ ContainerServices =
     Http? &
     AccessLog* &
     SecretStore? &
-    ZooKeeper? &
     GenericConfig*
 
 # TODO(ogronnesby): Change this configuration syntax
@@ -92,10 +91,6 @@ SecretStore = element secret-store {
     } +
 }
 
-ZooKeeper = element zookeeper {
-    empty
-}
-
 ModelEvaluation = element model-evaluation {
     empty
 }
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 415b64a5574..b1cd9c5c604 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
@@ -3,7 +3,6 @@ package com.yahoo.vespa.model.container;
 
 import com.yahoo.cloud.config.ClusterInfoConfig;
 import com.yahoo.cloud.config.ConfigserverConfig;
-import com.yahoo.cloud.config.CuratorConfig;
 import com.yahoo.cloud.config.RoutingProviderConfig;
 import com.yahoo.component.ComponentId;
 import com.yahoo.config.application.api.DeployLogger;
@@ -43,7 +42,6 @@ import static org.hamcrest.collection.IsIterableContainingInAnyOrder.containsInA
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
 
 /**
  * @author Simon Thoresen Hult
@@ -322,20 +320,6 @@ public class ContainerClusterTest {
         
     }
 
-    @Test
-    public void requireCuratorConfig() {
-        DeployState state = new DeployState.Builder().build();
-        MockRoot root = new MockRoot("foo", state);
-        ApplicationContainerCluster cluster = new ApplicationContainerCluster(root, "container0", "container1", state);
-        addContainer(root.deployLogger(), cluster, "c1", "host-c1");
-        addContainer(root.deployLogger(), cluster, "c2", "host-c2");
-        CuratorConfig.Builder configBuilder = new CuratorConfig.Builder();
-        cluster.getConfig(configBuilder);
-        CuratorConfig config = configBuilder.build();
-        assertEquals(List.of("host-c1", "host-c2"), config.server().stream().map(CuratorConfig.Server::hostname).collect(Collectors.toList()));
-        assertTrue(config.zookeeperLocalhostAffinity());
-    }
-
     private void verifyTesterApplicationInstalledBundles(Zone zone, List<String> expectedBundleNames) {
         ApplicationId appId = ApplicationId.from("tenant", "application", "instance-t");
         DeployState state = new DeployState.Builder().properties(
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 32dd5cc944c..13c1631e0ce 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
@@ -40,13 +40,11 @@ import com.yahoo.vespa.defaults.Defaults;
 import com.yahoo.vespa.model.AbstractService;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.container.ApplicationContainer;
-import com.yahoo.vespa.model.container.ApplicationContainerCluster;
 import com.yahoo.vespa.model.container.ContainerCluster;
 import com.yahoo.vespa.model.container.SecretStore;
 import com.yahoo.vespa.model.container.component.Component;
 import com.yahoo.vespa.model.container.http.ConnectorFactory;
 import com.yahoo.vespa.model.content.utils.ContentClusterUtils;
-import com.yahoo.vespa.model.test.VespaModelTester;
 import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithFilePkg;
 import org.hamcrest.Matchers;
 import org.junit.Rule;
@@ -61,7 +59,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.stream.Collectors;
 
@@ -874,46 +871,6 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase {
         assertThat(connectorConfig.ssl().caCertificate(), isEmptyString());
     }
 
-    @Test
-    public void cluster_with_zookeeper() {
-        Function<Integer, String> servicesXml = (nodeCount) -> "<container version='1.0' id='default'>" +
-                                                               "<nodes count='" + nodeCount + "'/>" +
-                                                               "<zookeeper/>" +
-                                                               "</container>";
-        VespaModelTester tester = new VespaModelTester();
-        tester.addHosts(3);
-        {
-            VespaModel model = tester.createModel(servicesXml.apply(3), true);
-            String componentId = "com.yahoo.vespa.curator.Curator";
-            ApplicationContainerCluster cluster = model.getContainerClusters().get("default");
-            assertNotNull(cluster);
-            Component<?, ?> curatorComponent = cluster.getComponentsMap().get(ComponentId.fromString(componentId));
-            assertNotNull(curatorComponent);
-        }
-        {
-            try {
-                tester.createModel(servicesXml.apply(1), true);
-                fail("Expected exception");
-            } catch (IllegalArgumentException ignored) {}
-        }
-        {
-            String xmlWithNodes =
-                    "<?xml version='1.0' encoding='utf-8' ?>" +
-                    "<services>" +
-                    "  <container version='1.0' id='container1'>" +
-                    "     <zookeeper/>" +
-                    "     <nodes of='content1'/>" +
-                    "  </container>" +
-                    "  <content version='1.0' id='content1'>" +
-                    "     <nodes count='3'/>" +
-                    "   </content>" +
-                    "</services>";
-            try {
-                tester.createModel(xmlWithNodes, true);
-                fail("Expected exception");
-            } catch (IllegalArgumentException ignored) {}
-        }
-    }
 
     private Element generateContainerElementWithRenderer(String rendererId) {
         return DomBuilderTest.parse(
@@ -923,5 +880,4 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase {
                 "  </search>",
                 "</container>");
     }
-
 }