From 68a155ce12ea8a7c9051d939b817e8ecab299a53 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Sun, 20 Nov 2022 23:21:47 +0100 Subject: Nicer deploy errors --- .../vespa/model/application/validation/QuotaValidator.java | 8 ++++---- .../vespa/model/application/validation/QuotaValidatorTest.java | 10 +++++----- .../yahoo/vespa/config/server/modelfactory/ModelsBuilder.java | 2 +- .../com/yahoo/vespa/config/server/session/SessionPreparer.java | 2 ++ .../com/yahoo/vespa/config/server/deploy/HostedDeployTest.java | 2 +- .../api/integration/configserver/ConfigServerException.java | 6 ++++++ .../controller/restapi/application/ApplicationApiTest.java | 2 +- 7 files changed, 20 insertions(+), 12 deletions(-) diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/QuotaValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/QuotaValidator.java index 7ea582b99e6..da470f804d9 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/QuotaValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/QuotaValidator.java @@ -86,19 +86,19 @@ public class QuotaValidator extends Validator { private static void throwIfBudgetNegative(double spend, BigDecimal budget, SystemName systemName) { if (budget.doubleValue() < 0) { - throw new IllegalArgumentException(quotaMessage("Please free up some capacity", systemName, spend, budget)); + throw new IllegalArgumentException(quotaMessage("Please free up some capacity.", systemName, spend, budget)); } } private static void throwIfBudgetExceeded(double spend, BigDecimal budget, SystemName systemName) { if (budget.doubleValue() < spend) { - throw new IllegalArgumentException(quotaMessage("Deployment exceeds its quota and has been blocked! Please contact support to update your plan", systemName, spend, budget)); + throw new IllegalArgumentException(quotaMessage("Contact support to upgrade your plan.", systemName, spend, budget)); } } private static String quotaMessage(String message, SystemName system, double spend, BigDecimal budget) { - String quotaDescription = String.format(Locale.ENGLISH, "Quota is $%.2f, but at least $%.2f is required", budget, spend); - return (system == SystemName.Public ? "" : system.value() + ": ") + message + ": " + quotaDescription; + String quotaDescription = String.format(Locale.ENGLISH, "The max resources specified cost $%.2f but your quota is $%.2f", spend, budget); + return (system == SystemName.Public ? "" : system.value() + ": ") + quotaDescription + ": " + message; } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/QuotaValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/QuotaValidatorTest.java index ef22f0b2770..1a7b3d62cb7 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/QuotaValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/QuotaValidatorTest.java @@ -47,7 +47,7 @@ public class QuotaValidatorTest { tester.deploy(null, getServices("testCluster", 10), Environment.prod, null); fail(); } catch (RuntimeException e) { - assertEquals("Deployment exceeds its quota and has been blocked! Please contact support to update your plan: Quota is $1.25, but at least $1.63 is required", e.getMessage()); + assertEquals("The max resources specified cost $1.63 but your quota is $1.25: Contact support to upgrade your plan.", e.getMessage()); } } @@ -58,7 +58,7 @@ public class QuotaValidatorTest { tester.deploy(null, getServices("testCluster", 10), Environment.prod, null); fail(); } catch (RuntimeException e) { - assertEquals("publiccd: Deployment exceeds its quota and has been blocked! Please contact support to update your plan: Quota is $1.00, but at least $1.63 is required", e.getMessage()); + assertEquals("publiccd: The max resources specified cost $1.63 but your quota is $1.00: Contact support to upgrade your plan.", e.getMessage()); } } @@ -69,7 +69,7 @@ public class QuotaValidatorTest { tester.deploy(null, getServices("testCluster", 10), Environment.prod, null); fail(); } catch (RuntimeException e) { - assertEquals("publiccd: Deployment exceeds its quota and has been blocked! Please contact support to update your plan: Quota is $1.25, but at least $1.63 is required", e.getMessage()); + assertEquals("publiccd: The max resources specified cost $1.63 but your quota is $1.25: Contact support to upgrade your plan.", e.getMessage()); } } @@ -82,8 +82,8 @@ public class QuotaValidatorTest { tester.deploy(null, getServices("testCluster", 10), Environment.prod, null); fail(); } catch (RuntimeException e) { - assertEquals("Please free up some capacity: Quota is $--.--, but at least $-.-- is required", - ValidationTester.censorNumbers(e.getMessage())); + assertEquals("The max resources specified cost $-.-- but your quota is $--.--: Please free up some capacity.", + ValidationTester.censorNumbers(e.getMessage())); } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java index 8a541abf4ae..d32d1a4e000 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java @@ -134,7 +134,7 @@ public abstract class ModelsBuilder { } else { if (e instanceof IllegalArgumentException) { - var wrapped = new InvalidApplicationException("Error loading " + applicationId, e); + var wrapped = new InvalidApplicationException("Invalid application", e); deployLogger.logApplicationPackage(Level.SEVERE, Exceptions.toMessageString(wrapped)); throw wrapped; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java index 8d023cac88a..4928af488e1 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java @@ -148,6 +148,8 @@ public class SessionPreparer { return preparation.result(); } catch (IllegalArgumentException e) { + if (e instanceof InvalidApplicationException) + throw e; throw new InvalidApplicationException("Invalid application package", e); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java index 95f44bf09a1..39b1a588a17 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java @@ -294,7 +294,7 @@ public class HostedDeployTest { DeployTester tester = createTester(hosts, modelFactories, prodZone); // Not OK when failing version is requested. - assertEquals("Invalid application package", + assertEquals("Invalid application", assertThrows(IllegalArgumentException.class, () -> tester.deployApp("src/test/apps/hosted/", wantedVersion.toFullString())) .getMessage()); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java index 47373413a0d..99b2968a43f 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java @@ -16,6 +16,12 @@ public class ConfigServerException extends RuntimeException { private final ErrorCode code; private final String message; + public ConfigServerException(ErrorCode code, String message) { + super(message); + this.code = code; + this.message = message; + } + public ConfigServerException(ErrorCode code, String message, String context) { super(context + ": " + message); this.code = code; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index d6da677cb27..ee1094ac32e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -1208,7 +1208,7 @@ public class ApplicationApiTest extends ControllerContainerTest { 400); ConfigServerMock configServer = tester.serviceRegistry().configServerMock(); - configServer.throwOnNextPrepare(new ConfigServerException(ConfigServerException.ErrorCode.INVALID_APPLICATION_PACKAGE, "Failed to prepare application", "Invalid application package")); + configServer.throwOnNextPrepare(new ConfigServerException(ConfigServerException.ErrorCode.INVALID_APPLICATION_PACKAGE, "Deployment failed", "Invalid application package")); // GET non-existent application package tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/package", GET).userIdentity(HOSTED_VESPA_OPERATOR), -- cgit v1.2.3