Revert "Minor cleanup in config-model"

6 files changed, 76 insertions, 40 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java
index 1928fb0f9ad..79f65264249 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java
@@ -79,19 +79,19 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable
 
     private ZooKeepersConfigProvider zooKeepersConfigProvider;
     private final FileDistributionConfigProducer fileDistribution;
-    private final boolean multiTenant;
+    private final boolean multitenant;
 
     public Admin(AbstractConfigProducer<?> parent,
                  Monitoring monitoring,
                  Metrics metrics,
-                 boolean multiTenant,
+                 boolean multitenant,
                  boolean isHostedVespa,
                  ApplicationType applicationType) {
         super(parent, "admin");
         this.isHostedVespa = isHostedVespa;
         this.monitoring = monitoring;
         this.metrics = metrics;
-        this.multiTenant = multiTenant;
+        this.multitenant = multitenant;
         this.fileDistribution = new FileDistributionConfigProducer(parent);
         this.applicationType = applicationType;
     }
@@ -149,7 +149,7 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable
     public void setClusterControllers(ClusterControllerContainerCluster clusterControllers, DeployState deployState) {
         this.clusterControllers = clusterControllers;
         if (isHostedVespa) {
-            // Prefer to put slobroks on the admin cluster running cluster controllers to avoid unnecessary
+            // Prefer to put Slobroks on the admin cluster running cluster controllers to avoid unnecessary
             // movement of the slobroks when there are changes to the content cluster nodes
             removeSlobroks();
             addSlobroks(createSlobroksOn(clusterControllers, deployState));
@@ -250,9 +250,9 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable
             boolean actuallyAdd = true;
             var membership = host.spec().membership();
             if (membership.isPresent()) {
-                var clusterType = membership.get().cluster().type();
+                var clustertype = membership.get().cluster().type();
                 // XXX should skip only if this.isHostedVespa is true?
-                if (clusterType == ClusterSpec.Type.admin) {
+                if (clustertype == ClusterSpec.Type.admin) {
                     actuallyAdd = logForwarderIncludeAdmin;
                 }
             }
@@ -263,7 +263,8 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable
     }
 
     private void addConfigSentinel(DeployState deployState, HostResource host,
-                                   ApplicationId applicationId, Zone zone, ModelContext.FeatureFlags featureFlags) {
+                                   ApplicationId applicationId, Zone zone, ModelContext.FeatureFlags featureFlags)
+    {
         ConfigSentinel configSentinel = new ConfigSentinel(host.getHost(), applicationId, zone, featureFlags);
         addAndInitializeService(deployState, host, configSentinel);
         host.getHost().setConfigSentinel(configSentinel);
@@ -319,7 +320,9 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable
         return slobs;
     }
 
-    public boolean multiTenant() { return multiTenant; }
+    public boolean multitenant() {
+        return multitenant;
+    }
 
     public ApplicationType getApplicationType() { return applicationType; }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java
index c3bfeca91ae..4c74282c061 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.builder.xml.dom;
 
+import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.model.ConfigModelContext.ApplicationType;
 import com.yahoo.config.model.api.ConfigServerSpec;
 import com.yahoo.config.model.deploy.DeployState;
@@ -19,6 +20,7 @@ import com.yahoo.vespa.model.admin.monitoring.builder.Metrics;
 import com.yahoo.vespa.model.admin.monitoring.builder.PredefinedMetricSets;
 import com.yahoo.vespa.model.admin.monitoring.builder.xml.MetricsBuilder;
 import org.w3c.dom.Element;
+
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
@@ -33,11 +35,13 @@ public abstract class DomAdminBuilderBase extends VespaDomBuilder.DomConfigProdu
 
     private final ApplicationType applicationType;
     protected final List<ConfigServerSpec> configServerSpecs;
-    protected final boolean multiTenant;
+    protected final boolean multitenant;
 
-    DomAdminBuilderBase(ApplicationType applicationType, boolean multiTenant, List<ConfigServerSpec> configServerSpecs) {
+    DomAdminBuilderBase(ApplicationType applicationType,
+                        boolean multitenant,
+                        List<ConfigServerSpec> configServerSpecs) {
         this.applicationType = applicationType;
-        this.multiTenant = multiTenant;
+        this.multitenant = multitenant;
         this.configServerSpecs = configServerSpecs;
     }
 
@@ -61,7 +65,7 @@ public abstract class DomAdminBuilderBase extends VespaDomBuilder.DomConfigProdu
         Monitoring monitoring = getMonitoring(XML.getChild(adminElement,"monitoring"), deployState.isHosted());
         Metrics metrics = new MetricsBuilder(applicationType, PredefinedMetricSets.get())
                                   .buildMetrics(XML.getChild(adminElement, "metrics"));
-        Admin admin = new Admin(parent, monitoring, metrics, multiTenant, deployState.isHosted(), applicationType);
+        Admin admin = new Admin(parent, monitoring, metrics, multitenant, deployState.isHosted(), applicationType);
         doBuildAdmin(deployState, admin, adminElement);
         new ModelConfigProvider(admin);
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java
index 68e34f7bf06..e081848d01c 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java
@@ -1,7 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.builder.xml.dom;
 
-import com.yahoo.config.model.ConfigModelContext.ApplicationType;
+import com.yahoo.config.model.ConfigModelContext;
 import com.yahoo.config.model.api.ConfigServerSpec;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
@@ -17,6 +17,7 @@ import com.yahoo.vespa.model.admin.clustercontroller.ClusterControllerContainerC
 import com.yahoo.vespa.model.builder.xml.dom.VespaDomBuilder.DomConfigProducerBuilder;
 import com.yahoo.vespa.model.container.Container;
 import org.w3c.dom.Element;
+
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
@@ -31,39 +32,45 @@ public class DomAdminV2Builder extends DomAdminBuilderBase {
 
     private static final String ATTRIBUTE_CLUSTER_CONTROLLER_STANDALONE_ZK = "standalone-zookeeper";
 
-    public DomAdminV2Builder(ApplicationType applicationType, boolean multiTenant, List<ConfigServerSpec> configServerSpecs) {
-        super(applicationType, multiTenant, configServerSpecs);
+    public DomAdminV2Builder(ConfigModelContext.ApplicationType applicationType,
+                             boolean multitenant,
+                             List<ConfigServerSpec> configServerSpecs) {
+        super(applicationType, multitenant, configServerSpecs);
     }
 
     @Override
     protected void doBuildAdmin(DeployState deployState, Admin admin, Element adminE) {
+        List<Configserver> configservers = parseConfigservers(deployState, admin, adminE);
         admin.setLogserver(parseLogserver(deployState, admin, adminE));
-        admin.addConfigservers(parseConfigServers(deployState, admin, adminE));
+        admin.addConfigservers(configservers);
         admin.addSlobroks(getSlobroks(deployState, admin, XML.getChild(adminE, "slobroks")));
-        if ( ! admin.multiTenant())
+        if ( ! admin.multitenant())
             admin.setClusterControllers(addConfiguredClusterControllers(deployState, admin, adminE), deployState);
+
         addLogForwarders(new ModelElement(adminE).child("logforwarding"), admin);
     }
 
-    private List<Configserver> parseConfigServers(DeployState deployState, Admin admin, Element adminE) {
-        List<Configserver> configServers = multiTenant
-                ? getConfigServersFromSpec(deployState, admin)
-                : getConfigServers(deployState, admin, adminE);
-
-        if (configServers.isEmpty() && !multiTenant)
-            configServers = createSingleConfigServer(deployState, admin);
-        if (configServers.size() % 2 == 0)
+    private List<Configserver> parseConfigservers(DeployState deployState, Admin admin, Element adminE) {
+        List<Configserver> configservers;
+        if (multitenant)
+            configservers = getConfigServersFromSpec(deployState, admin);
+        else
+            configservers = getConfigServers(deployState, admin, adminE);
+        if (configservers.isEmpty() && ! multitenant)
+            configservers = createSingleConfigServer(deployState, admin);
+        if (configservers.size() % 2 == 0)
             deployState.getDeployLogger().logApplicationPackage(Level.WARNING,
-                                                                "An even number (" + configServers.size() +
+                                                                "An even number (" + configservers.size() +
                                                                 ") of config servers have been configured. " +
                                                                 "This is discouraged, see doc for configuration server ");
-        return configServers;
+        return configservers;
     }
 
     private Logserver parseLogserver(DeployState deployState, Admin admin, Element adminE) {
         Element logserverE = XML.getChild(adminE, "logserver");
-        if (logserverE == null)
+        if (logserverE == null) {
             logserverE = XML.getChild(adminE, "adminserver");
+        }
         return new LogserverBuilder().build(deployState, admin, logserverE);
     }
 
@@ -76,9 +83,10 @@ public class DomAdminV2Builder extends DomAdminBuilderBase {
         List<Element> controllers = XML.getChildren(controllersElements, "cluster-controller");
         if (controllers.isEmpty()) return null;
 
-        boolean standaloneZooKeeper = "true".equals(controllersElements.getAttribute(ATTRIBUTE_CLUSTER_CONTROLLER_STANDALONE_ZK)) || multiTenant;
-        if (standaloneZooKeeper)
+        boolean standaloneZooKeeper = "true".equals(controllersElements.getAttribute(ATTRIBUTE_CLUSTER_CONTROLLER_STANDALONE_ZK)) || multitenant;
+        if (standaloneZooKeeper) {
             parent = new ClusterControllerCluster(parent, "standalone", deployState);
+        }
         var cluster = new ClusterControllerContainerCluster(parent,
                                                             "cluster-controllers",
                                                             "cluster-controllers",
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
index b6ee08cb5ec..dae2f7e8cb8 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java
@@ -33,9 +33,9 @@ public class DomAdminV4Builder extends DomAdminBuilderBase {
     private final Collection<ContainerModel> containerModels;
     private final ConfigModelContext context;
 
-    public DomAdminV4Builder(ConfigModelContext context, boolean multiTenant, List<ConfigServerSpec> configServerSpecs,
+    public DomAdminV4Builder(ConfigModelContext context, boolean multitenant, List<ConfigServerSpec> configServerSpecs,
                              Collection<ContainerModel> containerModels) {
-        super(context.getApplicationType(), multiTenant, configServerSpecs);
+        super(context.getApplicationType(), multitenant, configServerSpecs);
         this.containerModels = containerModels;
         this.context = context;
     }
@@ -131,7 +131,7 @@ public class DomAdminV4Builder extends DomAdminBuilderBase {
      * The list returns the same nodes on each invocation given the same available nodes.
      *
      * @param count the desired number of nodes. More nodes may be returned to ensure a smooth transition
-     *        on topology changes, and fewer nodes may be returned if fewer are available
+     *        on topology changes, and less nodes may be returned if fewer are available
      * @param minHostsPerContainerCluster the desired number of hosts per cluster
      */
     private List<HostResource> pickContainerHostsForSlobrok(int count, int minHostsPerContainerCluster) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
index 7f262b4aee7..776d23e5227 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
@@ -287,7 +287,7 @@ public class ContentCluster extends AbstractConfigProducer<AbstractConfigProduce
             if (context.properties().hostedVespa()) {
                 clusterControllers = getDedicatedSharedControllers(contentElement, admin, context, deployState, clusterName);
             }
-            else if (admin.multiTenant()) { // system tests: Put on logserver
+            else if (admin.multitenant()) { // system tests: Put on logserver
                 if (admin.getClusterControllers() == null) {
                     // TODO: logserver == null only obtains in unit tests, disallow it
                     List<HostResource> host = admin.getLogserver() == null ? List.of() : List.of(admin.getLogserver().getHostResource());
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2BuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2BuilderTest.java
index 093b0765530..1a41d2689a2 100755
--- a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2BuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2BuilderTest.java
@@ -2,7 +2,7 @@
 package com.yahoo.vespa.model.builder.xml.dom;
 
 import com.yahoo.cloud.config.log.LogdConfig;
-import com.yahoo.config.model.ConfigModelContext.ApplicationType;
+import com.yahoo.config.model.ConfigModelContext;
 import com.yahoo.config.model.api.ConfigServerSpec;
 import com.yahoo.config.model.builder.xml.test.DomBuilderTest;
 import com.yahoo.config.model.deploy.DeployState;
@@ -16,12 +16,13 @@ import com.yahoo.vespa.model.admin.monitoring.Monitoring;
 import org.junit.Before;
 import org.junit.Test;
 import org.w3c.dom.Element;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
-import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertEquals;
 
 /**
  * @author hmusum
@@ -35,6 +36,16 @@ public class DomAdminV2BuilderTest extends DomBuilderTest {
         root = new MockRoot("root");
     }
 
+    // Supported for backwards compatibility
+    private Element servicesConfigserver() {
+        return XML.getDocument(
+                        "<admin version=\"2.0\">" +
+                        "  <configserver hostalias=\"mockhost\"/>" +
+                        "  <adminserver hostalias=\"mockhost\"/>" +
+                        "</admin>").getDocumentElement();
+
+    }
+
     private Element servicesOverride() {
         return XML.getDocument(
                         "<admin version=\"2.0\">" +
@@ -120,10 +131,19 @@ public class DomAdminV2BuilderTest extends DomBuilderTest {
     }
 
     /**
+     * Tests that configserver works (deprecated, but allowed in admin 2.0)
+     */
+    @Test
+    public void adminWithConfigserverElement() {
+        Admin admin = buildAdmin(servicesConfigserver());
+        assertEquals(1, admin.getConfigservers().size());
+    }
+
+    /**
      * Tests that configservers/configserver works
      */
     @Test
-    public void adminWithConfigServersElement() {
+    public void adminWithConfigserversElement() {
         Admin admin = buildAdmin(servicesConfigservers());
         assertEquals(1, admin.getConfigservers().size());
     }
@@ -182,12 +202,13 @@ public class DomAdminV2BuilderTest extends DomBuilderTest {
     }
 
     private Admin buildAdmin(Element xml) {
-        return buildAdmin(xml, false, List.of());
+        return buildAdmin(xml, false, new ArrayList<>());
     }
 
-    private Admin buildAdmin(Element xml, boolean multitenant, List<ConfigServerSpec> configServers) {
+    private Admin buildAdmin(Element xml, boolean multitenant, List<ConfigServerSpec> configServerSpecs) {
         DeployState deployState = DeployState.createTestState();
-        DomAdminV2Builder domAdminBuilder = new DomAdminV2Builder(ApplicationType.DEFAULT, multitenant, configServers);
+        final DomAdminV2Builder domAdminBuilder =
+                new DomAdminV2Builder(ConfigModelContext.ApplicationType.DEFAULT, multitenant, configServerSpecs);
         Admin admin = domAdminBuilder.build(deployState, root, xml);
         admin.addPerHostServices(root.hostSystem().getHosts(), deployState);
         return admin;