diff options
author | jonmv <venstad@gmail.com> | 2022-09-12 15:58:29 +0200 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2022-09-12 15:58:29 +0200 |
commit | 9b3d41620d9f728d38f9e32084a14f411e1856fc (patch) | |
tree | 961b9e50c8668132788989e32bcef43bf7607c42 /controller-server | |
parent | 2d3a688ae54547c3bae0dc2b79ccbb2a438e5338 (diff) |
Add legacy confidence, which lets upgrade move apps across majors
Diffstat (limited to 'controller-server')
8 files changed, 58 insertions, 27 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java index 626937dc6ac..2441da19b90 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java @@ -11,6 +11,9 @@ import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus; import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatusList; +import com.yahoo.vespa.hosted.controller.versions.VersionStatus; +import com.yahoo.vespa.hosted.controller.versions.VespaVersion; +import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence; import java.time.Instant; import java.util.Collection; @@ -18,6 +21,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.OptionalInt; import java.util.function.Function; import static java.util.Comparator.comparing; @@ -52,17 +56,25 @@ public class InstanceList extends AbstractFilteringList<ApplicationId, InstanceL } /** - * Returns the subset of instances whose application have a deployment on the given major, or specify it in deployment spec. + * Returns the subset of instances whose application have a deployment on the given major, + * or specify it in deployment spec, + * or which are on a {@link VespaVersion.Confidence#legacy} platform, and do not specify that in deployment spec. * * @param targetMajorVersion the target major version which applications returned allows upgrading to */ - public InstanceList allowingMajorVersion(int targetMajorVersion) { + public InstanceList allowingMajorVersion(int targetMajorVersion, VersionStatus versions) { return matching(id -> { Application application = application(id); - return application.deploymentSpec().majorVersion().map(allowed -> targetMajorVersion <= allowed) - .orElseGet(() -> application.productionDeployments().values().stream() - .flatMap(List::stream) - .anyMatch(deployment -> targetMajorVersion <= deployment.version().getMajor())); + Optional<Integer> majorVersion = application.deploymentSpec().majorVersion(); + if (majorVersion.isPresent()) + return majorVersion.get() >= targetMajorVersion; + + for (List<Deployment> deployments : application.productionDeployments().values()) + for (Deployment deployment : deployments) { + if (deployment.version().getMajor() >= targetMajorVersion) return true; + if (versions.version(deployment.version()).confidence() == Confidence.legacy) return true; + } + return false; }); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java index c1e7f890d09..789b7137830 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java @@ -54,9 +54,9 @@ public class DeploymentUpgrader extends ControllerMaintainer { Versions target = new Versions(targetPlatform, last.versions().targetRevision(), Optional.of(last.versions().targetPlatform()), Optional.of(last.versions().targetRevision())); if ( ! last.hasEnded()) continue; if (application.revisions().get(last.versions().targetRevision()).compileVersion() - .map(version -> controller().applications().versionCompatibility(instance.id()).refuse(version, target.targetPlatform())) - .orElse(false)) continue; - // TODO jonmv: respect major-version from deployment spec o_O + .map(version -> controller().applications().versionCompatibility(instance.id()).refuse(version, target.targetPlatform())) + .orElse(false)) continue; + // TODO jonmv: respect major-version from deployment spec o_O Probably add it to ApplicationVersion. if ( ! deployment.version().isBefore(target.targetPlatform())) continue; if ( ! isLikelyNightFor(job)) continue; 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 f3c82e72ae3..037dacfcac9 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 @@ -107,7 +107,7 @@ public class Upgrader extends ControllerMaintainer { Map<ApplicationId, Version> targets = new LinkedHashMap<>(); for (Version version : DeploymentStatus.targetsForPolicy(versionStatus, controller().systemVersion(versionStatus), policy)) { targetAndNewer.add(version); - InstanceList eligible = eligibleForVersion(remaining, version); + InstanceList eligible = eligibleForVersion(remaining, version, versionStatus); InstanceList outdated = cancellationCriterion.apply(eligible); cancelUpgradesOf(outdated.upgrading(), "Upgrading to outdated versions"); @@ -134,10 +134,10 @@ public class Upgrader extends ControllerMaintainer { } } - private InstanceList eligibleForVersion(InstanceList instances, Version version) { + private InstanceList eligibleForVersion(InstanceList instances, Version version, VersionStatus versionStatus) { Change change = Change.of(version); return instances.not().failingOn(version) - .allowingMajorVersion(version.getMajor()) + .allowingMajorVersion(version.getMajor(), versionStatus) .compatibleWithPlatform(version, controller().applications()::versionCompatibility) .not().hasCompleted(change) // Avoid rescheduling change for instances without production steps. .onLowerVersionThan(version) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java index 71b8f1cd9b7..420fed9d5d5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java @@ -13,6 +13,7 @@ import java.util.logging.Level; import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.aborted; import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.broken; import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.high; +import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.legacy; import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.low; import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.normal; @@ -47,14 +48,15 @@ public class VersionStatusUpdater extends ControllerMaintainer { } static SystemMonitor.Confidence convert(VespaVersion.Confidence confidence) { - switch (confidence) { - case aborted: return aborted; - case broken: return broken; - case low: return low; - case normal: return normal; - case high: return high; - default: throw new IllegalArgumentException("Unexpected confidence '" + confidence + "'"); - } + return switch (confidence) { + case aborted -> aborted; + case broken -> broken; + case low -> low; + case legacy -> legacy; + case normal -> normal; + case high -> high; + default -> throw new IllegalArgumentException("Unexpected confidence '" + confidence + "'"); + }; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java index 77b2182934e..db9b67636f3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java @@ -225,14 +225,14 @@ public record VersionStatus(List<VespaVersion> versions) { if (!confidenceIsOverridden) { // Always compute confidence for system and controller if (isSystemVersion || isControllerVersion) { - confidence = VespaVersion.confidenceFrom(statistics, controller); + confidence = VespaVersion.confidenceFrom(statistics, controller, versionStatus); } else { // This is an older version, so we preserve the existing confidence, if any confidence = versionStatus.versions().stream() .filter(v -> statistics.version().equals(v.versionNumber())) .map(VespaVersion::confidence) .findFirst() - .orElseGet(() -> VespaVersion.confidenceFrom(statistics, controller)); + .orElseGet(() -> VespaVersion.confidenceFrom(statistics, controller, versionStatus)); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java index 4cb10aa6ba9..0814e9ea6ec 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java @@ -29,11 +29,11 @@ public record VespaVersion(Version version, List<NodeVersion> nodeVersions, Confidence confidence) implements Comparable<VespaVersion> { - public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller) { + public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller, VersionStatus versionStatus) { int thisMajorVersion = statistics.version().getMajor(); InstanceList all = InstanceList.from(controller.jobController().deploymentStatuses(ApplicationList.from(controller.applications().asList()) .withProductionDeployment())) - .allowingMajorVersion(thisMajorVersion); + .allowingMajorVersion(thisMajorVersion, versionStatus); // 'production on this': All production deployment jobs upgrading to this version have completed without failure InstanceList productionOnThis = all.matching(instance -> statistics.productionSuccesses().stream().anyMatch(run -> run.id().application().equals(instance))) .not().failingUpgrade() @@ -123,6 +123,9 @@ public record VespaVersion(Version version, /** We don't have sufficient evidence that this version is working */ low, + + /** This version works, but we want users to stop using it */ + legacy, /** We have sufficient evidence that this version is working */ normal, 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 044d9402629..9edcc5de312 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 @@ -2286,11 +2286,13 @@ public class DeploymentTriggerTest { Version version2 = new Version("6.3"); assertEquals(version0, tester.newDeploymentContext("t", "a1", "default").submit().deploy().application().oldestDeployedPlatform().get()); + // A new version, with normal confidence, is the default for a new app. tester.controllerTester().upgradeSystem(version1); tester.upgrader().overrideConfidence(version1, Confidence.normal); tester.controllerTester().computeVersionStatus(); assertEquals(version1, tester.newDeploymentContext("t", "a2", "default").submit().deploy().application().oldestDeployedPlatform().get()); + // A newer version has broken confidence, leaving the previous version as the default. tester.controllerTester().upgradeSystem(version2); tester.upgrader().overrideConfidence(version2, Confidence.broken); tester.controllerTester().computeVersionStatus(); @@ -2307,7 +2309,7 @@ public class DeploymentTriggerTest { } dev1.assertNotRunning(JobType.dev("us-east-1")); - tester.controllerTester().upgradeSystem(version2); + // Normal confidence lets the newest version be the default again. tester.upgrader().overrideConfidence(version2, Confidence.normal); tester.controllerTester().computeVersionStatus(); assertEquals(version2, tester.newDeploymentContext("t", "a4", "default").submit().deploy().application().oldestDeployedPlatform().get()); 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 76f0cacb7c5..a97e2918483 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 @@ -921,8 +921,20 @@ public class UpgraderTest { instance -> instance.withChange(instance.change().withoutPin())))); tester.upgrader().maintain(); assertEquals(version1, - app1.instance().change().platform().orElseThrow(), - "Application upgrades to latest allowed major"); + app1.instance().change().platform().orElseThrow(), + "Application upgrades to latest allowed major"); + + // Version on old major becomes legacy, so app upgrades once it does not specify the old major in deployment spec. + app1.runJob(systemTest).runJob(stagingTest).runJob(productionUsWest1).runJob(productionUsEast3); + app1.submit(new ApplicationPackageBuilder().majorVersion(6).region("us-east-3").region("us-west-1").build()).deploy(); + tester.upgrader().maintain(); + assertEquals(Change.empty(), app1.instance().change()); + + app1.submit(new ApplicationPackageBuilder().region("us-east-3").region("us-west-1").build()).deploy(); + tester.upgrader().overrideConfidence(version1, Confidence.legacy); + tester.controllerTester().computeVersionStatus(); + tester.upgrader().maintain(); + assertEquals(Change.of(version2), app1.instance().change()); } @Test |