diff options
author | Valerij Fredriksen <freva@users.noreply.github.com> | 2021-07-05 12:17:42 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-07-05 12:17:42 +0200 |
commit | de0e4caae8412071e68f6b939fa82669ddfb042c (patch) | |
tree | c726d064cbd0391a8a29fffb205f44c7d5292ce5 | |
parent | 82ef508085200f50b40e87a907dc592227ac7609 (diff) | |
parent | 7bde4f3fcc7830acc76f7499cf76ca1f02e961ac (diff) |
Merge pull request #18527 from vespa-engine/revert-18505-revert-18498-jonmv/deploy-tester-and-real-in-parallel
Revert "Revert "Deploy tester and real apps in parallell""
6 files changed, 71 insertions, 16 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index cc29ed5b840..47478d14c33 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -88,7 +88,9 @@ import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.error; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.installationFailed; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.outOfCapacity; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.running; +import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.success; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.testFailure; +import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.succeeded; import static com.yahoo.vespa.hosted.controller.deployment.Step.copyVespaLogs; import static com.yahoo.vespa.hosted.controller.deployment.Step.deactivateReal; import static com.yahoo.vespa.hosted.controller.deployment.Step.deactivateTester; @@ -189,11 +191,20 @@ public class InternalStepRunner implements StepRunner { } private Optional<RunStatus> deployReal(RunId id, boolean setTheStage, DualLogger logger) { + Optional<X509Certificate> testerCertificate = controller.jobController().run(id).get().testerCertificate(); return deploy(() -> controller.applications().deploy(id.job(), setTheStage), controller.jobController().run(id).get() .stepInfo(setTheStage ? deployInitialReal : deployReal).get() .startTime().get(), - logger); + logger) + .filter(result -> { + // If no tester cert, or deployment failed, propagate original result. + if (testerCertificate.isEmpty() || result != running) + return true; + // If tester cert, ensure real is deployed with the tester cert whose key was successfully deployed. + return controller.jobController().run(id).get().stepStatus(deployTester).get() == succeeded + && testerCertificate.equals(controller.jobController().run(id).get().testerCertificate()); + }); } private Optional<RunStatus> deployTester(RunId id, DualLogger logger) { @@ -250,7 +261,7 @@ public class InternalStepRunner implements StepRunner { case OUT_OF_CAPACITY: logger.log(e.message()); return controller.system().isCd() && startTime.plus(timeouts.capacity()).isAfter(controller.clock().instant()) - ? Optional.empty() + ? result : Optional.of(outOfCapacity); case INVALID_APPLICATION_PACKAGE: case BAD_REQUEST: diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java index baba4771370..3077dc5211a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java @@ -34,7 +34,7 @@ public enum Step { installTester(false, deployTester), /** Download and deploy the initial real application, for staging tests. */ - deployInitialReal(false, deployTester), + deployInitialReal(false), /** See that the real application has had its nodes converge to the initial state. */ installInitialReal(false, deployInitialReal), @@ -46,7 +46,7 @@ public enum Step { endStagingSetup(false, startStagingSetup), /** Download and deploy real application, restarting services if required. */ - deployReal(false, endStagingSetup, deployTester), + deployReal(false, endStagingSetup), /** See that real application has had its nodes converge to the wanted version and generation. */ installReal(false, deployReal), diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java index 2819d9970b7..d8bddac4187 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java @@ -33,7 +33,6 @@ import org.junit.Test; import java.io.IOException; import java.io.UncheckedIOException; -import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -54,11 +53,11 @@ import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.app import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTester.instanceId; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.deploymentFailed; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.installationFailed; -import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.running; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.failed; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.succeeded; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.unfinished; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; /** @@ -422,6 +421,43 @@ public class InternalStepRunnerTest { } @Test + public void realDeploymentRequiresForTesterCert() { + tester.controllerTester().zoneRegistry().setSystemName(SystemName.Public); + var zones = List.of(ZoneApiMock.fromId("test.aws-us-east-1c"), + ZoneApiMock.fromId("staging.aws-us-east-1c"), + ZoneApiMock.fromId("prod.aws-us-east-1c")); + tester.controllerTester().zoneRegistry() + .setZones(zones) + .setRoutingMethod(zones, RoutingMethod.exclusive); + tester.configServer().bootstrap(tester.controllerTester().zoneRegistry().zones().all().ids(), SystemApplication.values()); + ZoneId testZone = JobType.systemTest.zone(tester.controller().system()); + RunId id = app.newRun(JobType.systemTest); + tester.configServer().throwOnPrepare(instanceId -> { + if (instanceId.instance().isTester()) + throw new ConfigServerException(ConfigServerException.ErrorCode.PARENT_HOST_NOT_READY, "provisioning", "deploy tester"); + }); + tester.runner().run(); + assertEquals(unfinished, tester.jobs().run(id).get().stepStatuses().get(Step.deployTester)); + assertEquals(unfinished, tester.jobs().run(id).get().stepStatuses().get(Step.deployReal)); + + List<X509Certificate> oldTrusted = new ArrayList<>(DeploymentContext.publicApplicationPackage().trustedCertificates()); + X509Certificate oldCert = tester.jobs().run(id).get().testerCertificate().get(); + oldTrusted.add(oldCert); + assertEquals(oldTrusted, tester.configServer().application(app.instanceId(), id.type().zone(system())).get().applicationPackage().trustedCertificates()); + + tester.configServer().throwOnNextPrepare(null); + tester.runner().run(); + assertEquals(succeeded, tester.jobs().run(id).get().stepStatuses().get(Step.deployTester)); + assertEquals(succeeded, tester.jobs().run(id).get().stepStatuses().get(Step.deployReal)); + + List<X509Certificate> newTrusted = new ArrayList<>(DeploymentContext.publicApplicationPackage().trustedCertificates()); + X509Certificate newCert = tester.jobs().run(id).get().testerCertificate().get(); + newTrusted.add(newCert); + assertEquals(newTrusted, tester.configServer().application(app.instanceId(), id.type().zone(system())).get().applicationPackage().trustedCertificates()); + assertNotEquals(oldCert, newCert); + } + + @Test public void certificateTimeoutAbortsJob() { tester.controllerTester().zoneRegistry().setSystemName(SystemName.Public); var zones = List.of(ZoneApiMock.fromId("test.aws-us-east-1c"), diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index fbe591a6433..407ab2432a5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -63,6 +63,10 @@ import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.BooleanSupplier; +import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.Supplier; import java.util.logging.Level; import java.util.stream.Collectors; @@ -94,7 +98,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer private List<ProtonMetrics> protonMetrics; private Version lastPrepareVersion = null; - private RuntimeException prepareException = null; + private Consumer<ApplicationId> prepareException = null; private ConfigChangeActions configChangeActions = null; private Supplier<String> log = () -> "INFO - All good"; @@ -206,9 +210,14 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer return Optional.ofNullable(lastPrepareVersion); } + /** Sets a function that may throw, determined by app id. */ + public void throwOnPrepare(Consumer<ApplicationId> prepareThrower) { + this.prepareException = prepareThrower; + } + /** The exception to throw on the next prepare run, or null to continue normally */ public void throwOnNextPrepare(RuntimeException prepareException) { - this.prepareException = prepareException; + this.prepareException = prepareException == null ? null : id -> { this.prepareException = null; throw prepareException; }; } /** Set version for an application in a given zone */ @@ -367,11 +376,10 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer @Override public PreparedApplication deploy(DeploymentData deployment) { lastPrepareVersion = deployment.platform(); - if (prepareException != null) { - RuntimeException prepareException = this.prepareException; - this.prepareException = null; - throw prepareException; - } + if (prepareException != null) + prepareException.accept(ApplicationId.from(deployment.instance().tenant(), + deployment.instance().application(), + deployment.instance().instance())); DeploymentId id = new DeploymentId(deployment.instance(), deployment.zone()); applications.put(id, new Application(id.applicationId(), lastPrepareVersion, new ApplicationPackage(deployment.applicationPackage()))); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java index f677cc079cc..023c5671b60 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java @@ -131,8 +131,8 @@ public class JobRunnerTest { Map<Step, Status> steps = run.get().stepStatuses(); runner.maintain(); assertEquals(steps, run.get().stepStatuses()); - assertEquals(List.of(deployTester), run.get().readySteps()); - assertStepsWithStartTime(run.get(), deployTester); + assertEquals(List.of(deployTester, deployReal), run.get().readySteps()); + assertStepsWithStartTime(run.get(), deployTester, deployReal); outcomes.put(deployTester, running); runner.maintain(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-job.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-job.json index 3234333c092..77998d91f20 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-job.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-job.json @@ -63,7 +63,7 @@ "deactivateTester": "unfinished", "report": "unfinished" }, - "tasks": {}, + "tasks": {"deploy": "running"}, "log": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/job/system-test/run/2" } } |