Merge pull request #17742 from vespa-engine/arnej/no-splunk-on-admin-nodes

skip adding logforwarder on hosts if they have membership with cluste…

5 files changed, 103 insertions, 5 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 2b850d8f3eb..d1ba5d633a8 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
@@ -9,6 +9,7 @@ import com.yahoo.config.model.ConfigModelContext.ApplicationType;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.model.AbstractService;
 import com.yahoo.vespa.model.ConfigProxy;
@@ -60,11 +61,13 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable
     private Logserver logserver;
 
     private LogForwarder.Config logForwarderConfig = null;
+    private boolean logForwarderIncludeAdmin = false;
 
     private ApplicationType applicationType = ApplicationType.DEFAULT;
 
-    public void setLogForwarderConfig(LogForwarder.Config cfg) {
+    public void setLogForwarderConfig(LogForwarder.Config cfg, boolean includeAdmin) {
         this.logForwarderConfig = cfg;
+        this.logForwarderIncludeAdmin = includeAdmin;
     }
 
     /**
@@ -236,7 +239,18 @@ public class Admin extends AbstractConfigProducer<Admin> implements Serializable
         addConfigProxy(deployState.getDeployLogger(), host);
         addFileDistribution(host);
         if (logForwarderConfig != null) {
-            addLogForwarder(deployState.getDeployLogger(), host);
+            boolean actuallyAdd = true;
+            var membership = host.spec().membership();
+            if (membership.isPresent()) {
+                var clustertype = membership.get().cluster().type();
+                // XXX should skip only if this.isHostedVespa is true?
+                if (clustertype == ClusterSpec.Type.admin) {
+                    actuallyAdd = logForwarderIncludeAdmin;
+                }
+            }
+            if (actuallyAdd) {
+                addLogForwarder(deployState.getDeployLogger(), host);
+            }
         }
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java
index c5edfb9bbf7..963d2dde7fc 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java
@@ -114,14 +114,14 @@ public abstract class DomAdminBuilderBase extends VespaDomBuilder.DomConfigProdu
 
     void addLogForwarders(ModelElement logForwardingElement, Admin admin) {
         if (logForwardingElement == null) return;
-
+        boolean alsoForAdminCluster = logForwardingElement.booleanAttribute("include-admin");
         for (ModelElement e : logForwardingElement.children("splunk")) {
             LogForwarder.Config cfg = LogForwarder.cfg()
 		    .withSplunkHome(e.stringAttribute("splunk-home"))
 		    .withDeploymentServer(e.stringAttribute("deployment-server"))
 		    .withClientName(e.stringAttribute("client-name"))
             .withPhoneHomeInterval(e.integerAttribute("phone-home-interval"));
-            admin.setLogForwarderConfig(cfg);
+            admin.setLogForwarderConfig(cfg, alsoForAdminCluster);
         }
     }
 
diff --git a/config-model/src/main/resources/schema/admin.rnc b/config-model/src/main/resources/schema/admin.rnc
index 784fb82d319..a75b51a567a 100644
--- a/config-model/src/main/resources/schema/admin.rnc
+++ b/config-model/src/main/resources/schema/admin.rnc
@@ -105,6 +105,7 @@ ClusterControllers = element cluster-controllers {
 }
 
 LogForwarding = element logforwarding {
+    attribute include-admin { xsd:boolean }? &
     element splunk {
         attribute splunk-home { xsd:string }? &
         attribute deployment-server { xsd:string } &
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 499ec94f8ee..326bf577acc 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
@@ -863,6 +863,89 @@ public class ModelProvisioningTest {
     }
 
     @Test
+    public void testLogForwarderNotInAdminCluster() {
+        String services =
+                "<?xml version='1.0' encoding='utf-8' ?>\n" +
+                        "<services>" +
+                        "  <admin version='4.0'>" +
+                        "    <logservers>" +
+                        "      <nodes count='1' dedicated='true'/>" +
+                        "    </logservers>" +
+                        "    <logforwarding>" +
+                        "      <splunk deployment-server='bardeplserv:123' client-name='barclinam' phone-home-interval='987' />" +
+                        "    </logforwarding>" +
+                        "  </admin>" +
+                        "  <container version='1.0' id='foo'>" +
+                        "     <nodes count='1'/>" +
+                        "  </container>" +
+                        "</services>";
+
+        int numberOfHosts = 2;
+        VespaModelTester tester = new VespaModelTester();
+        tester.addHosts(numberOfHosts+1);
+
+        VespaModel model = tester.createModel(Zone.defaultZone(), services, true);
+        assertThat(model.getRoot().hostSystem().getHosts().size(), is(numberOfHosts));
+
+        Admin admin = model.getAdmin();
+        Logserver logserver = admin.getLogserver();
+        HostResource hostResource = logserver.getHostResource();
+
+        assertNotNull(hostResource.getService("logserver"));
+        assertNull(hostResource.getService("container"));
+        assertNull(hostResource.getService("logforwarder"));
+
+        var clist = model.getContainerClusters().get("foo").getContainers();
+        assertThat(clist.size(), is(1));
+        hostResource = clist.get(0).getHostResource();
+        assertNull(hostResource.getService("logserver"));
+        assertNotNull(hostResource.getService("container"));
+        assertNotNull(hostResource.getService("logforwarder"));
+    }
+
+
+    @Test
+    public void testLogForwarderInAdminCluster() {
+        String services =
+                "<?xml version='1.0' encoding='utf-8' ?>\n" +
+                        "<services>" +
+                        "  <admin version='4.0'>" +
+                        "    <logservers>" +
+                        "      <nodes count='1' dedicated='true'/>" +
+                        "    </logservers>" +
+                        "    <logforwarding include-admin='true'>" +
+                        "      <splunk deployment-server='bardeplserv:123' client-name='barclinam' phone-home-interval='987' />" +
+                        "    </logforwarding>" +
+                        "  </admin>" +
+                        "  <container version='1.0' id='foo'>" +
+                        "     <nodes count='1'/>" +
+                        "  </container>" +
+                        "</services>";
+
+        int numberOfHosts = 2;
+        VespaModelTester tester = new VespaModelTester();
+        tester.addHosts(numberOfHosts+1);
+
+        VespaModel model = tester.createModel(Zone.defaultZone(), services, true);
+        assertThat(model.getRoot().hostSystem().getHosts().size(), is(numberOfHosts));
+
+        Admin admin = model.getAdmin();
+        Logserver logserver = admin.getLogserver();
+        HostResource hostResource = logserver.getHostResource();
+
+        assertNotNull(hostResource.getService("logserver"));
+        assertNull(hostResource.getService("container"));
+        assertNotNull(hostResource.getService("logforwarder"));
+
+        var clist = model.getContainerClusters().get("foo").getContainers();
+        assertThat(clist.size(), is(1));
+        hostResource = clist.get(0).getHostResource();
+        assertNull(hostResource.getService("logserver"));
+        assertNotNull(hostResource.getService("container"));
+        assertNotNull(hostResource.getService("logforwarder"));
+    }
+
+    @Test
     public void testImplicitLogserverContainer() {
         String services =
                 "<?xml version='1.0' encoding='utf-8' ?>\n" +
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 f1f794c5057..60672c7df07 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
@@ -147,7 +147,7 @@ public class DedicatedAdminV4Test {
                 "  <admin version='4.0'>" +
                 "    <slobroks><nodes count='2' dedicated='true'/></slobroks>" +
                 "    <logservers><nodes count='1' dedicated='true'/></logservers>" +
-                "    <logforwarding>" +
+                "    <logforwarding include-admin='true'>" +
                 "      <splunk deployment-server='foo:123' client-name='foocli' phone-home-interval='900'/>" +
                 "    </logforwarding>" +
                 "  </admin>" +