diff options
author | Jon Bratseth <bratseth@yahoo-inc.com> | 2017-10-09 15:19:17 +0200 |
---|---|---|
committer | Jon Bratseth <bratseth@yahoo-inc.com> | 2017-10-09 15:19:17 +0200 |
commit | e9f9e0697b8bc4789058171b3f642ea9fd18b97a (patch) | |
tree | cea2d6c0e3def9c9ae037435a89e36e83c876ae9 | |
parent | a9516bbd206d26180d9bc6f0ab080588b4ca52a0 (diff) |
Support blocking application changes
13 files changed, 140 insertions, 38 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java index b2a908077e5..d1e8dc21549 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java @@ -270,8 +270,8 @@ public class DeploymentSpec { // TODO: Remove block-upgrade on Vespa 7 if ( ! "block-change".equals(tag.getTagName()) && !"block-upgrade".equals(tag.getTagName())) continue; - boolean blockVersions = trueOrMissing(tag.getAttribute("versions")); - boolean blockRevisions = trueOrMissing(tag.getAttribute("revisions")) + boolean blockVersions = trueOrMissing(tag.getAttribute("version")); + boolean blockRevisions = trueOrMissing(tag.getAttribute("revision")) && !tag.getTagName().equals("block-upgrade"); // TODO: Remove condition on Vespa 7 String daySpec = tag.getAttribute("days"); diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java index c2b68b54655..943432213ac 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java @@ -313,7 +313,7 @@ public class DeploymentSpecTest { public void deploymentSpecWithChangeBlocker() { StringReader r = new StringReader( "<deployment>\n" + - " <block-change revisions='false' days='mon,tue' hours='15-16'/>\n" + + " <block-change revision='false' days='mon,tue' hours='15-16'/>\n" + " <block-change days='sat' hours='10' time-zone='CET'/>\n" + " <prod>\n" + " <region active='true'>us-west-1</region>\n" + diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc index 16cc3acfafa..03331807d15 100644 --- a/config-model/src/main/resources/schema/deployment.rnc +++ b/config-model/src/main/resources/schema/deployment.rnc @@ -17,8 +17,8 @@ Upgrade = element upgrade { } BlockChange = element block-change { - attribute revisions { xsd:boolean }? & - attribute versions { xsd:boolean }? & + attribute revision { xsd:boolean }? & + attribute version { xsd:boolean }? & attribute days { xsd:string } & attribute hours { xsd:string } & attribute time-zone { xsd:string }? diff --git a/config-model/src/test/schema-test-files/deployment.xml b/config-model/src/test/schema-test-files/deployment.xml index 2fd6d7c3ec8..1b2a61e1a7a 100644 --- a/config-model/src/test/schema-test-files/deployment.xml +++ b/config-model/src/test/schema-test-files/deployment.xml @@ -1,7 +1,7 @@ <!-- Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> <deployment version='1.0'> <upgrade policy='canary'/> - <block-change revisions='true' versions='false' days="mon,tue" hours="14,15"/> + <block-change revision='true' version='false' days="mon,tue" hours="14,15"/> <block-change days="mon,tue" hours="14,15" time-zone="CET"/> <test/> <staging/> diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java index f68ce3ebfa5..d53c16d2ce6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java @@ -276,4 +276,16 @@ public class Application { return true; } + /** Returns true if there is no outstanding change to deploy - i.e deploying is empty or completedly deployed */ + public boolean deployingCompleted() { + if ( ! deploying.isPresent()) return true; + return deploymentJobs().isDeployed(deploying.get()); + } + + /** Returns true if there is a current change which is blocked from being deployed to production at this instant */ + public boolean deployingBlocked(Instant instant) { + if ( ! deploying.isPresent()) return false; + return deploying.get().blockedBy(deploymentSpec, instant); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java index 596cbbebd45..bc82451580a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java @@ -2,7 +2,9 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.component.Version; +import com.yahoo.config.application.api.DeploymentSpec; +import java.time.Instant; import java.util.Objects; import java.util.Optional; @@ -13,6 +15,9 @@ import java.util.Optional; */ public abstract class Change { + /** Returns true if this change is blocked by the given spec art the given instant */ + public abstract boolean blockedBy(DeploymentSpec deploymentSpec, Instant instant); + /** A change to the application package revision of an application */ public static class ApplicationChange extends Change { @@ -27,6 +32,11 @@ public abstract class Change { public Optional<ApplicationRevision> revision() { return revision; } @Override + public boolean blockedBy(DeploymentSpec deploymentSpec, Instant instant) { + return ! deploymentSpec.canChangeRevisionAt(instant); + } + + @Override public int hashCode() { return revision.hashCode(); } @Override @@ -71,6 +81,11 @@ public abstract class Change { public Version version() { return version; } @Override + public boolean blockedBy(DeploymentSpec deploymentSpec, Instant instant) { + return ! deploymentSpec.canUpgradeAt(instant); + } + + @Override public int hashCode() { return version.hashCode(); } @Override diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java index 26bef06adcf..5036493603a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java @@ -118,14 +118,11 @@ public class DeploymentJobs { return true; // other environments do not have any preconditions } - /** Returns whether change has been deployed completely */ - public boolean isDeployed(Optional<Change> change) { - if (!change.isPresent()) { - return true; - } + /** Returns whether the given change has been deployed completely */ + public boolean isDeployed(Change change) { return status.values().stream() .filter(status -> status.type().isProduction()) - .allMatch(status -> isSuccessful(change.get(), status.type())); + .allMatch(status -> isSuccessful(change, status.type())); } /** Returns whether job has completed successfully */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java index 48ebbc9f972..0dac79964f7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java @@ -94,6 +94,9 @@ public class DeploymentOrder { return job == JobType.component; } + /** Returns whether this is the last job before production */ + public boolean isLastBeforeProduction(JobType jobType) { return jobType == JobType.stagingTest; } + /** Returns whether the given job is last in a deployment */ public boolean isLast(JobType job, Application application) { List<DeploymentSpec.Step> deploymentSteps = deploymentSteps(application); @@ -105,7 +108,7 @@ public class DeploymentOrder { // Step may not exist for all jobs, e.g. component return step.map(s -> s.equals(lastStep)).orElse(false); } - + /** Returns jobs for given deployment spec, in the order they are declared */ public List<JobType> jobsFrom(DeploymentSpec deploymentSpec) { return deploymentSpec.steps().stream() 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 423d8c674df..5fe6edf84cc 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 @@ -65,17 +65,26 @@ public class DeploymentTrigger { Application application = applications().require(report.applicationId()); application = application.withJobCompletion(report, clock.instant(), controller); - // Handle successful first and last job - if (order.isFirst(report.jobType()) && report.success()) { // the first job tells us that a change occurred - if (application.deploying().isPresent() && ! application.deploymentJobs().hasFailures()) { // postpone until the current deployment is done - applications().store(application.withOutstandingChange(true), lock); + // Handle successful starting and ending + if (report.success()) { + if (order.isFirst(report.jobType())) { + // the first job tells us that a change occurred + if (application.deploying().isPresent() && !application.deploymentJobs().hasFailures()) { // postpone until the current deployment is done + applications().store(application.withOutstandingChange(true), lock); + return; + } else { // start a new change deployment + application = application.withDeploying(Optional.of(Change.ApplicationChange.unknown())); + } + } + else if (order.isLastBeforeProduction(report.jobType()) && application.deployingBlocked(clock.instant())) { + // run tests to provide feedback on errors but stop before production + applications().store(application.withDeploying(Optional.empty()), lock); return; } - else { // start a new change deployment - application = application.withDeploying(Optional.of(Change.ApplicationChange.unknown())); + else if (order.isLast(report.jobType(), application) && application.deployingCompleted()) { + // change completed + application = application.withDeploying(Optional.empty()); } - } else if (order.isLast(report.jobType(), application) && report.success() && application.deploymentJobs().isDeployed(application.deploying())) { - application = application.withDeploying(Optional.empty()); } // Trigger next diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java index f193d4718c6..4394ae0498e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java @@ -29,6 +29,7 @@ public class ApplicationPackageBuilder { private final StringBuilder environmentBody = new StringBuilder(); private final StringBuilder validationOverridesBody = new StringBuilder(); private final StringBuilder blockUpgrade = new StringBuilder(); + private String searchDefinition = "search test { }"; public ApplicationPackageBuilder upgradePolicy(String upgradePolicy) { this.upgradePolicy = upgradePolicy; @@ -61,14 +62,15 @@ public class ApplicationPackageBuilder { return this; } - public ApplicationPackageBuilder blockUpgrade(String daySpec, String hourSpec, String zoneSpec) { - blockUpgrade.append(" <block-upgrade days=\""); - blockUpgrade.append(daySpec); - blockUpgrade.append("\" hours=\""); - blockUpgrade.append(hourSpec); - blockUpgrade.append("\" time-zone=\""); - blockUpgrade.append(zoneSpec); - blockUpgrade.append("\"/>\n"); + public ApplicationPackageBuilder blockChange(boolean revision, boolean version, + String daySpec, String hourSpec, String zoneSpec) { + blockUpgrade.append(" <block-change"); + blockUpgrade.append(" revision='").append(revision).append("'"); + blockUpgrade.append(" version='").append(version).append("'"); + blockUpgrade.append(" days='").append(daySpec).append("'"); + blockUpgrade.append(" hours='").append(hourSpec).append("'"); + blockUpgrade.append(" time-zone='").append(zoneSpec).append("'"); + blockUpgrade.append("/>\n"); return this; } @@ -80,7 +82,13 @@ public class ApplicationPackageBuilder { validationOverridesBody.append("</allow>\n"); return this; } - + + /** Sets the content of the search definition test.sd */ + public ApplicationPackageBuilder searchDefinition(String testSearchDefinition) { + this.searchDefinition = testSearchDefinition; + return this; + } + private byte[] deploymentSpec() { StringBuilder xml = new StringBuilder("<deployment version='1.0'>\n"); if (upgradePolicy != null) { @@ -98,7 +106,7 @@ public class ApplicationPackageBuilder { xml.append(">\n</deployment>"); return xml.toString().getBytes(StandardCharsets.UTF_8); } - + private byte[] validationOverrides() { String xml = "<validation-overrides version='1.0'>\n" + validationOverridesBody + @@ -106,6 +114,10 @@ public class ApplicationPackageBuilder { return xml.getBytes(StandardCharsets.UTF_8); } + private byte[] searchDefinition() { + return searchDefinition.getBytes(StandardCharsets.UTF_8); + } + public ApplicationPackage build() { ByteArrayOutputStream zip = new ByteArrayOutputStream(); ZipOutputStream out = new ZipOutputStream(zip); @@ -116,6 +128,9 @@ public class ApplicationPackageBuilder { out.putNextEntry(new ZipEntry("validation-overrides.xml")); out.write(validationOverrides()); out.closeEntry(); + out.putNextEntry(new ZipEntry("search-definitions/test.sd")); + out.write(searchDefinition()); + out.closeEntry(); } catch (IOException e) { throw new UncheckedIOException(e); } finally { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java index ad723677686..c4829e5594f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java @@ -22,8 +22,10 @@ import com.yahoo.vespa.hosted.controller.maintenance.Upgrader; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import java.time.Duration; +import java.util.List; import java.util.Optional; import java.util.UUID; +import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -128,12 +130,23 @@ public class DeploymentTester { public void deployCompletely(Application application, ApplicationPackage applicationPackage) { notifyJobCompletion(JobType.component, application, true); assertTrue(applications().require(application.id()).deploying().isPresent()); - completeDeployment(application, applicationPackage, Optional.empty()); + completeDeployment(application, applicationPackage, Optional.empty(), true); } - private void completeDeployment(Application application, ApplicationPackage applicationPackage, Optional<JobType> failOnJob) { + /** Deploy application using the given application package, but expecting to stop after test phases */ + public void deployTestOnly(Application application, ApplicationPackage applicationPackage) { + notifyJobCompletion(JobType.component, application, true); + assertTrue(applications().require(application.id()).deploying().isPresent()); + completeDeployment(application, applicationPackage, Optional.empty(), false); + } + + private void completeDeployment(Application application, ApplicationPackage applicationPackage, + Optional<JobType> failOnJob, boolean includingProductionZones) { DeploymentOrder order = new DeploymentOrder(controller()); - for (JobType job : order.jobsFrom(applicationPackage.deploymentSpec())) { + List<JobType> jobs = order.jobsFrom(applicationPackage.deploymentSpec()); + if ( ! includingProductionZones) + jobs = jobs.stream().filter(job -> ! job.isProduction()).collect(Collectors.toList()); + for (JobType job : jobs) { boolean failJob = failOnJob.map(j -> j.equals(job)).orElse(false); deployAndNotify(application, applicationPackage, !failJob, job); if (failJob) { @@ -159,7 +172,7 @@ public class DeploymentTester { public void completeUpgrade(Application application, Version version, String upgradePolicy) { assertTrue(applications().require(application.id()).deploying().isPresent()); assertEquals(new Change.VersionChange(version), applications().require(application.id()).deploying().get()); - completeDeployment(application, applicationPackage(upgradePolicy), Optional.empty()); + completeDeployment(application, applicationPackage(upgradePolicy), Optional.empty(), true); } public void completeUpgradeWithError(Application application, Version version, String upgradePolicy, JobType failOnJob) { @@ -173,7 +186,7 @@ public class DeploymentTester { private void completeUpgradeWithError(Application application, Version version, ApplicationPackage applicationPackage, Optional<JobType> failOnJob) { assertTrue(applications().require(application.id()).deploying().isPresent()); assertEquals(new Change.VersionChange(version), applications().require(application.id()).deploying().get()); - completeDeployment(application, applicationPackage, failOnJob); + completeDeployment(application, applicationPackage, failOnJob, true); } public void deploy(JobType job, Application application, ApplicationPackage applicationPackage) { 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 5abadf28cfb..650a5865006 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 @@ -3,7 +3,9 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.yahoo.component.Version; import com.yahoo.config.provision.Environment; +import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.controller.Application; +import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; @@ -11,6 +13,7 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType; import org.junit.Test; import java.time.Duration; +import java.time.Instant; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -257,6 +260,41 @@ public class DeploymentTriggerTest { } @Test + public void testBlockRevisionChange() { + ManualClock clock = new ManualClock(Instant.parse("2017-09-26T17:30:00.00Z")); // Tuesday, 17:30 + DeploymentTester tester = new DeploymentTester(new ControllerTester(clock)); + Version version = Version.fromString("5.0"); + tester.updateVersionStatus(version); + + ApplicationPackageBuilder applicationPackageBuilder = new ApplicationPackageBuilder() + .upgradePolicy("canary") + // Block revision changes on tuesday in hours 18 and 19 + .blockChange(true, false, "tue", "18-19", "UTC") + .region("us-west-1"); + + Application app = tester.createAndDeploy("app1", 1, applicationPackageBuilder.build()); + + tester.clock().advance(Duration.ofHours(1)); // Enter block window: 18:30 + + String searchDefinition = + "search test {\n" + + " document test {\n" + + " field test type string {\n" + + " }\n" + + " }\n" + + "}\n"; + ApplicationPackage changedApplication = applicationPackageBuilder.searchDefinition(searchDefinition).build(); + + tester.deployTestOnly(app, changedApplication); + assertFalse(tester.application("app1").deploying().isPresent()); + + tester.clock().advance(Duration.ofHours(2)); // Exit block window: 20:30 + + tester.deployCompletely(app, changedApplication); + assertFalse(tester.application("app1").deploying().isPresent()); + } + + @Test public void testHandleMultipleNotificationsFromLastJob() { DeploymentTester tester = new DeploymentTester(); BuildSystem buildSystem = tester.buildSystem(); 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 95b700f4dd9..08878dd0051 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 @@ -364,7 +364,7 @@ public class UpgraderTest { } @Test - public void testConsidersBlockUpgradeWindow() { + public void testBlockVersionChange() { ManualClock clock = new ManualClock(Instant.parse("2017-09-26T18:00:00.00Z")); // A tuesday DeploymentTester tester = new DeploymentTester(new ControllerTester(clock)); Version version = Version.fromString("5.0"); @@ -373,7 +373,7 @@ public class UpgraderTest { ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .upgradePolicy("canary") // Block upgrades on tuesday in hours 18 and 19 - .blockUpgrade("tue", "18-19", "UTC") + .blockChange(false, true, "tue", "18-19", "UTC") .region("us-west-1") .build(); |