From 1685cbb1951d2f65181a6e26acd78d22ae45f541 Mon Sep 17 00:00:00 2001
From: Harald Musum <musum@yahooinc.com>
Date: Mon, 20 Sep 2021 15:05:04 +0200
Subject: Remove code for only logging when using resource-limits in hosted

No apps are using this now, so we will always throw exception if
'resource-limits' is used in services.xml in hosted. Prepare for
removing feature flag.
---
 .../com/yahoo/config/model/api/ModelContext.java   |  2 +-
 .../vespa/model/content/ClusterResourceLimits.java | 13 ++--
 .../model/content/cluster/ContentCluster.java      |  2 -
 .../content/cluster/DomResourceLimitsBuilder.java  | 19 +-----
 .../model/content/ClusterResourceLimitsTest.java   | 70 +++-------------------
 .../model/content/FleetControllerClusterTest.java  |  3 -
 .../config/server/deploy/ModelContextImpl.java     |  4 +-
 7 files changed, 19 insertions(+), 94 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 9e417776c73..49e3bff50b7 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
@@ -96,7 +96,7 @@ public interface ModelContext {
         @ModelFeatureFlag(owners = {"tokle", "bjorncs"}, removeAfter = "7.450") default boolean enableCustomAclMapping() { return true; }
         @ModelFeatureFlag(owners = {"geirst", "vekterli"}) default int numDistributorStripes() { return 0; }
         @ModelFeatureFlag(owners = {"arnej"}) default boolean requireConnectivityCheck() { return true; }
-        @ModelFeatureFlag(owners = {"hmusum"}) default boolean throwIfResourceLimitsSpecified() { return true; }
+        @ModelFeatureFlag(owners = {"hmusum"}, removeAfter = "7.470") default boolean throwIfResourceLimitsSpecified() { return true; }
         @ModelFeatureFlag(owners = {"hmusum"}) default double resourceLimitDisk() { return 0.8; }
         @ModelFeatureFlag(owners = {"hmusum"}) default double resourceLimitMemory() { return 0.8; }
         @ModelFeatureFlag(owners = {"geirst", "vekterli"}) default double minNodeRatioPerGroup() { return 0.0; }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterResourceLimits.java b/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterResourceLimits.java
index dc5e6c9baee..dff47515957 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterResourceLimits.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterResourceLimits.java
@@ -1,7 +1,6 @@
 // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.content;
 
-import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.vespa.model.builder.xml.dom.ModelElement;
 import com.yahoo.vespa.model.content.cluster.DomResourceLimitsBuilder;
 
@@ -37,20 +36,18 @@ public class ClusterResourceLimits {
 
         private final boolean enableFeedBlockInDistributor;
         private final boolean hostedVespa;
-        private final boolean throwIfSpecified;
-        private final DeployLogger deployLogger;
         private final double resourceLimitDisk;
         private final double resourceLimitMemory;
 
         private ResourceLimits.Builder ctrlBuilder = new ResourceLimits.Builder();
         private ResourceLimits.Builder nodeBuilder = new ResourceLimits.Builder();
 
-        public Builder(boolean enableFeedBlockInDistributor, boolean hostedVespa, boolean throwIfSpecified,
-                       DeployLogger deployLogger, double resourceLimitDisk, double resourceLimitMemory) {
+        public Builder(boolean enableFeedBlockInDistributor,
+                       boolean hostedVespa,
+                       double resourceLimitDisk,
+                       double resourceLimitMemory) {
             this.enableFeedBlockInDistributor = enableFeedBlockInDistributor;
             this.hostedVespa = hostedVespa;
-            this.throwIfSpecified = throwIfSpecified;
-            this.deployLogger = deployLogger;
             this.resourceLimitDisk = resourceLimitDisk;
             this.resourceLimitMemory = resourceLimitMemory;
             verifyLimits(resourceLimitDisk, resourceLimitMemory);
@@ -67,7 +64,7 @@ public class ClusterResourceLimits {
         private ResourceLimits.Builder createBuilder(ModelElement element) {
             return element == null
                     ? new ResourceLimits.Builder()
-                    : DomResourceLimitsBuilder.createBuilder(element, hostedVespa, throwIfSpecified, deployLogger);
+                    : DomResourceLimitsBuilder.createBuilder(element, hostedVespa);
         }
 
         public void setClusterControllerBuilder(ResourceLimits.Builder builder) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
index 1f443da51db..a746dc36540 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
@@ -123,8 +123,6 @@ public class ContentCluster extends AbstractConfigProducer<AbstractConfigProduce
             boolean enableFeedBlockInDistributor = deployState.getProperties().featureFlags().enableFeedBlockInDistributor();
             var resourceLimits = new ClusterResourceLimits.Builder(enableFeedBlockInDistributor,
                                                                    stateIsHosted(deployState),
-                                                                   deployState.featureFlags().throwIfResourceLimitsSpecified(),
-                                                                   deployState.getDeployLogger(),
                                                                    deployState.featureFlags().resourceLimitDisk(),
                                                                    deployState.featureFlags().resourceLimitMemory())
                     .build(contentElement);
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomResourceLimitsBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomResourceLimitsBuilder.java
index 32b0f5b6477..cb417c2d559 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomResourceLimitsBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomResourceLimitsBuilder.java
@@ -1,12 +1,9 @@
 // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.content.cluster;
 
-import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.vespa.model.builder.xml.dom.ModelElement;
 import com.yahoo.vespa.model.content.ResourceLimits;
 
-import java.util.logging.Level;
-
 /**
  * Builder for feed block resource limits.
  *
@@ -14,23 +11,13 @@ import java.util.logging.Level;
  */
 public class DomResourceLimitsBuilder {
 
-    public static ResourceLimits.Builder createBuilder(ModelElement contentXml,
-                                                       boolean hostedVespa,
-                                                       boolean throwIfSpecified,
-                                                       DeployLogger deployLogger) {
+    public static ResourceLimits.Builder createBuilder(ModelElement contentXml, boolean hostedVespa) {
         ResourceLimits.Builder builder = new ResourceLimits.Builder();
         ModelElement resourceLimits = contentXml.child("resource-limits");
         if (resourceLimits == null) { return builder; }
 
-        if (hostedVespa) {
-            String message = "Element '" + resourceLimits + "' is not allowed to be set";
-            if (throwIfSpecified) throw new IllegalArgumentException(message);
-
-
-            deployLogger.logApplicationPackage(Level.WARNING, message);
-            // TODO: return (default values will then be used). Cannot be done now as an app needs current behavior
-            //return builder;
-        }
+        if (hostedVespa)
+            throw new IllegalArgumentException("Element '" + resourceLimits + "' is not allowed to be set");
 
         if (resourceLimits.child("disk") != null) {
             builder.setDiskLimit(resourceLimits.childAsDouble("disk"));
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterResourceLimitsTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterResourceLimitsTest.java
index c0b1a64bace..f7aed7099d4 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterResourceLimitsTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ClusterResourceLimitsTest.java
@@ -1,14 +1,10 @@
 // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.content;
 
-import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.model.api.ModelContext;
-import com.yahoo.config.model.application.provider.BaseDeployLogger;
 import com.yahoo.config.model.deploy.TestProperties;
-import com.yahoo.searchdefinition.derived.TestableDeployLogger;
 import com.yahoo.text.XML;
 import com.yahoo.vespa.model.builder.xml.dom.ModelElement;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -28,7 +24,6 @@ public class ClusterResourceLimitsTest {
     private static class Fixture {
         private final boolean enableFeedBlockInDistributor;
         private final boolean hostedVespa;
-        private final boolean throwIfSpecified;
         private final ResourceLimits.Builder ctrlBuilder = new ResourceLimits.Builder();
         private final ResourceLimits.Builder nodeBuilder = new ResourceLimits.Builder();
 
@@ -37,13 +32,12 @@ public class ClusterResourceLimitsTest {
         }
 
         public Fixture(boolean enableFeedBlockInDistributor) {
-            this(enableFeedBlockInDistributor, false, false);
+            this(enableFeedBlockInDistributor, false);
         }
 
-        public Fixture(boolean enableFeedBlockInDistributor, boolean hostedVespa, boolean throwIfSpecified) {
+        public Fixture(boolean enableFeedBlockInDistributor, boolean hostedVespa) {
             this.enableFeedBlockInDistributor = enableFeedBlockInDistributor;
             this.hostedVespa = hostedVespa;
-            this.throwIfSpecified = throwIfSpecified;
         }
 
         public Fixture ctrlDisk(double limit) {
@@ -66,8 +60,6 @@ public class ClusterResourceLimitsTest {
             ModelContext.FeatureFlags featureFlags = new TestProperties();
             var builder = new ClusterResourceLimits.Builder(enableFeedBlockInDistributor,
                                                             hostedVespa,
-                                                            throwIfSpecified,
-                                                            new BaseDeployLogger(),
                                                             featureFlags.resourceLimitDisk(),
                                                             featureFlags.resourceLimitMemory());
             builder.setClusterControllerBuilder(ctrlBuilder);
@@ -143,49 +135,19 @@ public class ClusterResourceLimitsTest {
                 new Fixture(true));
     }
 
-    @Test
-    @Ignore // TODO: Remove hosted_limits_are_used_if_app_is_allowed_to_set_limits and enable this when code is fixed to do so
-    public void hosted_log_when_resource_limits_are_specified() {
-        TestableDeployLogger logger = new TestableDeployLogger();
-
-        var limits = hostedBuildAndLogIfSpecified(logger);
-        assertEquals(1, logger.warnings.size());
-        assertEquals("Element 'resource-limits' is not allowed to be set", logger.warnings.get(0));
-
-        // Verify that default limits are used
-        assertLimits(0.8, 0.8, limits.getClusterControllerLimits());
-        assertLimits(0.9, 0.9, limits.getContentNodeLimits());
-    }
-
     @Test
     public void hosted_exception_is_thrown_when_resource_limits_are_specified() {
         expectedException.expect(IllegalArgumentException.class);
         expectedException.expectMessage(containsString("Element 'resource-limits' is not allowed to be set"));
-        hostedBuildAndThrowIfSpecified();
-    }
-
-    @Test
-    // TODO: Remove this and enable hosted_log_when_resource_limits_are_specified when code is fixed to do so
-    public void hosted_limits_are_used_if_app_is_allowed_to_set_limits() {
-        TestableDeployLogger logger = new TestableDeployLogger();
-
-        var limits = hostedBuildAndLogIfSpecified(logger);
-        assertEquals(1, logger.warnings.size());
-        assertEquals("Element 'resource-limits' is not allowed to be set", logger.warnings.get(0));
-
-        // Verify that limits in XML are used
-        assertLimits(0.8, 0.92, limits.getClusterControllerLimits());
-        assertLimits(0.9, 0.96, limits.getContentNodeLimits());
+        hostedBuild();
     }
 
     @Test
     public void hosted_limits_from_feature_flag_are_used() {
-        TestableDeployLogger logger = new TestableDeployLogger();
-
         TestProperties featureFlags = new TestProperties();
         featureFlags.setResourceLimitDisk(0.85);
         featureFlags.setResourceLimitMemory(0.90);
-        var limits = hostedBuild(false, logger, featureFlags, false);
+        var limits = hostedBuild(featureFlags, false);
 
         // Verify that limits from feature flags are used
         assertLimits(0.85, 0.90, limits.getClusterControllerLimits());
@@ -194,35 +156,23 @@ public class ClusterResourceLimitsTest {
 
     @Test
     public void exception_is_thrown_when_resource_limits_are_out_of_range() {
-        TestableDeployLogger logger = new TestableDeployLogger();
-
         TestProperties featureFlags = new TestProperties();
         featureFlags.setResourceLimitDisk(1.1);
         expectedException.expect(IllegalArgumentException.class);
         expectedException.expectMessage(containsString("Resource limit for disk is set to illegal value 1.1, but must be in the range [0.0, 1.0]"));
-        hostedBuild(false, logger, featureFlags, false);
+        hostedBuild(featureFlags, false);
 
         featureFlags = new TestProperties();
         featureFlags.setResourceLimitDisk(-0.1);
         expectedException.expectMessage(containsString("Resource limit for disk is set to illegal value -0.1, but must be in the range [0.0, 1.0]"));
-        hostedBuild(false, logger, featureFlags, false);
-    }
-
-    private void hostedBuildAndThrowIfSpecified() {
-        hostedBuild(true, new TestableDeployLogger(), new TestProperties(), true);
-    }
-
-    private ClusterResourceLimits hostedBuildAndLogIfSpecified(DeployLogger deployLogger) {
-        return hostedBuild(false, deployLogger);
+        hostedBuild(featureFlags, false);
     }
 
-    private ClusterResourceLimits hostedBuild(boolean throwIfSpecified, DeployLogger deployLogger) {
-        return hostedBuild(throwIfSpecified, deployLogger, new TestProperties(), true);
+    private ClusterResourceLimits hostedBuild() {
+        return hostedBuild(new TestProperties(), true);
     }
 
-    private ClusterResourceLimits hostedBuild(boolean throwIfSpecified,
-                                              DeployLogger deployLogger,
-                                              ModelContext.FeatureFlags featureFlags,
+    private ClusterResourceLimits hostedBuild(ModelContext.FeatureFlags featureFlags,
                                               boolean limitsInXml) {
         Document clusterXml = XML.getDocument("<cluster id=\"test\">" +
                                               "  <tuning>\n" +
@@ -237,8 +187,6 @@ public class ClusterResourceLimitsTest {
 
         ClusterResourceLimits.Builder builder = new ClusterResourceLimits.Builder(true,
                                                                                   true,
-                                                                                  throwIfSpecified,
-                                                                                  deployLogger,
                                                                                   featureFlags.resourceLimitDisk(),
                                                                                   featureFlags.resourceLimitMemory());
         return builder.build(new ModelElement((limitsInXml ? clusterXml : noLimitsXml).getDocumentElement()));
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java
index cc1b96dc588..e5b757e1514 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java
@@ -2,7 +2,6 @@
 package com.yahoo.vespa.model.content;
 
 import com.yahoo.config.model.api.ModelContext;
-import com.yahoo.config.model.application.provider.BaseDeployLogger;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.deploy.TestProperties;
 import com.yahoo.config.model.test.MockRoot;
@@ -28,8 +27,6 @@ public class FleetControllerClusterTest {
                                                    clusterElement,
                                                    new ClusterResourceLimits.Builder(enableFeedBlockInDistributor,
                                                                                      false,
-                                                                                     false,
-                                                                                     new BaseDeployLogger(),
                                                                                      featureFlags.resourceLimitDisk(),
                                                                                      featureFlags.resourceLimitMemory())
                                                            .build(clusterElement).getClusterControllerLimits())
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 8aebd42b53c..89987891c61 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
@@ -183,7 +183,6 @@ public class ModelContextImpl implements ModelContext {
         private final int maxConcurrentMergesPerContentNode;
         private final int maxMergeQueueSize;
         private final int largeRankExpressionLimit;
-        private final boolean throwIfResourceLimitsSpecified;
         private final double resourceLimitDisk;
         private final double resourceLimitMemory;
         private final double minNodeRatioPerGroup;
@@ -211,7 +210,6 @@ public class ModelContextImpl implements ModelContext {
             this.requireConnectivityCheck = flagValue(source, appId, Flags.REQUIRE_CONNECTIVITY_CHECK);
             this.maxConcurrentMergesPerContentNode = flagValue(source, appId, Flags.MAX_CONCURRENT_MERGES_PER_NODE);
             this.maxMergeQueueSize = flagValue(source, appId, Flags.MAX_MERGE_QUEUE_SIZE);
-            this.throwIfResourceLimitsSpecified = flagValue(source, appId, Flags.THROW_EXCEPTION_IF_RESOURCE_LIMITS_SPECIFIED);
             this.resourceLimitDisk = flagValue(source, appId, PermanentFlags.RESOURCE_LIMIT_DISK);
             this.resourceLimitMemory = flagValue(source, appId, PermanentFlags.RESOURCE_LIMIT_MEMORY);
             this.minNodeRatioPerGroup = flagValue(source, appId, Flags.MIN_NODE_RATIO_PER_GROUP);
@@ -241,7 +239,7 @@ public class ModelContextImpl implements ModelContext {
         @Override public boolean requireConnectivityCheck() { return requireConnectivityCheck; }
         @Override public int maxConcurrentMergesPerNode() { return maxConcurrentMergesPerContentNode; }
         @Override public int maxMergeQueueSize() { return maxMergeQueueSize; }
-        @Override public boolean throwIfResourceLimitsSpecified() { return throwIfResourceLimitsSpecified; }
+        @Override public boolean throwIfResourceLimitsSpecified() { return true; }
         @Override public double resourceLimitDisk() { return resourceLimitDisk; }
         @Override public double resourceLimitMemory() { return resourceLimitMemory; }
         @Override public double minNodeRatioPerGroup() { return minNodeRatioPerGroup; }
-- 
cgit v1.2.3


From cfb626cac0aae54649f2bc081d3cec641c208dfc Mon Sep 17 00:00:00 2001
From: Harald Musum <musum@yahooinc.com>
Date: Mon, 20 Sep 2021 15:17:31 +0200
Subject: Remove flag

---
 flags/src/main/java/com/yahoo/vespa/flags/Flags.java | 7 -------
 1 file changed, 7 deletions(-)

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 7c9ee425d8f..e59a85dd8ce 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -234,13 +234,6 @@ public class Flags {
             "Takes effect on next restart",
             ZONE_ID, APPLICATION_ID);
 
-    public static final UnboundBooleanFlag THROW_EXCEPTION_IF_RESOURCE_LIMITS_SPECIFIED = defineFeatureFlag(
-            "throw-exception-if-resource-limits-specified", true,
-            List.of("hmusum"), "2021-06-07", "2021-10-01",
-            "Whether to throw an exception in hosted Vespa if the application specifies resource limits in services.xml",
-            "Takes effect on next deployment through controller",
-            APPLICATION_ID);
-
     public static final UnboundListFlag<String> DEFER_APPLICATION_ENCRYPTION = defineListFlag(
             "defer-application-encryption", List.of(), String.class,
             List.of("mpolden", "hakonhall"), "2021-06-23", "2021-10-01",
-- 
cgit v1.2.3