diff options
author | Harald Musum <musum@yahoo-inc.com> | 2017-09-11 13:00:14 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-09-11 13:00:14 +0200 |
commit | bcfa0697aa47018dc6d178ab6062abc61ec77a27 (patch) | |
tree | 40e6eddda9b68ecf4506f9b07fdf5afe870b4727 | |
parent | 82fb9f5eefb21bcd8c00475bff92c77f7b8ed060 (diff) | |
parent | 5e6f05de84115826b216373979d9991d4fa5e0a2 (diff) |
Merge pull request #3379 from vespa-engine/mpolden/idempotent-deactivation
Make deactivate idempotent
3 files changed, 35 insertions, 16 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 1b3821a12bf..1edaef6edb8 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 @@ -22,6 +22,10 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname; import com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; +import com.yahoo.vespa.hosted.controller.api.integration.athens.NToken; +import com.yahoo.vespa.hosted.controller.api.integration.athens.ZmsClient; +import com.yahoo.vespa.hosted.controller.api.integration.athens.ZmsClientFactory; +import com.yahoo.vespa.hosted.controller.api.integration.athens.ZmsException; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerClient; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log; import com.yahoo.vespa.hosted.controller.api.integration.configserver.NoInstanceException; @@ -31,10 +35,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordId; import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingEndpoint; import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerator; -import com.yahoo.vespa.hosted.controller.api.integration.athens.ZmsClient; -import com.yahoo.vespa.hosted.controller.api.integration.athens.ZmsClientFactory; -import com.yahoo.vespa.hosted.controller.api.integration.athens.ZmsException; -import com.yahoo.vespa.hosted.controller.api.integration.athens.NToken; import com.yahoo.vespa.hosted.controller.api.rotation.Rotation; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.ApplicationRevision; @@ -490,20 +490,30 @@ public class ApplicationController { } } + /** Deactivate application in the given zone */ + public Application deactivate(Application application, Zone zone) { + return deactivate(application, zone, Optional.empty(), false); + } + + /** Deactivate a known deployment of the given application */ public Application deactivate(Application application, Deployment deployment, boolean requireThatDeploymentHasExpired) { + return deactivate(application, deployment.zone(), Optional.of(deployment), requireThatDeploymentHasExpired); + } + + private Application deactivate(Application application, Zone zone, Optional<Deployment> deployment, + boolean requireThatDeploymentHasExpired) { try (Lock lock = lock(application.id())) { - // TODO: ignore no application errors for config server client, only return such errors from sherpa client. - if (requireThatDeploymentHasExpired && ! DeploymentExpirer.hasExpired(controller.zoneRegistry(), deployment, - clock.instant())) + if (deployment.isPresent() && requireThatDeploymentHasExpired && ! DeploymentExpirer.hasExpired( + controller.zoneRegistry(), deployment.get(), clock.instant())) { return application; + } try { - configserverClient.deactivate(new DeploymentId(application.id(), deployment.zone())); - } - catch (NoInstanceException e) { + configserverClient.deactivate(new DeploymentId(application.id(), zone)); + } catch (NoInstanceException ignored) { // ok; already gone } - application = application.withoutDeploymentIn(deployment.zone()); + application = application.withoutDeploymentIn(zone); store(application, lock); return application; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index d701f3d57a0..6d1edc22b93 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -796,13 +796,15 @@ public class ApplicationApiHandler extends LoggingRequestHandler { private HttpResponse deactivate(String tenantName, String applicationName, String instanceName, String environment, String region) { Application application = controller.applications().require(ApplicationId.from(tenantName, applicationName, instanceName)); - + Zone zone = new Zone(Environment.from(environment), RegionName.from(region)); Deployment deployment = application.deployments().get(zone); - if (deployment == null) - return ErrorResponse.notFoundError("Could not deactivate: " + application + " is not deployed in " + zone); - - controller.applications().deactivate(application, deployment, false); + if (deployment == null) { + // Attempt to deactivate application even if the deployment is not known by the controller + controller.applications().deactivate(application, zone); + } else { + controller.applications().deactivate(application, deployment, false); + } // TODO: Change to return JSON return new StringResponse("Deactivated " + path(TenantResource.API_PATH, tenantName, 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 97e1bac35c8..7d1700270ea 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 @@ -194,6 +194,13 @@ public class ApplicationApiTest extends ControllerContainerTest { "", Request.Method.DELETE), "Deactivated tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default"); + + // DELETE (deactivate) a deployment is idempotent + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default", + "", + Request.Method.DELETE), + "Deactivated tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default"); + // DELETE an application tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1", "", Request.Method.DELETE), ""); |