Update tests and fix host/port allocation.

11 files changed, 79 insertions, 38 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 072882a17cc..bda9b37b0a1 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
@@ -203,12 +203,13 @@ public class Admin extends AbstractConfigProducer implements Serializable {
     }
 
     private void addMetricsProxyCluster(List<HostResource> hosts, DeployState deployState) {
-        var metricsProxyCluster = new MetricsProxyContainerCluster(this, "metrics-proxies", deployState);
-        hosts.forEach(host -> {
-            var container = new MetricsProxyContainer(metricsProxyCluster);
+        var metricsProxyCluster = new MetricsProxyContainerCluster(this, "metrics", deployState);
+        int index = 0;
+        for (var host : hosts) {
+            var container = new MetricsProxyContainer(metricsProxyCluster, index++);
             addAndInitializeService(deployState.getDeployLogger(), host, container);
             metricsProxyCluster.addContainer(container);
-        });
+        }
     }
 
     private void addCommonServices(HostResource host, DeployState deployState) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java
index 21dbe486f28..965ac3e80f1 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java
@@ -13,8 +13,8 @@ import static com.yahoo.config.model.api.container.ContainerServiceType.METRICS_
  */
 public class MetricsProxyContainer extends Container {
 
-    public MetricsProxyContainer(AbstractConfigProducer parent) {
-        super(parent, "" + 0, 0);
+    public MetricsProxyContainer(AbstractConfigProducer parent, int index) {
+        super(parent, "" + index, index);
     }
 
     @Override
@@ -22,4 +22,25 @@ public class MetricsProxyContainer extends Container {
         return METRICS_PROXY_CONTAINER;
     }
 
+    @Override
+    public int getWantedPort() {
+        return 19092; // TODO: current metrics-proxy uses 19091 as rpc port, will now get 19093.
+    }
+
+    @Override
+    public boolean requiresWantedPort() {
+        return true;
+    }
+
+    @Override
+    public int getPortCount() {
+        return super.getPortCount() + 1;
+    }
+
+    @Override
+    protected void tagServers() {
+        super.tagServers();
+        portsMeta.on(numHttpServerPorts).tag("rpc").tag("metrics");
+    }
+
 }
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 18b30f26739..437df42d531 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
@@ -13,6 +13,7 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC
 
     public MetricsProxyContainerCluster(AbstractConfigProducer<?> parent, String name, DeployState deployState) {
         super(parent, name, name, deployState);
+        setRpcServerEnabled(false);
         addDefaultHandlersExceptStatus();
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java b/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java
index 8e0a06ad69c..6f5eaee2098 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java
@@ -69,7 +69,7 @@ public abstract class Container extends AbstractService implements
 
     private final JettyHttpServer defaultHttpServer = new JettyHttpServer(new ComponentId("DefaultHttpServer"));
 
-    private final int numHttpServerPorts;
+    protected final int numHttpServerPorts;
     private static final int numRpcServerPorts = 2;
 
     protected Container(AbstractConfigProducer parent, String name, int index) {
@@ -155,7 +155,7 @@ public abstract class Container extends AbstractService implements
         tagServers();
     }
 
-    private void tagServers() {
+    protected void tagServers() {
         if (numHttpServerPorts > 0) {
             portsMeta.on(0).tag("http").tag("query").tag("external").tag("state");
         }
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 d7819f63f23..a370e0dc38b 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
@@ -674,7 +674,7 @@ public abstract class ContainerCluster<CONTAINER extends Container>
 
     boolean messageBusEnabled() { return messageBusEnabled; }
 
-    public void setRpcServerEnabled(boolean rpcServerEnabled) { this.rpcServerEnabled = rpcServerEnabled; }
+    public final void setRpcServerEnabled(boolean rpcServerEnabled) { this.rpcServerEnabled = rpcServerEnabled; }
 
     boolean rpcServerEnabled() { return rpcServerEnabled; }
 
diff --git a/config-model/src/test/java/com/yahoo/config/model/deploy/SystemModelTestCase.java b/config-model/src/test/java/com/yahoo/config/model/deploy/SystemModelTestCase.java
index 7a2c702a103..074ff610e9a 100644
--- a/config-model/src/test/java/com/yahoo/config/model/deploy/SystemModelTestCase.java
+++ b/config-model/src/test/java/com/yahoo/config/model/deploy/SystemModelTestCase.java
@@ -115,8 +115,8 @@ public class SystemModelTestCase {
         assertEquals(host2, host3);
 
         // all three host aliases are for the same host, so the number of services should be 3 + 8
-        // (3 simpleservices and logd, configproxy, config sentinel, admin server config server, slobrok and log server)
-        assertEquals(9, host1.getServices().size());
+        // (3 simpleservices and logd, configproxy, config sentinel, admin server config server, slobrok, logserver and metricsproxy)
+        assertEquals(10, host1.getServices().size());
 
         assertNotNull(host1.getService("simpleservice"));
         assertNotNull(host1.getService("simpleservice2"));
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java
index 2c8887b0247..f493bb568f6 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java
@@ -28,6 +28,7 @@ import org.junit.Test;
 
 import java.util.Set;
 
+import static com.yahoo.config.model.api.container.ContainerServiceType.METRICS_PROXY_CONTAINER;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -37,9 +38,6 @@ import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 
-/**
- * @author gjoranv
- */
 public class AdminTestCase {
 
     private static final String TESTDIR = "src/test/cfg/admin/";
@@ -100,11 +98,12 @@ public class AdminTestCase {
         SentinelConfig.Builder b = new SentinelConfig.Builder();
         vespaModel.getConfig(b, localhostConfigId);
         SentinelConfig sentinelConfig = new SentinelConfig(b);
-        assertThat(sentinelConfig.service().size(), is(4));
+        assertThat(sentinelConfig.service().size(), is(5));
         assertThat(sentinelConfig.service(0).name(), is("logserver"));
         assertThat(sentinelConfig.service(1).name(), is("slobrok"));
         assertThat(sentinelConfig.service(2).name(), is("slobrok2"));
-        assertThat(sentinelConfig.service(3).name(), is("logd"));
+        assertThat(sentinelConfig.service(3).name(), is(METRICS_PROXY_CONTAINER.serviceName));
+        assertThat(sentinelConfig.service(4).name(), is("logd"));
     }
 
     /**
@@ -135,10 +134,11 @@ public class AdminTestCase {
         SentinelConfig.Builder b = new SentinelConfig.Builder();
         vespaModel.getConfig(b, localhostConfigId);
         SentinelConfig sentinelConfig = new SentinelConfig(b);
-        assertThat(sentinelConfig.service().size(), is(3));
+        assertThat(sentinelConfig.service().size(), is(4));
         assertThat(sentinelConfig.service(0).name(), is("logserver"));
         assertThat(sentinelConfig.service(1).name(), is("slobrok"));
-        assertThat(sentinelConfig.service(2).name(), is("logd"));
+        assertThat(sentinelConfig.service(2).name(), is(METRICS_PROXY_CONTAINER.serviceName));
+        assertThat(sentinelConfig.service(3).name(), is("logd"));
         assertThat(sentinelConfig.service(0).affinity().cpuSocket(), is(-1));
         assertTrue(sentinelConfig.service(0).preShutdownCommand().isEmpty());
 
@@ -289,10 +289,11 @@ public class AdminTestCase {
         TestRoot root = new TestDriver().buildModel(state);
         String localhost = HostName.getLocalhost();
         SentinelConfig sentinelConfig = root.getConfig(SentinelConfig.class, "hosts/" + localhost);
-        assertThat(sentinelConfig.service().size(), is(3));
+        assertThat(sentinelConfig.service().size(), is(4));
         assertThat(sentinelConfig.service(0).name(), is("logserver"));
         assertThat(sentinelConfig.service(1).name(), is("slobrok"));
-        assertThat(sentinelConfig.service(2).name(), is("logd"));
+        assertThat(sentinelConfig.service(2).name(), is(METRICS_PROXY_CONTAINER.serviceName));
+        assertThat(sentinelConfig.service(3).name(), is("logd"));
     }
 
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java
index 9f68b8b32bd..917927a72aa 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java
@@ -5,6 +5,7 @@ import com.yahoo.cloud.config.LogforwarderConfig;
 import com.yahoo.cloud.config.SentinelConfig;
 import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.model.NullConfigModelRegistry;
+import com.yahoo.config.model.api.container.ContainerServiceType;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.deploy.TestProperties;
 import com.yahoo.config.model.provision.Hosts;
@@ -26,7 +27,9 @@ import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
-import static com.yahoo.vespa.model.admin.monitoring.DefaultMetricsConsumer.VESPA_CONSUMER_ID;
+import static com.yahoo.config.model.api.container.ContainerServiceType.CONTAINER;
+import static com.yahoo.config.model.api.container.ContainerServiceType.METRICS_PROXY_CONTAINER;
+import static com.yahoo.config.model.api.container.ContainerServiceType.QRSERVER;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
@@ -71,10 +74,13 @@ public class DedicatedAdminV4Test {
         VespaModel model = createModel(hosts, services);
         assertEquals(3, model.getHosts().size());
 
-        assertHostContainsServices(model, "hosts/myhost0", "slobrok", "logd");
-        assertHostContainsServices(model, "hosts/myhost1", "slobrok", "logd");
+        assertHostContainsServices(model, "hosts/myhost0", "slobrok", "logd",
+                                   METRICS_PROXY_CONTAINER.serviceName);
+        assertHostContainsServices(model, "hosts/myhost1", "slobrok", "logd",
+                                   METRICS_PROXY_CONTAINER.serviceName);
         // Note: A container is always added on logserver host
-        assertHostContainsServices(model, "hosts/myhost2", "logserver", "logd", "container");
+        assertHostContainsServices(model, "hosts/myhost2", "logserver", "logd",
+                                   METRICS_PROXY_CONTAINER.serviceName, CONTAINER.serviceName);
 
         Monitoring monitoring = model.getAdmin().getMonitoring();
         assertEquals("vespa.routing", monitoring.getClustername());
@@ -126,10 +132,14 @@ public class DedicatedAdminV4Test {
         assertEquals(4, model.getHosts().size());
 
         // 4 slobroks, 2 per cluster where possible
-        assertHostContainsServices(model, "hosts/myhost0", "slobrok", "logd", "logserver", "qrserver");
-        assertHostContainsServices(model, "hosts/myhost1", "slobrok", "logd", "qrserver");
-        assertHostContainsServices(model, "hosts/myhost2", "slobrok", "logd", "qrserver");
-        assertHostContainsServices(model, "hosts/myhost3", "slobrok", "logd",  "qrserver");
+        assertHostContainsServices(model, "hosts/myhost0", "slobrok", "logd", "logserver",
+                                   METRICS_PROXY_CONTAINER.serviceName, QRSERVER.serviceName);
+        assertHostContainsServices(model, "hosts/myhost1", "slobrok", "logd",
+                                   METRICS_PROXY_CONTAINER.serviceName, QRSERVER.serviceName);
+        assertHostContainsServices(model, "hosts/myhost2", "slobrok", "logd",
+                                   METRICS_PROXY_CONTAINER.serviceName, QRSERVER.serviceName);
+        assertHostContainsServices(model, "hosts/myhost3", "slobrok", "logd",
+                                   METRICS_PROXY_CONTAINER.serviceName, QRSERVER.serviceName);
     }
 
     @Test
@@ -147,10 +157,13 @@ public class DedicatedAdminV4Test {
         VespaModel model = createModel(hosts, services);
         assertEquals(3, model.getHosts().size());
 
-        assertHostContainsServices(model, "hosts/myhost0", "logd", "logforwarder", "slobrok");
-        assertHostContainsServices(model, "hosts/myhost1", "logd", "logforwarder", "slobrok");
+        assertHostContainsServices(model, "hosts/myhost0", "logd", "logforwarder", "slobrok",
+                                   METRICS_PROXY_CONTAINER.serviceName);
+        assertHostContainsServices(model, "hosts/myhost1", "logd", "logforwarder", "slobrok",
+                                   METRICS_PROXY_CONTAINER.serviceName);
         // Note: A container is always added on logserver host
-        assertHostContainsServices(model, "hosts/myhost2", "logd", "logforwarder", "logserver", "container");
+        assertHostContainsServices(model, "hosts/myhost2", "logd", "logforwarder", "logserver",
+                                   METRICS_PROXY_CONTAINER.serviceName, CONTAINER.serviceName);
 
         Set<String> configIds = model.getConfigIds();
         // 1 logforwarder on each host
@@ -193,7 +206,8 @@ public class DedicatedAdminV4Test {
                 .properties(new TestProperties().setHostedVespa(true)));
         assertEquals(1, model.getHosts().size());
         // Should create a container on the same node as logserver
-        assertHostContainsServices(model, "hosts/myhost0", "slobrok", "logd", "logserver", "container");
+        assertHostContainsServices(model, "hosts/myhost0", "slobrok", "logd", "logserver",
+                                   METRICS_PROXY_CONTAINER.serviceName , CONTAINER.serviceName);
     }
 
     private Set<String> serviceNames(VespaModel model, String hostname) {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigValueChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigValueChangeValidatorTest.java
index f13f53e8648..66f4a714e74 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigValueChangeValidatorTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigValueChangeValidatorTest.java
@@ -58,9 +58,10 @@ public class ConfigValueChangeValidatorTest {
                 createVespaModel(createQrStartConfigSegment(true, 2096)),
                 createVespaModel(createQrStartConfigSegment(false, 2096))
         );
-        assertEquals(2, changes.size());
+        assertEquals(3, changes.size());
         assertComponentsEquals(changes, "default/container.0", 0);
         assertComponentsEquals(changes, "admin/cluster-controllers/0", 1);
+        assertComponentsEquals(changes, "admin/metrics/0", 2);
     }
 
     @Test
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java
index 2c3e447533d..e7a1cbd6d8b 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/builder/xml/dom/ContentBuilderTest.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.model.builder.xml.dom;
 import com.yahoo.collections.CollectionUtil;
 import com.yahoo.config.ConfigInstance;
 import com.yahoo.config.application.api.ApplicationPackage;
+import com.yahoo.config.model.api.container.ContainerServiceType;
 import com.yahoo.config.model.builder.xml.test.DomBuilderTest;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.deploy.TestProperties;
@@ -34,6 +35,8 @@ import org.junit.rules.ExpectedException;
 import java.util.Arrays;
 import java.util.List;
 
+import static com.yahoo.config.model.api.container.ContainerServiceType.CLUSTERCONTROLLER_CONTAINER;
+import static com.yahoo.config.model.api.container.ContainerServiceType.METRICS_PROXY_CONTAINER;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.Matchers.containsString;
 import static org.junit.Assert.assertEquals;
@@ -236,10 +239,9 @@ public class ContentBuilderTest extends DomBuilderTest {
         assertEquals(1, cluster.getRoot().getHostSystem().getHosts().size());
         HostResource h = cluster.getRoot().getHostSystem().getHost("mockhost");
         String [] expectedServices = {
-                "logd", "configproxy",
-                "config-sentinel", "configserver", "logserver",
-                "slobrok", "container-clustercontroller",
-                "storagenode", "distributor","searchnode","transactionlogserver"
+                "logd", "configproxy", "config-sentinel", "configserver", "logserver",
+                "slobrok", "storagenode", "distributor","searchnode","transactionlogserver",
+                CLUSTERCONTROLLER_CONTAINER.serviceName, METRICS_PROXY_CONTAINER.serviceName
         };
         assertServices(h, expectedServices);
 
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java
index 14659494780..64fc4bc9062 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java
@@ -266,7 +266,7 @@ public class VespaModelTestCase {
         assertThat(hosts.size(), is(1));
         //logd, config proxy, sentinel, config server, slobrok, log server
         HostInfo host = hosts.iterator().next();
-        assertThat(host.getServices().size(), is(6));
+        assertThat(host.getServices().size(), is(7));
         new LogdConfig((LogdConfig.Builder) model.getConfig(new LogdConfig.Builder(), "admin/model"));
 
     }