Merge pull request #10556 from vespa-engine/arnej/more-robust-container-ports

Arnej/more robust container ports

3 files changed, 55 insertions, 78 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..8dff6aeb830 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,42 @@ 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");
+        portsMeta.on(0).tag("http").tag("query").tag("external").tag("state");
+
+        // XXX remove:
         from.wantPort(start++, "http/1");
-        from.wantPort(start++, "rpc/admin");
-        from.wantPort(start++, "rpc/metrics");
-    }
+        portsMeta.on(1).tag("unused");
 
-    @Override
-    public int getPortCount() {
-        return metricsRpcPortOffset() + 1;
+        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");
+        portsMeta.on(2).tag("rpc").tag("admin");
+        metricsRpcPort = from.wantPort(start++, "rpc/metrics");
+        portsMeta.on(3).tag("rpc").tag("metrics");
     }
 
     @Override
-    protected void tagServers() {
-        super.tagServers();
-        portsMeta.on(metricsRpcPortOffset()).tag("rpc").tag("metrics");
+    public int getPortCount() {
+        return 4;
     }
 
     @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..c1e644ba524 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,8 +75,6 @@ public abstract class Container extends AbstractService implements
 
     private final JettyHttpServer defaultHttpServer = new JettyHttpServer(new ComponentId("DefaultHttpServer"));
 
-    protected final int numHttpServerPorts;
-
     protected Container(AbstractConfigProducer parent, String name, int index) {
         this(parent, name, false, index);
     }
@@ -89,13 +87,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 +131,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; }
 
@@ -159,25 +156,6 @@ public abstract class Container extends AbstractService implements
         if (getHttp() == null) {
             initDefaultJettyConnector();
         }
-
-        tagServers();
-    }
-
-    protected void tagServers() {
-        int offset = 0;
-        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");
-
-        if (messageBusEnabled()) {
-            portsMeta.on(offset++).tag("rpc").tag("messaging");
-        }
-        if (rpcServerEnabled()) {
-            portsMeta.on(offset++).tag("rpc").tag("admin");
-        }
     }
 
     private int getPort(ConnectorFactory connectorFactory) {
@@ -228,22 +206,24 @@ 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();
     }
 
     @Override
     public void allocatePorts(int start, PortAllocBridge from) {
         if (start == 0) start = BASEPORT;
-        int off = 2;
+        int offset = 0;
         if (getHttp() == null) {
             if (requireSpecificPorts) {
-                from.requirePort(start, "http");
+                allocatedSearchPort = from.requirePort(start, "http");
             } else {
-                from.allocatePort("http");
+                allocatedSearchPort = from.allocatePort("http");
             }
+            portsMeta.on(offset++).tag("http").tag("query").tag("external").tag("state");
+            // XXX unused - remove:
             from.allocatePort("http/1");
+            portsMeta.on(offset++).tag("http").tag("external");
         } else if (getHttp().getHttpServer() == null) {
             // no http server ports
         } else {
@@ -251,24 +231,24 @@ public abstract class Container extends AbstractService implements
                 int port = getPort(connectorFactory);
                 String name = "http/" + connectorFactory.getName();
                 from.requirePort(port, name);
+                if (offset == 0) {
+                    portsMeta.on(offset++).tag("http").tag("query").tag("external").tag("state");
+                } else {
+                    portsMeta.on(offset++).tag("http").tag("external");
+                }
             }
         }
         if (messageBusEnabled()) {
-            from.allocatePort("messaging");
-            ++off;
+            allocatedMessagingPort = from.allocatePort("messaging");
+            portsMeta.on(offset++).tag("rpc").tag("messaging");
         }
         if (rpcServerEnabled()) {
-            from.allocatePort("rpc/admin");
-            ++off;
-        }
-        // TODO: remove this
-        if (getHttp() == null) {
-            from.allocatePort("unused/" + off);
-            ++off;
-            from.allocatePort("unused/" + off);
+            allocatedRpcPort = from.allocatePort("rpc/admin");
+            portsMeta.on(offset++).tag("rpc").tag("admin");
         }
     }
 
+    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 +256,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"));