diff options
author | Jon Bratseth <bratseth@gmail.com> | 2022-09-16 16:22:03 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-09-16 16:22:03 +0200 |
commit | cdcd2970ad451bbb944a6bc564f6b0e9a88168b8 (patch) | |
tree | 74882e66714fe9998b2a42daded0b73ce0caf5ae /controller-server | |
parent | c2a9a266f28545fe015f7d4e22afd4f9619e97db (diff) | |
parent | 2366525ce85985aeadeec358b3392dc1ba94ef6e (diff) |
Merge pull request #24086 from vespa-engine/bratseth/make-validation-messages-clearer
Make validation messages clearer given multiple instances
Diffstat (limited to 'controller-server')
4 files changed, 13 insertions, 11 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 92c3198175a..92ebc5b7177 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 @@ -695,9 +695,10 @@ public class ApplicationController { deploymentsToRemove.stream() .map(zone -> zone.region().value()) .collect(joining(", ")) + - ", but does not include " + - (deploymentsToRemove.size() > 1 ? "these zones" : "this zone") + - " in deployment.xml. " + + ", but " + (deploymentsToRemove.size() > 1 ? "these " : "this ") + + "instance and region combination" + + (deploymentsToRemove.size() > 1 ? "s are" : " is") + + " removed from deployment.xml. " + ValidationOverrides.toAllowMessage(ValidationId.deploymentRemoval)); // Remove the instance as well, if it is no longer referenced, and contains only production deployments that are removed now. boolean removeInstance = ! deploymentSpec.instanceNames().contains(instance) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java index 402a4bf49a8..d66d1491f73 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Instance.java @@ -220,6 +220,6 @@ public class Instance { @Override public String toString() { - return "application '" + id + "'"; + return "application instance '" + id.toFullString() + "'"; } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 554bf2a57b4..c4e138c4d18 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -175,12 +175,13 @@ public class ControllerTest { fail("Expected exception due to illegal production deployment removal"); } catch (IllegalArgumentException e) { - assertEquals("deployment-removal: application 'tenant.application' is deployed in us-west-1, but does not include this zone in deployment.xml. " + - ValidationOverrides.toAllowMessage(ValidationId.deploymentRemoval), - e.getMessage()); + assertEquals("deployment-removal: application instance 'tenant.application.default' is deployed in us-west-1, " + + "but this instance and region combination is removed from deployment.xml. " + + ValidationOverrides.toAllowMessage(ValidationId.deploymentRemoval), + e.getMessage()); } assertNotNull(context.instance().deployments().get(productionUsWest1.zone()), - "Zone was not removed"); + "Zone was not removed"); // prod zone removal is allowed with override applicationPackage = new ApplicationPackageBuilder() 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 fee97b4e157..ed4f0597fad 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 @@ -973,14 +973,14 @@ public class ApplicationApiTest extends ControllerContainerTest { // Invalid deployment fails tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-central-1/global-rotation", GET) .userIdentity(USER_ID), - "{\"error-code\":\"NOT_FOUND\",\"message\":\"application 'tenant1.application1.instance1' has no deployment in prod.us-central-1\"}", + "{\"error-code\":\"NOT_FOUND\",\"message\":\"application instance 'tenant1.application1.instance1' has no deployment in prod.us-central-1\"}", 404); // Change status of non-existing deployment fails tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-central-1/global-rotation/override", PUT) .userIdentity(USER_ID) .data("{\"reason\":\"unit-test\"}"), - "{\"error-code\":\"NOT_FOUND\",\"message\":\"application 'tenant1.application1.instance1' has no deployment in prod.us-central-1\"}", + "{\"error-code\":\"NOT_FOUND\",\"message\":\"application instance 'tenant1.application1.instance1' has no deployment in prod.us-central-1\"}", 404); // GET global rotation status @@ -1038,7 +1038,7 @@ public class ApplicationApiTest extends ControllerContainerTest { // GET global rotation status without specifying endpointId fails tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-west-1/global-rotation", GET) .userIdentity(USER_ID), - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"application 'tenant1.application1.instance1' has multiple rotations. Query parameter 'endpointId' must be given\"}", + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"application instance 'tenant1.application1.instance1' has multiple rotations. Query parameter 'endpointId' must be given\"}", 400); // GET global rotation status for us-west-1 in default endpoint |