* try rewriting container port allocation

* simplify access to defaultHttpServer

3 files changed, 47 insertions, 45 deletions

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 01168c59442..cbfbe0d191e 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
@@ -58,19 +58,6 @@ public class MetricsProxyContainer extends Container implements
         addMetricsProxyComponent(VespaServices.class);
     }
 
-    int metricsRpcPortOffset() {
-        if (numHttpServerPorts != 2) {
-            throw new IllegalArgumentException("expecting 2 http server ports");
-        }
-        if (numMessageBusPorts() != 0) {
-            throw new IllegalArgumentException("expecting 0 message bus ports");
-        }
-        if (numRpcPorts() != 1) {
-            throw new IllegalArgumentException("expecting 1 rpc port");
-        }
-        return numHttpServerPorts + numMessageBusPorts() + numRpcPorts();
-    }
-
     @Override
     protected ContainerServiceType myServiceType() {
         return METRICS_PROXY_CONTAINER;
@@ -88,30 +75,44 @@ public class MetricsProxyContainer extends Container implements
         return true;
     }
 
+    private int metricsRpcPort;
+
     // Must have predictable ports for both http and rpc.
     @Override
     public void allocatePorts(int start, PortAllocBridge from) {
         if (start == 0) start = BASEPORT;
-        from.wantPort(start++, "http");
+        if (getHttp() != null) {
+            throw new IllegalArgumentException("unexpected HTTP setup");
+        }
+        allocatedSearchPort = from.wantPort(start++, "http");
+        // XXX remove:
         from.wantPort(start++, "http/1");
-        from.wantPort(start++, "rpc/admin");
-        from.wantPort(start++, "rpc/metrics");
+        // XXX change to 1:
+        numHttpServerPorts = 2;
+        if (numMessageBusPorts() != 0) {
+            throw new IllegalArgumentException("expecting 0 message bus ports");
+        }
+        if (numRpcPorts() != 1) {
+            throw new IllegalArgumentException("expecting 1 rpc port");
+        }
+        allocatedRpcPort = from.wantPort(start++, "rpc/admin");
+        metricsRpcPort = from.wantPort(start++, "rpc/metrics");
     }
 
     @Override
     public int getPortCount() {
-        return metricsRpcPortOffset() + 1;
+        return 4;
     }
 
     @Override
     protected void tagServers() {
         super.tagServers();
-        portsMeta.on(metricsRpcPortOffset()).tag("rpc").tag("metrics");
+        portsMeta.on(3).tag("rpc").tag("metrics");
     }
 
     @Override
     public void getConfig(RpcConnectorConfig.Builder builder) {
-        builder.port(getRelativePort(metricsRpcPortOffset()));
+        builder.port(metricsRpcPort);
     }
 
     @Override
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 3bc68e4a879..eca790cfc93 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
@@ -75,7 +75,7 @@ public abstract class Container extends AbstractService implements
 
     private final JettyHttpServer defaultHttpServer = new JettyHttpServer(new ComponentId("DefaultHttpServer"));
 
-    protected final int numHttpServerPorts;
+    protected int numHttpServerPorts = 0;
 
     protected Container(AbstractConfigProducer parent, String name, int index) {
         this(parent, name, false, index);
@@ -89,13 +89,7 @@ public abstract class Container extends AbstractService implements
         this.index = index;
 
         if (getHttp() == null) {
-            // TODO Vespa 8: set to 1. The second (health) port has not been used since Vespa 6 or earlier.
-            numHttpServerPorts = 2;
             addChild(defaultHttpServer);
-        } else if (getHttp().getHttpServer() == null) {
-            numHttpServerPorts = 0;
-        } else {
-            numHttpServerPorts = getHttp().getHttpServer().getConnectorFactories().size();
         }
         addBuiltinHandlers();
 
@@ -139,10 +133,15 @@ public abstract class Container extends AbstractService implements
         return (parent instanceof ContainerCluster) ? ((ContainerCluster) parent).getHttp() : null;
     }
 
-    public JettyHttpServer getDefaultHttpServer() {
-        return defaultHttpServer;
+    public JettyHttpServer getHttpServer() {
+        Http http = getHttp();
+        if (http == null) {
+            return defaultHttpServer;
+        } else {
+            return http.getHttpServer();
+        }
     }
-    
+
     /** Returns the index of this node. The index of a given node is stable through changes with best effort. */
     public final int index() { return index; }
 
@@ -168,7 +167,6 @@ public abstract class Container extends AbstractService implements
         if (numHttpServerPorts > 0) {
             portsMeta.on(offset++).tag("http").tag("query").tag("external").tag("state");
         }
-
         for (int i = 1; i < numHttpServerPorts; i++)
             portsMeta.on(offset++).tag("http").tag("external");
 
@@ -228,8 +226,7 @@ public abstract class Container extends AbstractService implements
      * @return the number of ports needed by the Container
      */
     public int getPortCount() {
-        // TODO Vespa 8: remove +2, only here for historical reasons
-        int httpPorts = (getHttp() != null) ? 0 : numHttpServerPorts + 2;
+        int httpPorts = (getHttp() != null) ? 0 : 2;
         return httpPorts + numMessageBusPorts() + numRpcPorts();
     }
 
@@ -239,26 +236,31 @@ public abstract class Container extends AbstractService implements
         int off = 2;
         if (getHttp() == null) {
             if (requireSpecificPorts) {
-                from.requirePort(start, "http");
+                allocatedSearchPort = from.requirePort(start, "http");
             } else {
-                from.allocatePort("http");
+                allocatedSearchPort = from.allocatePort("http");
             }
+            // XXX unused - remove:
             from.allocatePort("http/1");
+            // XXX change to 1:
+            numHttpServerPorts = 2;
         } else if (getHttp().getHttpServer() == null) {
             // no http server ports
+            numHttpServerPorts = 0;
         } else {
             for (ConnectorFactory connectorFactory : getHttp().getHttpServer().getConnectorFactories()) {
                 int port = getPort(connectorFactory);
                 String name = "http/" + connectorFactory.getName();
                 from.requirePort(port, name);
+                ++numHttpServerPorts;
             }
         }
         if (messageBusEnabled()) {
-            from.allocatePort("messaging");
+            allocatedMessagingPort = from.allocatePort("messaging");
             ++off;
         }
         if (rpcServerEnabled()) {
-            from.allocatePort("rpc/admin");
+            allocatedRpcPort = from.allocatePort("rpc/admin");
             ++off;
         }
         // TODO: remove this
@@ -269,6 +271,7 @@ public abstract class Container extends AbstractService implements
         }
     }
 
+    protected int allocatedSearchPort = 0;
     /**
      * @return the actual search port
      * TODO: Remove. Use {@link #getPortsMeta()} and check tags in conjunction with {@link #getRelativePort(int)}.
@@ -276,21 +279,19 @@ public abstract class Container extends AbstractService implements
     public int getSearchPort() {
         if (getHttp() != null)
             throw new AssertionError("getSearchPort must not be used when http section is present.");
-
-        return getRelativePort(0);
+        return allocatedSearchPort;
     }
 
+    protected int allocatedRpcPort = 0;
     private int getRpcPort() {
-        return rpcServerEnabled() ? getRelativePort(numHttpServerPorts + numMessageBusPorts()) : 0;
+        return allocatedRpcPort;
     }
-
     protected int numRpcPorts() { return rpcServerEnabled() ? 1 : 0; }
 
-
+    protected int allocatedMessagingPort = 0;
     private int getMessagingPort() {
-        return messageBusEnabled() ? getRelativePort(numHttpServerPorts) : 0;
+        return allocatedMessagingPort;
     }
-
     protected int numMessageBusPorts() { return messageBusEnabled() ? 1 : 0; }
 
     @Override
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java
index ef56a42cf1a..97b520f9803 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java
@@ -72,7 +72,7 @@ public class MetricsProxyContainerTest {
         VespaModel model = getModel(servicesWithContent(), self_hosted);
         MetricsProxyContainer container = (MetricsProxyContainer)model.id2producer().get(CONTAINER_CONFIG_ID);
 
-        int offset = container.metricsRpcPortOffset();
+        int offset = 3;
         assertEquals(2, container.getPortsMeta().getTagsAt(offset).size());
         assertTrue(container.getPortsMeta().getTagsAt(offset).contains("rpc"));
         assertTrue(container.getPortsMeta().getTagsAt(offset).contains("metrics"));
@@ -86,7 +86,7 @@ public class MetricsProxyContainerTest {
         VespaModel model = getModel(servicesWithContent(), self_hosted);
         MetricsProxyContainer container = (MetricsProxyContainer)model.id2producer().get(CONTAINER_CONFIG_ID);
 
-        int offset = container.metricsRpcPortOffset() - 1;
+        int offset = 2;
         assertEquals(2, container.getPortsMeta().getTagsAt(offset).size());
         assertTrue(container.getPortsMeta().getTagsAt(offset).contains("rpc"));
         assertTrue(container.getPortsMeta().getTagsAt(offset).contains("admin"));