From 27c867d6c67999ef8b10e8a1cb27833a182469b6 Mon Sep 17 00:00:00 2001 From: jonmv Date: Tue, 16 May 2023 20:09:32 +0200 Subject: Consider failing upgrade tests when guessing, too --- .../vespa/hosted/controller/deployment/DeploymentStatus.java | 11 +++++++---- .../vespa/hosted/controller/maintenance/UpgraderTest.java | 5 ++++- 2 files changed, 11 insertions(+), 5 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java index 91ece6733e1..ac896338643 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java @@ -645,10 +645,16 @@ public class DeploymentStatus { Optional platformReadyAt = step.dependenciesCompletedAt(change.withoutApplication(), Optional.of(job)); Optional revisionReadyAt = step.dependenciesCompletedAt(change.withoutPlatform(), Optional.of(job)); + boolean failingUpgradeOnlyTests = ! jobs().type(systemTest(job.type()), stagingTest(job.type())) + .failingHardOn(Versions.from(change.withoutApplication(), application, deploymentFor(job), () -> systemVersion)) + .isEmpty(); + // If neither change is ready, we guess based on the specified rollout. if (platformReadyAt.isEmpty() && revisionReadyAt.isEmpty()) { return switch (rollout) { - case separate -> List.of(change.withoutApplication(), change); // Platform should stay ahead. + case separate -> ! failingUpgradeOnlyTests + ? List.of(change.withoutApplication(), change) // Platform should stay ahead ... + : List.of(change); // ... unless upgrade-only is failing tests. case leading -> List.of(change); // They should eventually join. case simultaneous -> List.of(change.withoutPlatform(), change); // Revision should get ahead. }; @@ -663,9 +669,6 @@ public class DeploymentStatus { // Both changes are ready for this step, and we look to the specified rollout to decide. boolean platformReadyFirst = platformReadyAt.get().isBefore(revisionReadyAt.get()); boolean revisionReadyFirst = revisionReadyAt.get().isBefore(platformReadyAt.get()); - boolean failingUpgradeOnlyTests = ! jobs().type(systemTest(job.type()), stagingTest(job.type())) - .failingHardOn(Versions.from(change.withoutApplication(), application, deploymentFor(job), () -> systemVersion)) - .isEmpty(); return switch (rollout) { case separate -> // Let whichever change rolled out first, keep rolling first, unless upgrade alone is failing. (platformReadyFirst || platformReadyAt.get().equals(Instant.EPOCH)) // Assume platform was first if no jobs have run yet. 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 96c1d7c545d..f1e8697cf41 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 @@ -170,7 +170,10 @@ public class UpgraderTest { // --- Failing application is repaired by changing the application, causing confidence to move above 'high' threshold // Deploy application change default0.submit(applicationPackage("default")); - default0.deploy(); + default0.runJob(systemTest) + .jobAborted(stagingTest) // New revision causes run with failing upgrade alone to be aborted. + .runJob(stagingTest) + .deploy(); tester.controllerTester().computeVersionStatus(); assertEquals(VespaVersion.Confidence.high, tester.controller().readVersionStatus().systemVersion().get().confidence()); -- cgit v1.2.3