From 2cda575c128587d9668d867338cf73668337287b Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Wed, 5 May 2021 08:28:28 +0000 Subject: skip adding logforwarder on hosts if they have membership with cluster type admin --- .../main/java/com/yahoo/vespa/model/admin/Admin.java | 18 ++++++++++++++++-- .../model/builder/xml/dom/DomAdminBuilderBase.java | 4 ++-- config-model/src/main/resources/schema/admin.rnc | 1 + 3 files changed, 19 insertions(+), 4 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..269a638ce4e 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 implements Serializable private Logserver logserver; private LogForwarder.Config logForwarderConfig = null; + private boolean logForwarderUnconditional = false; private ApplicationType applicationType = ApplicationType.DEFAULT; - public void setLogForwarderConfig(LogForwarder.Config cfg) { + public void setLogForwarderConfig(LogForwarder.Config cfg, boolean unconditional) { this.logForwarderConfig = cfg; + this.logForwarderUnconditional = unconditional; } /** @@ -236,7 +239,18 @@ public class Admin extends AbstractConfigProducer 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 = logForwarderUnconditional; + } + } + 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..c4ec80c1b8a 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("unconditional"); 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..e584bb1c6a7 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 unconditional { xsd:boolean }? & element splunk { attribute splunk-home { xsd:string }? & attribute deployment-server { xsd:string } & -- cgit v1.2.3 From 78e01f1dd353b5fe9304a92a2949faa7ec048434 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Thu, 6 May 2021 07:43:13 +0000 Subject: test logforwarder not-in-admin --- .../model/provision/ModelProvisioningTest.java | 83 ++++++++++++++++++++++ 1 file changed, 83 insertions(+) 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 10019b00f61..1df4631a860 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 { testContainerOnLogserverHost(services, useDedicatedNodeForLogserver); } + @Test + public void testLogForwarderNotInAdminCluster() { + String services = + "\n" + + "" + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + ""; + + 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 = + "\n" + + "" + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + ""; + + 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 = -- cgit v1.2.3 From b6302ad4c546627b2a2391c7b98e9861836af9e3 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Thu, 6 May 2021 07:48:02 +0000 Subject: unconditional -> include-admin --- config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java | 4 ++-- .../com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java | 2 +- config-model/src/main/resources/schema/admin.rnc | 2 +- .../java/com/yahoo/config/model/provision/ModelProvisioningTest.java | 2 +- 4 files changed, 5 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 269a638ce4e..c3345b49c2c 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 @@ -65,9 +65,9 @@ public class Admin extends AbstractConfigProducer implements Serializable private ApplicationType applicationType = ApplicationType.DEFAULT; - public void setLogForwarderConfig(LogForwarder.Config cfg, boolean unconditional) { + public void setLogForwarderConfig(LogForwarder.Config cfg, boolean includeAdmin) { this.logForwarderConfig = cfg; - this.logForwarderUnconditional = unconditional; + this.logForwarderUnconditional = includeAdmin; } /** 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 c4ec80c1b8a..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,7 +114,7 @@ public abstract class DomAdminBuilderBase extends VespaDomBuilder.DomConfigProdu void addLogForwarders(ModelElement logForwardingElement, Admin admin) { if (logForwardingElement == null) return; - boolean alsoForAdminCluster = logForwardingElement.booleanAttribute("unconditional"); + boolean alsoForAdminCluster = logForwardingElement.booleanAttribute("include-admin"); for (ModelElement e : logForwardingElement.children("splunk")) { LogForwarder.Config cfg = LogForwarder.cfg() .withSplunkHome(e.stringAttribute("splunk-home")) diff --git a/config-model/src/main/resources/schema/admin.rnc b/config-model/src/main/resources/schema/admin.rnc index e584bb1c6a7..a75b51a567a 100644 --- a/config-model/src/main/resources/schema/admin.rnc +++ b/config-model/src/main/resources/schema/admin.rnc @@ -105,7 +105,7 @@ ClusterControllers = element cluster-controllers { } LogForwarding = element logforwarding { - attribute unconditional { xsd:boolean }? & + 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 1df4631a860..ecb63740fca 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 @@ -914,7 +914,7 @@ public class ModelProvisioningTest { " " + " " + " " + - " " + + " " + " " + " " + " " + -- cgit v1.2.3 From 1dc00e42469353508a61ded9f94ab7861bd4a275 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Thu, 6 May 2021 08:11:21 +0000 Subject: expects logforwarding in admin cluster, so ask for it --- .../src/test/java/com/yahoo/vespa/model/admin/DedicatedAdminV4Test.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 { " " + " " + " " + - " " + + " " + " " + " " + " " + -- cgit v1.2.3 From 087672628c627a793e828963192fe3aa700a6cf5 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Thu, 6 May 2021 17:23:34 +0000 Subject: cosmetic change to private variable name --- config-model/src/main/java/com/yahoo/vespa/model/admin/Admin.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 c3345b49c2c..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 @@ -61,13 +61,13 @@ public class Admin extends AbstractConfigProducer implements Serializable private Logserver logserver; private LogForwarder.Config logForwarderConfig = null; - private boolean logForwarderUnconditional = false; + private boolean logForwarderIncludeAdmin = false; private ApplicationType applicationType = ApplicationType.DEFAULT; public void setLogForwarderConfig(LogForwarder.Config cfg, boolean includeAdmin) { this.logForwarderConfig = cfg; - this.logForwarderUnconditional = includeAdmin; + this.logForwarderIncludeAdmin = includeAdmin; } /** @@ -245,7 +245,7 @@ public class Admin extends AbstractConfigProducer implements Serializable var clustertype = membership.get().cluster().type(); // XXX should skip only if this.isHostedVespa is true? if (clustertype == ClusterSpec.Type.admin) { - actuallyAdd = logForwarderUnconditional; + actuallyAdd = logForwarderIncludeAdmin; } } if (actuallyAdd) { -- cgit v1.2.3