diff options
author | Martin Polden <mpolden@mpolden.no> | 2021-03-19 12:54:32 +0100 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2021-03-19 12:54:32 +0100 |
commit | e99fbb9596991f1a84179c8f75c8805bc14f6ba8 (patch) | |
tree | b9bca43c3a84898cba7ff6d01e76763ff26c0acd /controller-server | |
parent | 93116766f5fecd91e62119f0f82bc7b103c516bc (diff) |
Extract validation method
Diffstat (limited to 'controller-server')
-rw-r--r-- | controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java | 100 |
1 files changed, 54 insertions, 46 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 51b7a24b5e4..4351ac17001 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 @@ -329,18 +329,24 @@ public class ApplicationController { }); } - public LockedApplication withNewInstance(LockedApplication application, ApplicationId id) { - if (id.instance().isTester()) - throw new IllegalArgumentException("'" + id + "' is a tester application!"); - InstanceId.validate(id.instance().value()); + /** Fetches the requested application package from the artifact store(s). */ + public ApplicationPackage getApplicationPackage(ApplicationId id, ApplicationVersion version) { + return new ApplicationPackage(applicationStore.get(id.tenant(), id.application(), version)); + } - if (getInstance(id).isPresent()) - throw new IllegalArgumentException("Could not create '" + id + "': Instance already exists"); - if (getInstance(dashToUnderscore(id)).isPresent()) // VESPA-1945 - throw new IllegalArgumentException("Could not create '" + id + "': Instance " + dashToUnderscore(id) + " already exists"); + /** Returns given application with a new instance */ + public LockedApplication withNewInstance(LockedApplication application, ApplicationId instance) { + if (instance.instance().isTester()) + throw new IllegalArgumentException("'" + instance + "' is a tester application!"); + InstanceId.validate(instance.instance().value()); - log.info("Created " + id); - return application.withNewInstance(id.instance()); + if (getInstance(instance).isPresent()) + throw new IllegalArgumentException("Could not create '" + instance + "': Instance already exists"); + if (getInstance(dashToUnderscore(instance)).isPresent()) // VESPA-1945 + throw new IllegalArgumentException("Could not create '" + instance + "': Instance " + dashToUnderscore(instance) + " already exists"); + + log.info("Created " + instance); + return application.withNewInstance(instance.instance()); } /** Deploys an application package for an existing application instance. */ @@ -369,14 +375,7 @@ public class ApplicationController { try (Lock lock = lock(applicationId)) { LockedApplication application = new LockedApplication(requireApplication(applicationId), lock); Instance instance = application.get().require(job.application().instance()); - - Deployment deployment = instance.deployments().get(zone); - if ( zone.environment().isProduction() && deployment != null - && ( platform.compareTo(deployment.version()) < 0 && ! instance.change().isPinned() - || revision.compareTo(deployment.applicationVersion()) < 0 && ! (revision.isUnknown() && controller.system().isCd()))) - throw new IllegalArgumentException(String.format("Rejecting deployment of application %s to %s, as the requested versions (platform: %s, application: %s)" + - " are older than the currently deployed (platform: %s, application: %s).", - job.application(), zone, platform, revision, deployment.version(), deployment.applicationVersion())); + rejectOldChange(instance, platform, revision, job, zone); if ( ! applicationPackage.trustedCertificates().isEmpty() && run.testerCertificate().isPresent()) @@ -403,21 +402,6 @@ public class ApplicationController { } } - private QuotaUsage deploymentQuotaUsage(ZoneId zoneId, ApplicationId applicationId) { - var application = configServer.nodeRepository().getApplication(zoneId, applicationId); - return DeploymentQuotaCalculator.calculateQuotaUsage(application); - } - - private ApplicationPackage getApplicationPackage(ApplicationId application, ZoneId zone, ApplicationVersion revision) { - return new ApplicationPackage(revision.isUnknown() ? applicationStore.getDev(application, zone) - : applicationStore.get(application.tenant(), application.application(), revision)); - } - - /** Fetches the requested application package from the artifact store(s). */ - public ApplicationPackage getApplicationPackage(ApplicationId id, ApplicationVersion version) { - return new ApplicationPackage(applicationStore.get(id.tenant(), id.application(), version)); - } - /** Stores the deployment spec and validation overrides from the application package, and runs cleanup. */ public LockedApplication storeWithUpdatedConfig(LockedApplication application, ApplicationPackage applicationPackage) { applicationPackageValidator.validate(application.get(), applicationPackage, clock.instant()); @@ -683,6 +667,11 @@ public class ApplicationController { } } + /** Sets suspension status of the given deployment in its zone. */ + public void setSuspension(DeploymentId deploymentId, boolean suspend) { + configServer.setSuspension(deploymentId, suspend); + } + /** Deactivate application in the given zone */ public void deactivate(ApplicationId id, ZoneId zone) { lockApplicationOrThrow(TenantAndApplicationId.from(id), @@ -710,14 +699,6 @@ public class ApplicationController { public DeploymentTrigger deploymentTrigger() { return deploymentTrigger; } - private TenantAndApplicationId dashToUnderscore(TenantAndApplicationId id) { - return TenantAndApplicationId.from(id.tenant().value(), id.application().value().replaceAll("-", "_")); - } - - private ApplicationId dashToUnderscore(ApplicationId id) { - return dashToUnderscore(TenantAndApplicationId.from(id)).instance(id.instance()); - } - /** * Returns a lock which provides exclusive rights to changing this application. * Any operation which stores an application need to first acquire this lock, then read, modify @@ -798,6 +779,38 @@ public class ApplicationController { } } + private void rejectOldChange(Instance instance, Version platform, ApplicationVersion revision, JobId job, ZoneId zone) { + Deployment deployment = instance.deployments().get(zone); + if (deployment == null) return; + if (!zone.environment().isProduction()) return; + + boolean platformIsOlder = platform.compareTo(deployment.version()) < 0 && !instance.change().isPinned(); + boolean revisionIsOlder = revision.compareTo(deployment.applicationVersion()) < 0 && + !(revision.isUnknown() && controller.system().isCd()); + if (platformIsOlder || revisionIsOlder) + throw new IllegalArgumentException(String.format("Rejecting deployment of application %s to %s, as the requested versions (platform: %s, application: %s)" + + " are older than the currently deployed (platform: %s, application: %s).", + job.application(), zone, platform, revision, deployment.version(), deployment.applicationVersion())); + } + + private TenantAndApplicationId dashToUnderscore(TenantAndApplicationId id) { + return TenantAndApplicationId.from(id.tenant().value(), id.application().value().replaceAll("-", "_")); + } + + private ApplicationId dashToUnderscore(ApplicationId id) { + return dashToUnderscore(TenantAndApplicationId.from(id)).instance(id.instance()); + } + + private QuotaUsage deploymentQuotaUsage(ZoneId zoneId, ApplicationId applicationId) { + var application = configServer.nodeRepository().getApplication(zoneId, applicationId); + return DeploymentQuotaCalculator.calculateQuotaUsage(application); + } + + private ApplicationPackage getApplicationPackage(ApplicationId application, ZoneId zone, ApplicationVersion revision) { + return new ApplicationPackage(revision.isUnknown() ? applicationStore.getDev(application, zone) + : applicationStore.get(application.tenant(), application.application(), revision)); + } + /* * Get the AthenzUser from this principal or Optional.empty if this does not represent a user. */ @@ -855,9 +868,4 @@ public class ApplicationController { return Map.copyOf(warnings); } - /** Sets suspension status of the given deployment in its zone. */ - public void setSuspension(DeploymentId deploymentId, boolean suspend) { - configServer.setSuspension(deploymentId, suspend); - } - } |