Use separate service type for logserver container by default

8 files changed, 12 insertions, 58 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 47160bdd4a2..8085d980c04 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,7 +55,8 @@ public interface ModelContext {
         boolean useDedicatedNodeForLogserver();
         boolean useFdispatchByDefault();
         boolean useAdaptiveDispatch();
-        boolean useSeparateServiceTypeForLogserverContainer();
+        // TODO: Remove when 7.33 is the oldest model in use
+        default boolean useSeparateServiceTypeForLogserverContainer() { return true; }
         boolean enableMetricsProxyContainer();
     }
 
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 89433cdf7c0..317351d5c3f 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
@@ -36,7 +36,6 @@ public class TestProperties implements ModelContext.Properties {
     private boolean useDedicatedNodeForLogserver = false;
     private boolean useFdispatchByDefault = true;
     private boolean useAdaptiveDispatch = false;
-    private boolean useSeparateServiceTypeForLogserverContainer = false;
     private boolean enableMetricsProxyContainer = false;
 
 
@@ -54,7 +53,6 @@ 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; }
     @Override public boolean enableMetricsProxyContainer() { return enableMetricsProxyContainer; }
 
     public TestProperties setApplicationId(ApplicationId applicationId) {
@@ -87,11 +85,6 @@ public class TestProperties implements ModelContext.Properties {
         return this;
     }
 
-    public TestProperties setUseSeparateServiceTypeForLogserverContainer(boolean useSeparateServiceTypeForLogserverContainer) {
-        this.useSeparateServiceTypeForLogserverContainer = useSeparateServiceTypeForLogserverContainer;
-        return this;
-    }
-
     public TestProperties setEnableMetricsProxyContainer(boolean enableMetricsProxyContainer) {
         this.enableMetricsProxyContainer = enableMetricsProxyContainer;
         return this;
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 3d8773dcc2a..09bc29b181a 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
@@ -5,6 +5,7 @@ import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.vespa.model.container.Container;
 import com.yahoo.vespa.model.container.ContainerCluster;
 import com.yahoo.config.model.api.container.ContainerServiceType;
+import com.yahoo.vespa.model.container.component.AccessLogComponent;
 import com.yahoo.vespa.model.container.component.Handler;
 
 /**
@@ -18,6 +19,7 @@ public class LogserverContainer extends Container {
     public LogserverContainer(AbstractConfigProducer parent, boolean useSeparateServiceTypeForLogserverContainer) {
         super(parent, "" + 0, 0);
         this.useSeparateServiceTypeForLogserverContainer = useSeparateServiceTypeForLogserverContainer;
+        addComponent(new AccessLogComponent(AccessLogComponent.AccessLogType.jsonAccessLog, ((LogserverContainerCluster) parent).getName(), true));
     }
 
     @Override
@@ -25,5 +27,4 @@ public class LogserverContainer extends Container {
         return useSeparateServiceTypeForLogserverContainer ? ContainerServiceType.LOGSERVER_CONTAINER : ContainerServiceType.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 f395ef680da..6c9b9fdc084 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
@@ -956,27 +956,7 @@ public class ModelProvisioningTest {
                         "  </container>" +
                         "</services>";
         boolean useDedicatedNodeForLogserver = false;
-        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);
+        testContainerOnLogserverHost(services, useDedicatedNodeForLogserver);
     }
 
     @Test
@@ -989,8 +969,7 @@ public class ModelProvisioningTest {
                         "  </container>" +
                         "</services>";
         boolean useDedicatedNodeForLogserver = true;
-        boolean useSeparateServiceTypeForLogserverContainer = false;
-        testContainerOnLogserverHost(services, useDedicatedNodeForLogserver, useSeparateServiceTypeForLogserverContainer);
+        testContainerOnLogserverHost(services, useDedicatedNodeForLogserver);
     }
 
     @Test
@@ -1833,11 +1812,10 @@ 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, boolean useSeparateServiceTypeForLogserverContainer) {
+    private void testContainerOnLogserverHost(String services, boolean useDedicatedNodeForLogserver) {
         int numberOfHosts = 2;
         VespaModelTester tester = new VespaModelTester();
         tester.useDedicatedNodeForLogserver(useDedicatedNodeForLogserver);
-        tester.useSeparateServiceTypeForLogserverContainer(useSeparateServiceTypeForLogserverContainer);
         tester.addHosts(numberOfHosts);
 
         VespaModel model = tester.createModel(Zone.defaultZone(), services, true);
@@ -1847,9 +1825,7 @@ public class ModelProvisioningTest {
         Logserver logserver = admin.getLogserver();
         HostResource hostResource = logserver.getHostResource();
         assertNotNull(hostResource.getService("logserver"));
-        String containerServiceType = useSeparateServiceTypeForLogserverContainer
-                ? ContainerServiceType.LOGSERVER_CONTAINER.serviceName
-                : ContainerServiceType.CONTAINER.serviceName;
+        String containerServiceType = ContainerServiceType.LOGSERVER_CONTAINER.serviceName;
         assertNotNull(hostResource.getService(containerServiceType));
 
         // Test that the container gets config
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 f004cc8ff2a..3896b6d799a 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
@@ -28,6 +28,7 @@ import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
 import static com.yahoo.config.model.api.container.ContainerServiceType.CONTAINER;
+import static com.yahoo.config.model.api.container.ContainerServiceType.LOGSERVER_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;
@@ -77,7 +78,7 @@ public class DedicatedAdminV4Test {
         assertHostContainsServices(model, "hosts/myhost0", "slobrok", "logd");
         assertHostContainsServices(model, "hosts/myhost1", "slobrok", "logd");
         // Note: A container is always added on logserver host
-        assertHostContainsServices(model, "hosts/myhost2", "logserver", "logd", CONTAINER.serviceName);
+        assertHostContainsServices(model, "hosts/myhost2", "logserver", "logd", LOGSERVER_CONTAINER.serviceName);
 
         Monitoring monitoring = model.getAdmin().getMonitoring();
         assertEquals("vespa.routing", monitoring.getClustername());
@@ -157,7 +158,7 @@ public class DedicatedAdminV4Test {
         assertHostContainsServices(model, "hosts/myhost0", "logd", "logforwarder", "slobrok");
         assertHostContainsServices(model, "hosts/myhost1", "logd", "logforwarder", "slobrok");
         // Note: A container is always added on logserver host
-        assertHostContainsServices(model, "hosts/myhost2", "logd", "logforwarder", "logserver", CONTAINER.serviceName);
+        assertHostContainsServices(model, "hosts/myhost2", "logd", "logforwarder", "logserver", LOGSERVER_CONTAINER.serviceName);
 
         Set<String> configIds = model.getConfigIds();
         // 1 logforwarder on each host
@@ -200,7 +201,7 @@ 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.serviceName);
+        assertHostContainsServices(model, "hosts/myhost0", "slobrok", "logd", "logserver", LOGSERVER_CONTAINER.serviceName);
     }
 
     private Set<String> serviceNames(VespaModel model, String hostname) {
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 00bc82bbe02..801e138f3c7 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,7 +47,6 @@ public class VespaModelTester {
     private Map<String, Collection<Host>> hostsByFlavor = new HashMap<>();
     private ApplicationId applicationId = ApplicationId.defaultId();
     private boolean useDedicatedNodeForLogserver = false;
-    private boolean useSeparateServiceTypeForLogserverContainer = false;
     private boolean enableMetricsProxyContainer = false;
 
     public VespaModelTester() {
@@ -99,10 +98,6 @@ public class VespaModelTester {
         this.useDedicatedNodeForLogserver = useDedicatedNodeForLogserver;
     }
 
-    public void useSeparateServiceTypeForLogserverContainer(boolean useSeparateServiceTypeForLogserverContainer) {
-        this.useSeparateServiceTypeForLogserverContainer = useSeparateServiceTypeForLogserverContainer;
-    }
-
     public void enableMetricsProxyContainer(boolean enableMetricsProxyContainer) {
         this.enableMetricsProxyContainer = enableMetricsProxyContainer;
     }
@@ -148,7 +143,6 @@ public class VespaModelTester {
                 .setHostedVespa(hosted)
                 .setApplicationId(applicationId)
                 .setUseDedicatedNodeForLogserver(useDedicatedNodeForLogserver)
-                .setUseSeparateServiceTypeForLogserverContainer(useSeparateServiceTypeForLogserverContainer)
                 .setEnableMetricsProxyContainer(enableMetricsProxyContainer);
 
         DeployState deployState = new DeployState.Builder()
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 780135c893e..e75726f5a19 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,7 +131,6 @@ public class ModelContextImpl implements ModelContext {
         private final boolean useDedicatedNodeForLogserver;
         private final boolean useFdispatchByDefault;
         private final boolean useAdaptiveDispatch;
-        private final boolean useSeparateServiceTypeForLogserverContainer;
         private final boolean enableMetricsProxyContainer;
 
         public Properties(ApplicationId applicationId,
@@ -163,8 +162,6 @@ 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();
             this.enableMetricsProxyContainer = Flags.ENABLE_METRICS_PROXY_CONTAINER.bindTo(flagSource)
                     .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value();
         }
@@ -216,9 +213,6 @@ public class ModelContextImpl implements ModelContext {
         public boolean useAdaptiveDispatch() { return useAdaptiveDispatch; }
 
         @Override
-        public boolean useSeparateServiceTypeForLogserverContainer() { return useSeparateServiceTypeForLogserverContainer; }
-
-        @Override
         public boolean enableMetricsProxyContainer() { return enableMetricsProxyContainer; }
     }
 
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 5fa8e39ac81..e294aee02e7 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -107,12 +107,6 @@ 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", false,
-            "Use separate service type for Logserver container, resulting in logserver container not being an application endpoint",
-            "Takes effect at redeployment",
-            APPLICATION_ID);
-
     public static final UnboundBooleanFlag ENABLE_METRICS_PROXY_CONTAINER = defineFeatureFlag(
             "enable-metrics-proxy-container", false,
             "Start a container for metrics-proxy on every vespa node",