diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2018-10-31 19:39:59 +0100 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2018-10-31 19:39:59 +0100 |
commit | 4a325a7a1ba25d97a4913c0194c9b2a467379a18 (patch) | |
tree | d8f05fac5625406b7883884068fd7579207be46b /controller-server | |
parent | be0dde184b73e1ed9a27e577edade0290edf2e95 (diff) |
More flexibly change change
Diffstat (limited to 'controller-server')
7 files changed, 77 insertions, 61 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index aeb9e6fa453..6dbdefb0913 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -222,35 +222,46 @@ public class DeploymentTrigger { .map(Job::jobType).collect(toList()); } - /** - * Triggers a change of this application - * - * @param applicationId the application to trigger - * @throws IllegalArgumentException if this application already has an ongoing change - */ + /** Triggers a change of this application, unless it already has a change. */ public void triggerChange(ApplicationId applicationId, Change change) { applications().lockOrThrow(applicationId, application -> { - if (application.get().change().isPresent() && ! application.get().deploymentJobs().hasFailures()) - throw new IllegalArgumentException("Could not start " + change + " on " + application + ": " + - application.get().change() + " is already in progress"); - application = application.withChange(change); - if (change.application().isPresent()) - application = application.withOutstandingChange(Change.empty()); + if ( ! application.get().change().isPresent()) { + if (change.application().isPresent()) + application = application.withOutstandingChange(Change.empty()); - applications().store(application); + applications().store(application.withChange(change)); + } + }); + } + + /** Overrides the given application's platform and application changes with any contained in the given change. */ + public void forceChange(ApplicationId applicationId, Change change) { + applications().lockOrThrow(applicationId, application -> { + Change current = application.get().change(); + if (change.platform().isPresent()) + current = current.with(change.platform().get()); + if (change.application().isPresent()) + current = current.with(change.application().get()); + applications().store(application.withChange(current)); }); } /** Cancels a platform upgrade of the given application, and an application upgrade as well if {@code keepApplicationChange}. */ - public void cancelChange(ApplicationId applicationId, boolean keepApplicationChange) { + public void cancelChange(ApplicationId applicationId, ChangesToCancel cancellation) { applications().lockOrThrow(applicationId, application -> { - applications().store(application.withChange(application.get().change().application() - .filter(__ -> keepApplicationChange) - .map(Change::of) - .orElse(Change.empty()))); + Change change; + switch (cancellation) { + case ALL: change = Change.empty(); break; + case PLATFORM: change = application.get().change().withoutPlatform(); break; + case APPLICATION: change = application.get().change().withoutApplication(); break; + default: throw new IllegalArgumentException("Unknown cancellation choice '" + cancellation + "'!"); + } + applications().store(application.withChange(change)); }); } + public enum ChangesToCancel { ALL, PLATFORM, APPLICATION } + // ---------- Conveniences ---------- private ApplicationController applications() { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java index 0c267bd5f6a..2b317032c52 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java @@ -20,9 +20,8 @@ public class OutstandingChangeDeployer extends Maintainer { @Override protected void maintain() { for (Application application : controller().applications().asList()) { - if ( ! application.change().isPresent() // TODO jvenstad: Revisit this: should it check for only platform here? - && application.outstandingChange().isPresent() - && application.deploymentSpec().canChangeRevisionAt(controller().clock().instant())) { + if ( application.outstandingChange().isPresent() + && application.deploymentSpec().canChangeRevisionAt(controller().clock().instant())) { controller().applications().deploymentTrigger().triggerChange(application.id(), application.outstandingChange()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java index 702df58ab98..66481e88a98 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java @@ -7,6 +7,8 @@ import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.application.ApplicationList; +import com.yahoo.vespa.hosted.controller.application.Change; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence; @@ -22,6 +24,8 @@ import java.util.Optional; import java.util.logging.Level; import java.util.logging.Logger; +import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.PLATFORM; + /** * Maintenance job which schedules applications for Vespa version upgrade * @@ -100,20 +104,15 @@ public class Upgrader extends Maintainer { applications = applications.canUpgradeAt(controller().clock().instant()); // wait with applications that are currently blocking upgrades applications = applications.byIncreasingDeployedVersion(); // start with lowest versions applications = applications.first(numberOfApplicationsToUpgrade()); // throttle upgrades - for (Application application : applications.asList()) { - try { - controller().applications().deploymentTrigger().triggerChange(application.id(), application.change().with(version)); - } catch (IllegalArgumentException e) { - log.log(Level.INFO, "Could not trigger change: " + Exceptions.toMessageString(e)); - } - } + for (Application application : applications.asList()) + controller().applications().deploymentTrigger().triggerChange(application.id(), Change.of(version)); } private void cancelUpgradesOf(ApplicationList applications, String reason) { if (applications.isEmpty()) return; log.info("Cancelling upgrading of " + applications.asList().size() + " applications: " + reason); for (Application application : applications.asList()) - controller().applications().deploymentTrigger().cancelChange(application.id(), true); + controller().applications().deploymentTrigger().cancelChange(application.id(), PLATFORM); } /** Returns the number of applications to upgrade in this run */ 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 ff9aca9ee00..a0d3b8ff0e4 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 @@ -65,6 +65,7 @@ import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.RotationStatus; import com.yahoo.vespa.hosted.controller.application.SourceRevision; import com.yahoo.vespa.hosted.controller.athenz.impl.ZmsClientFacade; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel; import com.yahoo.vespa.hosted.controller.restapi.ErrorResponse; import com.yahoo.vespa.hosted.controller.restapi.MessageResponse; import com.yahoo.vespa.hosted.controller.restapi.ResourceResponse; @@ -97,6 +98,8 @@ import java.util.Scanner; import java.util.logging.Level; import java.util.stream.Collectors; +import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.ALL; + /** * This implements the application/v4 API which is used to deploy and manage applications * on hosted Vespa. @@ -738,22 +741,30 @@ public class ApplicationApiHandler extends LoggingRequestHandler { /** Trigger deployment of the last built application package, on a given version */ // TODO Consider move to API for maintenance related operations private HttpResponse deploy(String tenantName, String applicationName, HttpRequest request) { - Version version = decideDeployVersion(request); - if ( ! systemHasVersion(version)) - throw new IllegalArgumentException("Cannot trigger deployment of version '" + version + "': " + - "Version is not active in this system. " + - "Active versions: " + controller.versionStatus().versions() - .stream() - .map(VespaVersion::versionNumber) - .map(Version::toString) - .collect(Collectors.joining(", "))); - ApplicationId id = ApplicationId.from(tenantName, applicationName, "default"); + String requestVersion = readToString(request.getData()); + StringBuilder response = new StringBuilder(); controller.applications().lockOrThrow(id, application -> { - controller.applications().deploymentTrigger().triggerChange(application.get().id(), - application.get().change().with(version)); + Change change; + if ("commit".equals(requestVersion)) + change = Change.of(application.get().outstandingChange().application() + .orElseThrow(() -> new IllegalArgumentException("No outstanding commit to deploy!"))); + else { + Version version = requestVersion == null ? controller.systemVersion() : new Version(requestVersion); + if ( ! systemHasVersion(version)) + throw new IllegalArgumentException("Cannot trigger deployment of version '" + version + "': " + + "Version is not active in this system. " + + "Active versions: " + controller.versionStatus().versions() + .stream() + .map(VespaVersion::versionNumber) + .map(Version::toString) + .collect(Collectors.joining(", "))); + change = Change.of(version); + } + controller.applications().deploymentTrigger().forceChange(application.get().id(), change); + response.append("Triggered " + change + " for " + application); }); - return new MessageResponse("Triggered deployment of application '" + id + "' on version " + version); + return new MessageResponse(response.toString()); } /** Cancel any ongoing change for given application */ @@ -766,7 +777,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { return new MessageResponse("No deployment in progress for " + application + " at this time"); controller.applications().lockOrThrow(id, lockedApplication -> - controller.applications().deploymentTrigger().cancelChange(id, false)); + controller.applications().deploymentTrigger().cancelChange(id, ALL)); return new MessageResponse("Cancelled " + change + " for " + application); } @@ -1123,14 +1134,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler { return controller.versionStatus().versions().stream().anyMatch(v -> v.versionNumber().equals(version)); } - private Version decideDeployVersion(HttpRequest request) { - String requestVersion = readToString(request.getData()); - if (requestVersion != null) - return new Version(requestVersion); - else - return controller.systemVersion(); - } - public static void toSlime(DeploymentCost deploymentCost, Cursor object) { object.setLong("tco", (long)deploymentCost.getTco()); object.setLong("waste", (long)deploymentCost.getWaste()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index 9dd4c25050f..53ab488949e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -22,6 +22,7 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel; import com.yahoo.vespa.hosted.controller.maintenance.JobControl; import com.yahoo.vespa.hosted.controller.maintenance.ReadyJobsTrigger; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; @@ -51,6 +52,8 @@ import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobTy import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.productionUsWest1; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.stagingTest; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest; +import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.ALL; +import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.PLATFORM; import static java.time.temporal.ChronoUnit.MILLIS; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; @@ -133,7 +136,7 @@ public class DeploymentTriggerTest { tester.jobCompletion(component).application(app).nextBuildNumber().uploadArtifact(applicationPackage).submit(); tester.assertRunning(productionUsCentral1, app.id()); - tester.applications().deploymentTrigger().cancelChange(app.id(), false); + tester.applications().deploymentTrigger().cancelChange(app.id(), ALL); tester.deployAndNotify(app, false, systemTest); tester.deployAndNotify(app, false, stagingTest); tester.deployAndNotify(app, false, productionUsCentral1); @@ -561,11 +564,11 @@ public class DeploymentTriggerTest { assertEquals(appVersion1, app.get().deployments().get(ZoneId.from("prod.us-central-1")).applicationVersion()); // Verify the application change is not removed when change is cancelled. - tester.deploymentTrigger().cancelChange(application.id(), true); + tester.deploymentTrigger().cancelChange(application.id(), PLATFORM); assertEquals(Change.of(appVersion1), app.get().change()); // Now cancel the change as is done through the web API. - tester.deploymentTrigger().cancelChange(application.id(), false); + tester.deploymentTrigger().cancelChange(application.id(), ALL); assertEquals(Change.empty(), app.get().change()); // A new version is released, which should now deploy the currently deployed application version to avoid downgrades. @@ -609,7 +612,7 @@ public class DeploymentTriggerTest { tester.upgradeSystem(version1); // Deploy application2 to keep this version present in the system tester.deployCompletely(application2, applicationPackage); - tester.applications().deploymentTrigger().cancelChange(application1.id(), false); + tester.applications().deploymentTrigger().cancelChange(application1.id(), ALL); tester.buildService().clear(); // Clear stale build jobs for cancelled change // version2 is released and application1 starts upgrading @@ -688,7 +691,7 @@ public class DeploymentTriggerTest { tester.upgradeSystem(v2); tester.deployAndNotify(application, true, systemTest); tester.deployAndNotify(application, true, stagingTest); - tester.deploymentTrigger().cancelChange(application.id(), true); + tester.deploymentTrigger().cancelChange(application.id(), PLATFORM); tester.deploy(productionEuWest1, application, applicationPackage); assertEquals(v2, app.get().deployments().get(productionEuWest1.zone(main)).version()); assertEquals(v1, app.get().deployments().get(productionUsEast3.zone(main)).version()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java index 6d4f58531ae..587cbde991f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java @@ -27,6 +27,7 @@ import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobTy import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.productionUsWest1; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.stagingTest; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest; +import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.ALL; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -487,10 +488,10 @@ public class UpgraderTest { // We "manually" cancel upgrades to V1 so that we can use the applications to make V2 fail instead // But we keep one (default4) to avoid V1 being garbage collected - tester.deploymentTrigger().cancelChange(default0.id(), false); - tester.deploymentTrigger().cancelChange(default1.id(), false); - tester.deploymentTrigger().cancelChange(default2.id(), false); - tester.deploymentTrigger().cancelChange(default3.id(), false); + tester.deploymentTrigger().cancelChange(default0.id(), ALL); + tester.deploymentTrigger().cancelChange(default1.id(), ALL); + tester.deploymentTrigger().cancelChange(default2.id(), ALL); + tester.deploymentTrigger().cancelChange(default3.id(), ALL); tester.buildService().clear(); // Applications with default policy start upgrading to V2 diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-deployment.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-deployment.json index f74b2290d81..e3c0bbe0679 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-deployment.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-deployment.json @@ -1 +1 @@ -{"message":"Triggered deployment of application 'tenant1.application1' on version 6.1"}
\ No newline at end of file +{"message":"Triggered upgrade to 6.1 for application 'tenant1.application1'"}
\ No newline at end of file |