Merge pull request #13756 from vespa-engine/olaa/propagate-error-message

Propagate error message when setting plan fails

4 files changed, 43 insertions, 21 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingController.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingController.java
index bd9568fe891..24864c03530 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingController.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/BillingController.java
@@ -16,9 +16,9 @@ public interface BillingController {
     PlanId getPlan(TenantName tenant);
 
     /**
-     * Returns true if plan was changed
+     * @return String containing error message if something went wrong. Empty otherwise
      */
-    boolean setPlan(TenantName tenant, PlanId planId, boolean hasApplications);
+    PlanResult setPlan(TenantName tenant, PlanId planId, boolean hasApplications);
 
     Invoice.Id createInvoiceForPeriod(TenantName tenant, ZonedDateTime startTime, ZonedDateTime endTime, String agent);
 
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/MockBillingController.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/MockBillingController.java
index a4c25e301ba..4f367d6498e 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/MockBillingController.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/MockBillingController.java
@@ -30,9 +30,9 @@ public class MockBillingController implements BillingController {
     }
 
     @Override
-    public boolean setPlan(TenantName tenant, PlanId planId, boolean hasApplications) {
+    public PlanResult setPlan(TenantName tenant, PlanId planId, boolean hasApplications) {
         plans.put(tenant, planId);
-        return true;
+        return PlanResult.success();
     }
 
     @Override
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/PlanResult.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/PlanResult.java
new file mode 100644
index 00000000000..a4803ccde64
--- /dev/null
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/billing/PlanResult.java
@@ -0,0 +1,35 @@
+// 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.api.integration.billing;
+
+import java.util.Optional;
+
+/**
+ * Result of {@link BillingController#setPlan}
+ *
+ * @author olaa
+ */
+public class PlanResult {
+
+    private final Optional<String> errorMessage;
+
+    private PlanResult(Optional<String> errorMessage) {
+        this.errorMessage = errorMessage;
+    }
+
+    public static PlanResult success() {
+        return new PlanResult(Optional.empty());
+    }
+
+    public static PlanResult error(String errorMessage) {
+        return new PlanResult(Optional.of(errorMessage));
+    }
+
+    public boolean isSuccess() {
+        return errorMessage.isEmpty();
+    }
+
+    public Optional<String> getErrorMessage() {
+        return errorMessage;
+    }
+
+}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandler.java
index ccbee15d2c5..0e6f856b115 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/billing/BillingApiHandler.java
@@ -139,12 +139,12 @@ public class BillingApiHandler extends LoggingRequestHandler {
         var planId = PlanId.from(slime.field("plan").asString());
         var hasApplications = applicationController.asList(tenantName).size() > 0;
 
-        if (billingController.setPlan(tenantName, planId, hasApplications)) {
+        var result = billingController.setPlan(tenantName, planId, hasApplications);
+
+        if (result.isSuccess())
             return new StringResponse("Plan: " + planId.value());
-        } else {
-            return ErrorResponse.forbidden("Invalid plan change with active deployments");
-        }
 
+        return ErrorResponse.forbidden(result.getErrorMessage().orElse("Invalid plan change"));
     }
 
     private HttpResponse getBillingAllTenants(String until) {
@@ -374,19 +374,6 @@ public class BillingApiHandler extends LoggingRequestHandler {
         return inspector.field(field).asString();
     }
 
-    private DeploymentId getDeploymentIdOrNull(Inspector inspector) {
-        if (inspector.field("applicationId").valid() != inspector.field("zoneId").valid() ) {
-            throw new BadRequestException("Either both application id and zone id should be set, or neither.");
-        }
-        if (inspector.field("applicationId").valid()) {
-            return new DeploymentId(
-                    ApplicationId.fromSerializedForm(inspector.field("applicationId").asString()),
-                    com.yahoo.config.provision.zone.ZoneId.from(inspector.field("zoneId").asString())
-            );
-        }
-        return null;
-    }
-
     private LocalDate untilParameter(String until) {
         if (until == null || until.isEmpty() || until.isBlank())
             return LocalDate.now().plusDays(1);