Add feature flag for using a separate service type ('logserver-container') for logserver containers

8 files changed, 65 insertions, 11 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
index e873432f71d..edb7d878a38 100644
--- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
+++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
@@ -55,6 +55,7 @@ public interface ModelContext {
         boolean useDedicatedNodeForLogserver();
         boolean useFdispatchByDefault();
         boolean useAdaptiveDispatch();
+        boolean useSeparateServiceTypeForLogserverContainer();
     }
 
 }
diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
index 378c262cd72..c33eb9c6704 100644
--- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
+++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
@@ -8,7 +8,6 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.Rotation;
 import com.yahoo.config.provision.Zone;
-import com.yahoo.vespa.model.admin.Configserver;
 
 import java.net.URI;
 import java.util.Collections;
@@ -37,6 +36,7 @@ public class TestProperties implements ModelContext.Properties {
     private boolean useDedicatedNodeForLogserver = false;
     private boolean useFdispatchByDefault = true;
     private boolean useAdaptiveDispatch = false;
+    private boolean useSeparateServiceTypeForLogserverContainer = false;
 
     @Override public boolean multitenant() { return multitenant; }
     @Override public ApplicationId applicationId() { return applicationId; }
@@ -52,6 +52,7 @@ public class TestProperties implements ModelContext.Properties {
     @Override public boolean useAdaptiveDispatch() { return useAdaptiveDispatch; }
     @Override public boolean useDedicatedNodeForLogserver() { return useDedicatedNodeForLogserver; }
     @Override public boolean useFdispatchByDefault() { return useFdispatchByDefault; }
+    @Override public boolean useSeparateServiceTypeForLogserverContainer() { return useSeparateServiceTypeForLogserverContainer; }
 
     public TestProperties setApplicationId(ApplicationId applicationId) {
         this.applicationId = applicationId;
@@ -83,6 +84,11 @@ public class TestProperties implements ModelContext.Properties {
         return this;
     }
 
+    public TestProperties setUseSeparateServiceTypeForLogserverContainer(boolean useSeparateServiceTypeForLogserverContainer) {
+        this.useSeparateServiceTypeForLogserverContainer = useSeparateServiceTypeForLogserverContainer;
+        return this;
+    }
+
     public static class Spec implements ConfigServerSpec {
 
         private final String hostName;
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainer.java
index 78217064722..32e45a15e31 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainer.java
@@ -13,7 +13,9 @@ import com.yahoo.vespa.model.container.component.Handler;
  */
 public class LogserverContainer extends Container {
 
-    public LogserverContainer(AbstractConfigProducer parent) {
+    private final boolean useSeparateServiceTypeForLogserverContainer;
+
+    public LogserverContainer(AbstractConfigProducer parent, boolean useSeparateServiceTypeForLogserverContainer) {
         super(parent, "" + 0, 0, true);
         // Add base handlers and the log handler
         ContainerCluster logServerCluster = (ContainerCluster) parent;
@@ -22,12 +24,12 @@ public class LogserverContainer extends Container {
         logServerCluster.addDefaultRootHandler();
         logServerCluster.addVipHandler();
         addLogHandler(logServerCluster);
+        this.useSeparateServiceTypeForLogserverContainer = useSeparateServiceTypeForLogserverContainer;
     }
 
-    // TODO: Change service type to 'logserver-container'
     @Override
     public ContainerServiceType myServiceType() {
-        return ContainerServiceType.CONTAINER;
+        return useSeparateServiceTypeForLogserverContainer ? ContainerServiceType.LOGSERVER_CONTAINER : ContainerServiceType.CONTAINER;
     }
 
     private void addLogHandler(ContainerCluster cluster) {
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 6250131d292..1abb9e089da 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
@@ -109,7 +109,7 @@ public class DomAdminV4Builder extends DomAdminBuilderBase {
         ContainerModel logserverClusterModel = new ContainerModel(context.withParent(admin).withId(logServerCluster.getSubId()));
         logserverClusterModel.setCluster(logServerCluster);
 
-        Container container = new LogserverContainer(logServerCluster);
+        Container container = new LogserverContainer(logServerCluster, deployState.getProperties().useSeparateServiceTypeForLogserverContainer());
         container.setHostResource(hostResource);
         container.initService(deployState.getDeployLogger());
         logServerCluster.addContainer(container);
diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
index 588a722d476..7e570b25fa2 100644
--- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
@@ -4,6 +4,7 @@ package com.yahoo.config.model.provision;
 import com.yahoo.cloud.config.log.LogdConfig;
 import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.model.api.HostInfo;
+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.provision.ClusterMembership;
@@ -952,7 +953,27 @@ public class ModelProvisioningTest {
                         "  </container>" +
                         "</services>";
         boolean useDedicatedNodeForLogserver = false;
-        testContainerOnLogserverHost(services, useDedicatedNodeForLogserver);
+        boolean useSeparateServiceTypeForLogserverContainer = false;
+        testContainerOnLogserverHost(services, useDedicatedNodeForLogserver, useSeparateServiceTypeForLogserverContainer);
+    }
+
+    @Test
+    public void testLogserverContainerWhenDedicatedLogserverSeparateServiceType() {
+        String services =
+                "<?xml version='1.0' encoding='utf-8' ?>\n" +
+                        "<services>" +
+                        "  <admin version='4.0'>" +
+                        "    <logservers>" +
+                        "      <nodes count='1' dedicated='true'/>" +
+                        "    </logservers>" +
+                        "  </admin>" +
+                        "  <container version='1.0' id='foo'>" +
+                        "     <nodes count='1'/>" +
+                        "  </container>" +
+                        "</services>";
+        boolean useDedicatedNodeForLogserver = false;
+        boolean useSeparateServiceTypeForLogserverContainer = true;
+        testContainerOnLogserverHost(services, useDedicatedNodeForLogserver, useSeparateServiceTypeForLogserverContainer);
     }
 
     @Test
@@ -965,7 +986,8 @@ public class ModelProvisioningTest {
                         "  </container>" +
                         "</services>";
         boolean useDedicatedNodeForLogserver = true;
-        testContainerOnLogserverHost(services, useDedicatedNodeForLogserver);
+        boolean useSeparateServiceTypeForLogserverContainer = false;
+        testContainerOnLogserverHost(services, useDedicatedNodeForLogserver, useSeparateServiceTypeForLogserverContainer);
     }
 
     @Test
@@ -1808,10 +1830,11 @@ public class ModelProvisioningTest {
 
     // Tests that a container is allocated on logserver host and that
     // it is able to get config
-    private void testContainerOnLogserverHost(String services, boolean useDedicatedNodeForLogserver) {
+    private void testContainerOnLogserverHost(String services, boolean useDedicatedNodeForLogserver, boolean useSeparateServiceTypeForLogserverContainer) {
         int numberOfHosts = 2;
         VespaModelTester tester = new VespaModelTester();
         tester.useDedicatedNodeForLogserver(useDedicatedNodeForLogserver);
+        tester.useSeparateServiceTypeForLogserverContainer(useSeparateServiceTypeForLogserverContainer);
         tester.addHosts(numberOfHosts);
 
         VespaModel model = tester.createModel(Zone.defaultZone(), services, true);
@@ -1821,10 +1844,13 @@ public class ModelProvisioningTest {
         Logserver logserver = admin.getLogserver();
         HostResource hostResource = logserver.getHostResource();
         assertNotNull(hostResource.getService("logserver"));
-        assertNotNull(hostResource.getService("container"));
+        String containerServiceType = useSeparateServiceTypeForLogserverContainer
+                ? ContainerServiceType.LOGSERVER_CONTAINER.serviceName
+                : ContainerServiceType.CONTAINER.serviceName;
+        assertNotNull(hostResource.getService(containerServiceType));
 
         // Test that the container gets config
-        String configId = admin.getLogserver().getHostResource().getService("container").getConfigId();
+        String configId = admin.getLogserver().getHostResource().getService(containerServiceType).getConfigId();
         ApplicationMetadataConfig.Builder builder = new ApplicationMetadataConfig.Builder();
         model.getConfig(builder, configId);
         ApplicationMetadataConfig cfg = new ApplicationMetadataConfig(builder);
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
index 672e2932ab6..0922ede86b6 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
@@ -47,6 +47,7 @@ public class VespaModelTester {
     private Map<String, Collection<Host>> hostsByFlavor = new HashMap<>();
     private ApplicationId applicationId = ApplicationId.defaultId();
     private boolean useDedicatedNodeForLogserver = false;
+    private boolean useSeparateServiceTypeForLogserverContainer = false;
 
     public VespaModelTester() {
         this(new NullConfigModelRegistry());
@@ -97,6 +98,10 @@ public class VespaModelTester {
         this.useDedicatedNodeForLogserver = useDedicatedNodeForLogserver;
     }
 
+    public void useSeparateServiceTypeForLogserverContainer(boolean useSeparateServiceTypeForLogserverContainer) {
+        this.useSeparateServiceTypeForLogserverContainer = useSeparateServiceTypeForLogserverContainer;
+    }
+
     /** Creates a model which uses 0 as start index and fails on out of capacity */
     public VespaModel createModel(String services, String ... retiredHostNames) {
         return createModel(Zone.defaultZone(), services, true, retiredHostNames);
@@ -137,7 +142,8 @@ public class VespaModelTester {
                 .setMultitenant(true)
                 .setHostedVespa(hosted)
                 .setApplicationId(applicationId)
-                .setUseDedicatedNodeForLogserver(useDedicatedNodeForLogserver);
+                .setUseDedicatedNodeForLogserver(useDedicatedNodeForLogserver)
+                .setUseSeparateServiceTypeForLogserverContainer(useSeparateServiceTypeForLogserverContainer);
 
         DeployState deployState = new DeployState.Builder()
                 .applicationPackage(appPkg)
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
index 870a91f8b45..c04a00b2470 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
@@ -131,6 +131,7 @@ public class ModelContextImpl implements ModelContext {
         private final boolean useDedicatedNodeForLogserver;
         private final boolean useFdispatchByDefault;
         private final boolean useAdaptiveDispatch;
+        private final boolean useSeparateServiceTypeForLogserverContainer;
 
         public Properties(ApplicationId applicationId,
                           boolean multitenantFromConfig,
@@ -161,6 +162,8 @@ public class ModelContextImpl implements ModelContext {
                     .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value();
             this.useAdaptiveDispatch = Flags.USE_ADAPTIVE_DISPATCH.bindTo(flagSource)
                     .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value();
+            this.useSeparateServiceTypeForLogserverContainer = Flags.USE_SEPARATE_SERVICE_TYPE_FOR_LOGSERVER_CONTAINER.bindTo(flagSource)
+                    .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value();
         }
 
         @Override
@@ -205,8 +208,12 @@ public class ModelContextImpl implements ModelContext {
 
         @Override
         public boolean useFdispatchByDefault() { return useFdispatchByDefault; }
+
         @Override
         public boolean useAdaptiveDispatch() { return useAdaptiveDispatch; }
+
+        @Override
+        public boolean useSeparateServiceTypeForLogserverContainer() { return useSeparateServiceTypeForLogserverContainer; }
     }
 
 }
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
index 596b42f6e39..011f541fafd 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -112,6 +112,12 @@ public class Flags {
             "Takes effect on next node agent tick (but does not clear existing failure reports)",
             HOSTNAME);
 
+    public static final UnboundBooleanFlag USE_SEPARATE_SERVICE_TYPE_FOR_LOGSERVER_CONTAINER = defineFeatureFlag(
+            "use-separate-service-type-for-logserver-container", true,
+            "Use separate service type for Logserver container, resulting in logserver container not being an application endpoint",
+            "Takes effect at redeployment",
+            APPLICATION_ID);
+
     /** WARNING: public for testing: All flags should be defined in {@link Flags}. */
     public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, String description,
                                                        String modificationEffect, FetchVector.Dimension... dimensions) {