Merge pull request #15063 from vespa-engine/ogronnedby/dont-trust-max-resource-use

Compare .max() and .current() when calculating use

4 files changed, 201 insertions, 7 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index a6b3d2e43c1..a97bf55e17d 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -6,6 +6,7 @@ import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.application.api.ValidationId;
 import com.yahoo.config.application.api.ValidationOverrides;
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.ClusterResources;
 import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.InstanceName;
@@ -368,12 +369,8 @@ public class ApplicationController {
     }
 
     private QuotaUsage deploymentQuotaUsage(ZoneId zoneId, ApplicationId applicationId) {
-        var quotaUsage = configServer.nodeRepository().getApplication(zoneId, applicationId)
-                .clusters().values().stream()
-                .map(Cluster::max)
-                .mapToDouble(max -> max.nodes() * max.nodeResources().cost())
-                .sum();
-        return QuotaUsage.create(quotaUsage);
+        var application = configServer.nodeRepository().getApplication(zoneId, applicationId);
+        return DeploymentQuotaCalculator.calculateQuotaUsage(application);
     }
 
     private ApplicationPackage getApplicationPackage(ApplicationId application, ZoneId zone, ApplicationVersion revision) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentQuotaCalculator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentQuotaCalculator.java
index 82d83ea3585..4bef2ef7648 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentQuotaCalculator.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentQuotaCalculator.java
@@ -1,7 +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.hosted.controller.application;
 
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.ClusterResources;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.api.integration.billing.Quota;
@@ -10,7 +12,12 @@ import java.math.BigDecimal;
 import java.math.RoundingMode;
 import java.util.List;
 
-/** Calculates the quota to allocate to a deployment. */
+/**
+ * Calculates the quota to allocate to a deployment.
+ *
+ * @author ogronnesby
+ * @author andreer
+ */
 public class DeploymentQuotaCalculator {
 
     public static Quota calculate(Quota tenantQuota,
@@ -25,6 +32,23 @@ public class DeploymentQuotaCalculator {
         return getMaximumAllowedQuota(tenantQuota, tenantApps, deployingApp, deployingZone);
     }
 
+    public static QuotaUsage calculateQuotaUsage(com.yahoo.vespa.hosted.controller.api.integration.configserver.Application application) {
+        // the .max() resources are only specified when the user has specified a max.  to make sure we enforce quotas
+        // correctly we retrieve the maximum of .current() and .max() - otherwise we would keep adding 0s for those
+        // that are not using autoscaling.
+        var quotaUsageRate = application.clusters().values().stream()
+                .map(cluster -> largestQuotaUsage(cluster.current(), cluster.max()))
+                .mapToDouble(resources -> resources.nodes() * resources.nodeResources().cost())
+                .sum();
+        return QuotaUsage.create(quotaUsageRate);
+    }
+
+    private static ClusterResources largestQuotaUsage(ClusterResources a, ClusterResources b) {
+        var usageA = a.nodes() * a.nodeResources().cost();
+        var usageB = b.nodes() * b.nodeResources().cost();
+        return usageA < usageB ? b : a;
+    }
+
     /** Just get the maximum quota we are allowed to use. */
     private static Quota getMaximumAllowedQuota(Quota tenantQuota, List<Application> applications,
                                                 ApplicationId application, ZoneId zone) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/DeploymentQuotaCalculatorTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/DeploymentQuotaCalculatorTest.java
index 675cb2f3f76..fd719ab4619 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/DeploymentQuotaCalculatorTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/DeploymentQuotaCalculatorTest.java
@@ -1,11 +1,20 @@
 package com.yahoo.vespa.hosted.controller.application;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.zone.ZoneId;
+import com.yahoo.io.IOUtils;
 import com.yahoo.vespa.hosted.controller.api.integration.billing.Quota;
+import com.yahoo.vespa.hosted.controller.api.integration.noderepository.ApplicationData;
 import org.junit.Test;
 
+import java.io.File;
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.List;
 
 import static org.junit.Assert.*;
@@ -48,4 +57,12 @@ public class DeploymentQuotaCalculatorTest {
         assertEquals(calculated.budget().get().doubleValue(), 0, 1e-5);
     }
 
+    @Test
+    public void using_highest_resource_use() throws IOException, URISyntaxException {
+        var content = new String(Files.readAllBytes(Paths.get("src/test/java/com/yahoo/vespa/hosted/controller/application/response/application.json")));
+        var mapper = new ObjectMapper();
+        var application = mapper.readValue(content, ApplicationData.class).toApplication();
+        var usage = DeploymentQuotaCalculator.calculateQuotaUsage(application);
+        assertEquals(1.164, usage.rate(), 0.001);
+    }
 }
\ No newline at end of file
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/response/application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/response/application.json
new file mode 100644
index 00000000000..ccfb6af1635
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/response/application.json
@@ -0,0 +1,156 @@
+{
+  "url": "...",
+  "id": "vespa.album-recommendation.default",
+  "clusters": {
+    "default": {
+      "min": {
+        "nodes": 2,
+        "groups": 1,
+        "resources": {
+          "vcpu": 0.0,
+          "memoryGb": 0.0,
+          "diskGb": 0.0,
+          "bandwidthGbps": 0.0,
+          "diskSpeed": "fast",
+          "storageType": "any"
+        }
+      },
+      "max": {
+        "nodes": 2,
+        "groups": 1,
+        "resources": {
+          "vcpu": 0.0,
+          "memoryGb": 0.0,
+          "diskGb": 0.0,
+          "bandwidthGbps": 0.0,
+          "diskSpeed": "fast",
+          "storageType": "any"
+        }
+      },
+      "current": {
+        "nodes": 2,
+        "groups": 1,
+        "resources": {
+          "vcpu": 2.0,
+          "memoryGb": 8.0,
+          "diskGb": 50.0,
+          "bandwidthGbps": 0.3,
+          "diskSpeed": "fast",
+          "storageType": "remote"
+        }
+      },
+      "suggested": {
+        "nodes": 2,
+        "groups": 1,
+        "resources": {
+          "vcpu": 2.0,
+          "memoryGb": 8.0,
+          "diskGb": 75.0,
+          "bandwidthGbps": 0.3,
+          "diskSpeed": "fast",
+          "storageType": "local"
+        }
+      }
+    },
+    "logserver": {
+      "min": {
+        "nodes": 1,
+        "groups": 1,
+        "resources": {
+          "vcpu": 0.0,
+          "memoryGb": 0.0,
+          "diskGb": 0.0,
+          "bandwidthGbps": 0.0,
+          "diskSpeed": "fast",
+          "storageType": "any"
+        }
+      },
+      "max": {
+        "nodes": 1,
+        "groups": 1,
+        "resources": {
+          "vcpu": 0.0,
+          "memoryGb": 0.0,
+          "diskGb": 0.0,
+          "bandwidthGbps": 0.0,
+          "diskSpeed": "fast",
+          "storageType": "any"
+        }
+      },
+      "current": {
+        "nodes": 1,
+        "groups": 1,
+        "resources": {
+          "vcpu": 0.5,
+          "memoryGb": 4.0,
+          "diskGb": 50.0,
+          "bandwidthGbps": 0.3,
+          "diskSpeed": "fast",
+          "storageType": "remote"
+        }
+      },
+      "suggested": {
+        "nodes": 2,
+        "groups": 1,
+        "resources": {
+          "vcpu": 2.0,
+          "memoryGb": 4.0,
+          "diskGb": 50.0,
+          "bandwidthGbps": 0.3,
+          "diskSpeed": "fast",
+          "storageType": "local"
+        }
+      }
+    },
+    "music": {
+      "min": {
+        "nodes": 2,
+        "groups": 1,
+        "resources": {
+          "vcpu": 0.0,
+          "memoryGb": 0.0,
+          "diskGb": 0.0,
+          "bandwidthGbps": 0.0,
+          "diskSpeed": "fast",
+          "storageType": "any"
+        }
+      },
+      "max": {
+        "nodes": 2,
+        "groups": 1,
+        "resources": {
+          "vcpu": 0.0,
+          "memoryGb": 0.0,
+          "diskGb": 0.0,
+          "bandwidthGbps": 0.0,
+          "diskSpeed": "fast",
+          "storageType": "any"
+        }
+      },
+      "current": {
+        "nodes": 2,
+        "groups": 1,
+        "resources": {
+          "vcpu": 2.0,
+          "memoryGb": 8.0,
+          "diskGb": 50.0,
+          "bandwidthGbps": 0.3,
+          "diskSpeed": "fast",
+          "storageType": "remote"
+        }
+      },
+      "suggested": {
+        "nodes": 2,
+        "groups": 1,
+        "resources": {
+          "vcpu": 2.0,
+          "memoryGb": 4.0,
+          "diskGb": 50.0,
+          "bandwidthGbps": 0.3,
+          "diskSpeed": "fast",
+          "storageType": "local"
+        }
+      }
+    }
+  }
+}
\ No newline at end of file