diff options
author | Jon Bratseth <jonbratseth@yahoo.com> | 2017-12-14 08:53:35 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-12-14 08:53:35 +0100 |
commit | e494bf9f475d72f0a6f429e73dff03560f2c659f (patch) | |
tree | 62b2390df10d841df6a3f6e17ec27e293cdff9d0 | |
parent | 0c7edbb9ebcdebe1a212ed95769abf01b723cbb2 (diff) | |
parent | 3e6db913d54f50605f59756bdde060c8186df9dd (diff) |
Merge pull request #4440 from vespa-engine/mpolden/remove-all-instances-on-delete
Delete all instances on application delete
2 files changed, 53 insertions, 24 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 fb92109c5df..44e87d258df 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 @@ -7,7 +7,6 @@ import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.TenantName; -import com.yahoo.config.provision.Zone; import com.yahoo.config.provision.ZoneId; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.api.ActivateResult; @@ -22,6 +21,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.athenz.AthenzClientFactory; +import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken; +import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsClient; +import com.yahoo.vespa.hosted.controller.api.integration.athenz.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; @@ -42,10 +45,6 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobReport; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.SourceRevision; -import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory; -import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken; -import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsClient; -import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsException; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.maintenance.DeploymentExpirer; import com.yahoo.vespa.hosted.controller.persistence.ControllerDb; @@ -271,10 +270,11 @@ public class ApplicationController { public ActivateResult deployApplication(ApplicationId applicationId, ZoneId zone, ApplicationPackage applicationPackage, DeployOptions options) { try (Lock lock = lock(applicationId)) { - // TODO: Shouldn't this go through the above method? Seems you can cheat the checks here ... ? - LockedApplication application = get(applicationId).map(application1 -> new LockedApplication(application1, lock)).orElse(new LockedApplication( - new Application(applicationId), lock) - ); + // Not ideal, but since we create on missing and return a result computed inside the lock, + // the lock-with-action methods cannot be used + LockedApplication application = get(applicationId) + .map(app -> new LockedApplication(app, lock)) + .orElseGet(() -> new LockedApplication(new Application(applicationId), lock)); // Determine what we are doing Version version; @@ -505,31 +505,43 @@ public class ApplicationController { } /** - * Deletes the application with this id + * Deletes the the given application. All known instances of the applications will be deleted, + * including PR instances. * * @throws IllegalArgumentException if the application has deployments or the caller is not authorized - * @throws NotExistsException if the application does not exist + * @throws NotExistsException if no instances of the application exist */ - public void deleteApplication(ApplicationId id, Optional<NToken> token) { - if ( ! controller.applications().get(id).isPresent()) - throw new NotExistsException("Could not delete application '" + id + "': Application not found"); + public void deleteApplication(ApplicationId applicationId, Optional<NToken> token) { + // Find all instances of the application + List<ApplicationId> instances = controller.applications().asList(applicationId.tenant()) + .stream() + .map(Application::id) + .filter(id -> id.application().equals(applicationId.application()) && + id.tenant().equals(applicationId.tenant())) + .collect(Collectors.toList()); + if (instances.isEmpty()) { + throw new NotExistsException("Could not delete application '" + applicationId + "': Application not found"); + } - lockOrThrow(id, application -> { + // TODO: Make this one transaction when database is moved to ZooKeeper + instances.forEach(id -> lockOrThrow(id, application -> { if ( ! application.deployments().isEmpty()) throw new IllegalArgumentException("Could not delete '" + application + "': It has active deployments"); - + Tenant tenant = controller.tenants().tenant(new TenantId(id.tenant().value())).get(); if (tenant.isAthensTenant() && ! token.isPresent()) throw new IllegalArgumentException("Could not delete '" + application + "': No NToken provided"); - // NB: Next 2 lines should have been one transaction - if (tenant.isAthensTenant()) + // Only delete in Athenz once + if (id.instance().isDefault() && tenant.isAthensTenant()) { zmsClientFactory.createZmsClientWithAuthorizedServiceToken(token.get()) - .deleteApplication(tenant.getAthensDomain().get(), new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(id.application().value())); + .deleteApplication(tenant.getAthensDomain().get(), + new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(id.application().value())); + } db.deleteApplication(id); log.info("Deleted " + application); - }); + })); } /** 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 4a78c97750f..58c74c9d6d2 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 @@ -10,7 +10,6 @@ import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; -import com.yahoo.config.provision.Zone; import com.yahoo.config.provision.ZoneId; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.api.Tenant; @@ -23,6 +22,7 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.api.identifiers.UserGroup; import com.yahoo.vespa.hosted.controller.api.integration.BuildService.BuildJob; +import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken; import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; @@ -32,7 +32,6 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType; import com.yahoo.vespa.hosted.controller.application.JobStatus; -import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken; import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.BuildSystem; @@ -299,10 +298,14 @@ public class ControllerTest { // staging deployment long app1ProjectId = 22; - ApplicationId app1 = tester.createAndDeploy("tenant1", "domain1", "application1", Environment.staging, app1ProjectId).id(); + ApplicationId app1 = tester.createAndDeploy("tenant1", "domain1", + "application1", Environment.staging, + app1ProjectId).id(); // pull-request deployment - uses different instance id - ApplicationId app1pr = tester.createAndDeploy("tenant1", "domain1", "application1", "default-pr1", Environment.staging, app1ProjectId, null).id(); + ApplicationId app1pr = tester.createAndDeploy("tenant1", "domain1", + "application1", "default-pr1", + Environment.staging, app1ProjectId, null).id(); assertTrue(applications.get(app1).isPresent()); assertEquals(app1, applications.get(app1).get().id()); @@ -317,6 +320,20 @@ public class ControllerTest { assertEquals(app1, applications.get(app1).get().id()); assertTrue(applications.get(app1pr).isPresent()); assertEquals(app1pr, applications.get(app1pr).get().id()); + + // Deleting application also removes PR instance + ApplicationId app2 = tester.createAndDeploy("tenant1", "domain1", + "application2", Environment.staging, + 33).id(); + tester.controller().applications().deleteApplication(app1, Optional.of(new NToken("ntoken"))); + assertEquals("All instances deleted", 0, + tester.controller().applications().asList(app1.tenant()).stream() + .filter(app -> app.id().application().equals(app1.application())) + .count()); + assertEquals("Other application survives", 1, + tester.controller().applications().asList(app1.tenant()).stream() + .filter(app -> app.id().application().equals(app2.application())) + .count()); } @Test |