From 3771a46705c7baed4fc65ad2e7ffa22ee05e9435 Mon Sep 17 00:00:00 2001 From: jonmv Date: Fri, 2 Sep 2022 12:21:44 +0200 Subject: Consider only dep.spec. variables when computing compile version --- .../hosted/controller/ApplicationController.java | 83 ++++++++++++++-------- .../controller/application/InstanceList.java | 2 +- .../controller/deployment/DeploymentStatus.java | 2 +- .../vespa/hosted/controller/ControllerTest.java | 75 ++++++++++++++----- 4 files changed, 112 insertions(+), 50 deletions(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 78063a383dc..6ec1c0cb899 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -3,7 +3,9 @@ package com.yahoo.vespa.hosted.controller; import com.yahoo.component.Version; import com.yahoo.component.VersionCompatibility; +import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; +import com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy; import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ApplicationId; @@ -57,6 +59,7 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackageValid import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade; import com.yahoo.vespa.hosted.controller.certificate.EndpointCertificates; import com.yahoo.vespa.hosted.controller.concurrent.Once; +import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.deployment.JobStatus; import com.yahoo.vespa.hosted.controller.deployment.Run; @@ -71,6 +74,7 @@ import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; import com.yahoo.vespa.hosted.controller.tenant.Tenant; 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 com.yahoo.yolean.Exceptions; import java.security.Principal; @@ -99,7 +103,10 @@ import java.util.stream.Collectors; import static com.yahoo.vespa.flags.FetchVector.Dimension.APPLICATION_ID; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.active; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.reserved; +import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence.broken; +import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence.high; import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence.low; +import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence.normal; import static java.util.Comparator.naturalOrder; import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; @@ -304,56 +311,69 @@ public class ApplicationController { } /** Returns the oldest Vespa version installed on any active or reserved production node for the given application. */ - public Version oldestInstalledPlatform(TenantAndApplicationId id) { - return controller.jobController().deploymentStatus(requireApplication(id)).jobs() - .production().asList().stream() + public Optional oldestInstalledPlatform(Application application) { + return controller.jobController().deploymentStatus(application).jobs() + .production().asList() + .stream() .map(this::oldestInstalledPlatform) .flatMap(Optional::stream) - .min(naturalOrder()) - .orElse(controller.readSystemVersion()); + .min(naturalOrder()); } /** - * Returns the preferred Vespa version to compile against, for the given application, on an optionally restricted major. - * - * A target major may be specified as an argument; or it may be set specifically for the application; - * or in general, for all applications; the first such specification wins. - * + * Returns the preferred Vespa version to compile against, for the given application, optionally restricted to a major. + *

* The returned version is not newer than the oldest deployed platform for the application, unless * the target major differs from the oldest deployed platform, in which case it is not newer than * the oldest available platform version on that major instead. - * + *

* The returned version is compatible with a platform version available in the system. - * + *

* A candidate is sought first among versions with non-broken confidence, then among those with forgotten confidence. - * + *

* The returned version is the latest in the relevant candidate set. - * + *

* If no such version exists, an {@link IllegalArgumentException} is thrown. */ public Version compileVersion(TenantAndApplicationId id, OptionalInt wantedMajor) { + // todo jonmv: kill secondary overrides + // allow non-existent apps + // require confidence as usual // Read version status, and pick out target platforms we could run the compiled package on. - OptionalInt targetMajor = firstNonEmpty(wantedMajor, requireApplication(id).majorVersion(), targetMajorVersion()); + Optional application = getApplication(id); + Optional oldestInstalledPlatform = application.flatMap(this::oldestInstalledPlatform); VersionStatus versionStatus = controller.readVersionStatus(); - Version systemVersion = controller.systemVersion(versionStatus); - Version oldestInstalledPlatform = oldestInstalledPlatform(id); + UpgradePolicy policy = application.flatMap(app -> app.deploymentSpec().instances().stream() + .map(DeploymentInstanceSpec::upgradePolicy) + .max(naturalOrder())) + .orElse(UpgradePolicy.defaultPolicy); + Confidence targetConfidence = switch (policy) { + case canary -> broken; + case defaultPolicy -> normal; + case conservative -> high; + }; // Target platforms are all versions not older than the oldest installed platform, unless forcing a major version change. - // Only major version specified in deployment spec is enough to force a downgrade, while all sources may force an upgrade. - Predicate isTargetPlatform = targetMajor.isEmpty() - || targetMajor.getAsInt() == oldestInstalledPlatform.getMajor() - || wantedMajor.isEmpty() && targetMajor.getAsInt() <= oldestInstalledPlatform.getMajor() - ? version -> ! version.isBefore(oldestInstalledPlatform) - : version -> targetMajor.getAsInt() == version.getMajor(); - Set platformVersions = versionStatus.versions().stream() + // Only platforms not older than the system version, and with appropriate confidence, are considered targets. + Predicate isTargetPlatform = wantedMajor.isEmpty() && oldestInstalledPlatform.isEmpty() + ? __ -> true + : wantedMajor.isEmpty() || wantedMajor.getAsInt() == oldestInstalledPlatform.get().getMajor() + ? version -> ! version.isBefore(oldestInstalledPlatform.get()) + : version -> wantedMajor.getAsInt() == version.getMajor(); + Set platformVersions = versionStatus.deployableVersions().stream() + .filter(version -> version.confidence().equalOrHigherThan(targetConfidence)) .map(VespaVersion::versionNumber) - .filter(version -> ! version.isAfter(systemVersion)) .filter(isTargetPlatform) .collect(toSet()); + oldestInstalledPlatform.ifPresent(fallback -> { + if (wantedMajor.isEmpty() || wantedMajor.getAsInt() == fallback.getMajor()) + platformVersions.add(fallback); + }); + if (platformVersions.isEmpty()) throw new IllegalArgumentException("this system has no available versions" + - (targetMajor.isPresent() ? " on specified major: " + targetMajor.getAsInt() : "")); + (wantedMajor.isPresent() ? " on specified major: " + wantedMajor.getAsInt() : "")); // The returned compile version must be compatible with at least one target platform. // If it is incompatible with any of the current platforms, the system will trigger a platform change. @@ -361,10 +381,11 @@ public class ApplicationController { // and the oldest current platform, unless the two are incompatible, in which case only the target matters. VersionCompatibility compatibility = versionCompatibility(id.defaultInstance()); // Wrong id level >_< Version oldestTargetPlatform = platformVersions.stream().min(naturalOrder()).get(); - Version newestVersion = compatibility.accept(oldestInstalledPlatform, oldestTargetPlatform) - && oldestInstalledPlatform.isBefore(oldestTargetPlatform) - ? oldestInstalledPlatform - : oldestTargetPlatform; + Version newestVersion = oldestInstalledPlatform.isPresent() + && compatibility.accept(oldestInstalledPlatform.get(), oldestTargetPlatform) + && oldestInstalledPlatform.get().isBefore(oldestTargetPlatform) + ? oldestInstalledPlatform.get() + : oldestTargetPlatform; Predicate systemCompatible = version -> ! version.isAfter(newestVersion) && platformVersions.stream().anyMatch(platform -> compatibility.accept(platform, version)); @@ -386,7 +407,7 @@ public class ApplicationController { if (unknown.isPresent()) return unknown.get(); throw new IllegalArgumentException("no suitable, released compile version exists" + - (targetMajor.isPresent() ? " for specified major: " + targetMajor.getAsInt() : "")); + (wantedMajor.isPresent() ? " for specified major: " + wantedMajor.getAsInt() : "")); } private OptionalInt firstNonEmpty(OptionalInt... choices) { 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 153ce87ff6b..5f2a4121489 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 @@ -52,7 +52,7 @@ public class InstanceList extends AbstractFilteringList ! compatibleWithCompileVersion.test(deployment.version()))) { return targetsForPolicy(versionStatus, systemVersion, application.deploymentSpec().requireInstance(instance).upgradePolicy()) - .stream() // Pick the latest platform which is compatible with the compile version, and is ready for this instance. + .stream() // Pick the latest platform with appropriate confidence, which is compatible with the compile version. .filter(compatibleWithCompileVersion) .findFirst() .map(platform -> change.withoutPin().with(platform)) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 6c193e9b539..ff0502a2d1f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -49,6 +49,7 @@ import com.yahoo.vespa.hosted.controller.routing.RoutingStatus; import com.yahoo.vespa.hosted.controller.routing.context.DeploymentRoutingContext; import com.yahoo.vespa.hosted.controller.routing.rotation.RotationId; import com.yahoo.vespa.hosted.controller.routing.rotation.RotationLock; +import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence; import com.yahoo.vespa.hosted.rotation.config.RotationsConfig; import org.junit.jupiter.api.Test; @@ -75,6 +76,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -1130,12 +1132,16 @@ public class ControllerTest { // No deployments result in system version Version version0 = Version.fromString("7.1"); tester.controllerTester().upgradeSystem(version0); + tester.upgrader().overrideConfidence(version0, Confidence.normal); + tester.controllerTester().computeVersionStatus(); assertEquals(version0, tester.applications().compileVersion(application, OptionalInt.empty())); context.submit(applicationPackage).deploy(); // System is upgraded Version version1 = Version.fromString("7.2"); tester.controllerTester().upgradeSystem(version1); + tester.upgrader().overrideConfidence(version1, Confidence.normal); + tester.controllerTester().computeVersionStatus(); assertEquals(version0, tester.applications().compileVersion(application, OptionalInt.empty())); // Application is upgraded and compile version is bumped @@ -1143,9 +1149,21 @@ public class ControllerTest { context.deployPlatform(version1); assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.empty())); + DeploymentContext legacyApp = tester.newDeploymentContext("avoid", "gc", "default").submit().deploy(); + // A new major is released to the system Version version2 = Version.fromString("8.0"); tester.controllerTester().upgradeSystem(version2); + tester.upgrader().overrideConfidence(version2, Confidence.low); + tester.controllerTester().computeVersionStatus(); + assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.empty())); + assertEquals("this system has no available versions on specified major: 8", + assertThrows(IllegalArgumentException.class, + () -> tester.applications().compileVersion(application, OptionalInt.of(8))) + .getMessage()); + + tester.upgrader().overrideConfidence(version2, Confidence.normal); + tester.controllerTester().computeVersionStatus(); assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.empty())); assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(8))); @@ -1153,37 +1171,60 @@ public class ControllerTest { tester.controllerTester().flagSource().withListFlag(PermanentFlags.INCOMPATIBLE_VERSIONS.id(), List.of("8"), String.class); assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.of(8))); - // Default major version is set to 8. - tester.applications().setTargetMajorVersion(OptionalInt.of(8)); + // The only version on major 8 has low confidence. + tester.upgrader().overrideConfidence(version2, Confidence.low); + tester.controllerTester().computeVersionStatus(); + assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(7))); + assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.empty())); + assertEquals("this system has no available versions on specified major: 8", + assertThrows(IllegalArgumentException.class, + () -> tester.applications().compileVersion(application, OptionalInt.of(8))) + .getMessage()); + + // Version on major 8 has normal confidence. + tester.upgrader().overrideConfidence(version2, Confidence.normal); + tester.controllerTester().computeVersionStatus(); assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(7))); - assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.empty())); - - // Application sets target major to 7. - tester.applications().lockApplicationOrThrow(application, locked -> tester.applications().store(locked.withMajorVersion(7))); assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.empty())); assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.of(8))); - // Application sets target major to 8. - tester.applications().lockApplicationOrThrow(application, locked -> tester.applications().store(locked.withMajorVersion(8))); + // Application upgrades to major 8; major version from deployment spec should cause a downgrade. + context.submit(new ApplicationPackageBuilder().region("us-west-1").compileVersion(version2).build()).deploy(); assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(7))); assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.empty())); - // Application upgrades to major 8; only major version from deployment spec should cause a downgrade. - context.submit(new ApplicationPackageBuilder().region("us-west-1").compileVersion(version2).build()).deploy(); - tester.applications().setTargetMajorVersion(OptionalInt.empty()); - tester.applications().lockApplicationOrThrow(application, locked -> tester.applications().store(locked.withMajorVersion(null))); + // Reduced confidence should not cause a downgrade. + tester.upgrader().overrideConfidence(version2, Confidence.low); + tester.controllerTester().computeVersionStatus(); assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(7))); assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.empty())); + assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.of(8))); - // Default major version across all apps should not cause a downgrade. - tester.applications().setTargetMajorVersion(OptionalInt.of(7)); + // All versions on new major having broken confidence makes it all fail for upgraded apps, but this shouldn't happen in practice. + tester.upgrader().overrideConfidence(version2, Confidence.broken); + tester.controllerTester().computeVersionStatus(); assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(7))); - assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.empty())); + assertEquals("no suitable, released compile version exists", + assertThrows(IllegalArgumentException.class, + () -> tester.applications().compileVersion(application, OptionalInt.empty())) + .getMessage()); + assertEquals("no suitable, released compile version exists for specified major: 8", + assertThrows(IllegalArgumentException.class, + () -> tester.applications().compileVersion(application, OptionalInt.of(8))) + .getMessage()); + + // Major versions are not incompatible anymore, so the old compile version should work again. + tester.controllerTester().flagSource().withListFlag(PermanentFlags.INCOMPATIBLE_VERSIONS.id(), List.of(), String.class); + assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(7))); + assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.empty())); + assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(8))); - // Default major version for specific app should not cause a downgrade. - tester.applications().lockApplicationOrThrow(application, locked -> tester.applications().store(locked.withMajorVersion(7))); + // Simply reduced confidence shouldn't cause any changes. + tester.upgrader().overrideConfidence(version2, Confidence.low); + tester.controllerTester().computeVersionStatus(); assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(7))); assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.empty())); + assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.of(8))); } @Test -- cgit v1.2.3 From 5d6bf0289b55a3c7d5640903f611bbc1bd364870 Mon Sep 17 00:00:00 2001 From: jonmv Date: Fri, 2 Sep 2022 13:20:12 +0200 Subject: Remove curator-backed platform target overrides --- .../hosted/controller/ApplicationController.java | 30 +---- .../controller/application/InstanceList.java | 15 ++- .../hosted/controller/maintenance/Upgrader.java | 24 +--- .../hosted/controller/persistence/CuratorDb.java | 11 -- .../restapi/controller/ControllerApiHandler.java | 4 - .../restapi/controller/UpgraderResponse.java | 1 - .../hosted/controller/versions/VespaVersion.java | 3 +- .../vespa/hosted/controller/ControllerTest.java | 12 +- .../deployment/DeploymentTriggerTest.java | 50 ++++---- .../maintenance/MetricsReporterTest.java | 6 +- .../controller/maintenance/UpgraderTest.java | 135 +++++---------------- .../restapi/controller/ControllerApiTest.java | 12 -- .../controller/versions/VersionStatusTest.java | 1 - 13 files changed, 89 insertions(+), 215 deletions(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 6ec1c0cb899..047c059e9a6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -238,16 +238,6 @@ public class ApplicationController { return curator.readApplications(false); } - /** Returns the target major version for applications not specifying one */ - public OptionalInt targetMajorVersion() { - return curator.readTargetMajorVersion().map(OptionalInt::of).orElse(OptionalInt.empty()); - } - - /** Sets the default target major version. Set to empty to determine target version normally (by confidence) */ - public void setTargetMajorVersion(OptionalInt targetMajorVersion) { - curator.writeTargetMajorVersion(targetMajorVersion); - } - /** * Returns a snapshot of all readable applications. Unlike {@link ApplicationController#asList()} this ignores * applications that cannot currently be read (e.g. due to serialization issues) and may return an incomplete @@ -337,9 +327,6 @@ public class ApplicationController { */ public Version compileVersion(TenantAndApplicationId id, OptionalInt wantedMajor) { - // todo jonmv: kill secondary overrides - // allow non-existent apps - // require confidence as usual // Read version status, and pick out target platforms we could run the compiled package on. Optional application = getApplication(id); Optional oldestInstalledPlatform = application.flatMap(this::oldestInstalledPlatform); @@ -378,11 +365,13 @@ public class ApplicationController { // The returned compile version must be compatible with at least one target platform. // If it is incompatible with any of the current platforms, the system will trigger a platform change. // The returned compile version should also be at least as old as both the oldest target platform version, - // and the oldest current platform, unless the two are incompatible, in which case only the target matters. + // and the oldest current platform, unless the two are incompatible, in which case only the target matters, + // or there are no installed platforms, in which case we prefer the newest target platform. VersionCompatibility compatibility = versionCompatibility(id.defaultInstance()); // Wrong id level >_< Version oldestTargetPlatform = platformVersions.stream().min(naturalOrder()).get(); - Version newestVersion = oldestInstalledPlatform.isPresent() - && compatibility.accept(oldestInstalledPlatform.get(), oldestTargetPlatform) + Version newestVersion = oldestInstalledPlatform.isEmpty() + ? platformVersions.stream().max(naturalOrder()).get() + : compatibility.accept(oldestInstalledPlatform.get(), oldestTargetPlatform) && oldestInstalledPlatform.get().isBefore(oldestTargetPlatform) ? oldestInstalledPlatform.get() : oldestTargetPlatform; @@ -987,15 +976,6 @@ public class ApplicationController { throw new IllegalArgumentException("Not allowed to launch Athenz service " + athenzService.getFullName()); } - /** Returns the latest known version within the given major, which is not newer than the system version. */ - public Optional lastCompatibleVersion(int targetMajorVersion) { - VersionStatus versions = controller.readVersionStatus(); - return versions.deployableVersions().stream() - .map(VespaVersion::versionNumber) - .filter(version -> version.getMajor() == targetMajorVersion) - .max(naturalOrder()); - } - /** Extract deployment warnings metric from deployment result */ private static Map warningsFrom(ActivateResult result) { if (result.prepareResponse().log == null) return Map.of(); 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 5f2a4121489..626937dc6ac 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 @@ -52,15 +52,18 @@ public class InstanceList extends AbstractFilteringList targetMajorVersion <= application(id).deploymentSpec().majorVersion() - .orElse(application(id).majorVersion() - .orElse(defaultMajorVersion))); + public InstanceList allowingMajorVersion(int targetMajorVersion) { + 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())); + }); } /** Returns the subset of instances that are allowed to upgrade to the given version at the given time */ 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 ce98643c25a..f3c82e72ae3 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 @@ -61,17 +61,16 @@ public class Upgrader extends ControllerMaintainer { VersionStatus versionStatus = controller().readVersionStatus(); cancelBrokenUpgrades(versionStatus); - OptionalInt targetMajorVersion = targetMajorVersion(); DeploymentStatusList deploymentStatuses = deploymentStatuses(versionStatus); for (UpgradePolicy policy : UpgradePolicy.values()) - updateTargets(versionStatus, deploymentStatuses, policy, targetMajorVersion); + updateTargets(versionStatus, deploymentStatuses, policy); return 1.0; } private DeploymentStatusList deploymentStatuses(VersionStatus versionStatus) { return controller().jobController().deploymentStatuses(ApplicationList.from(controller().applications().readable()) - .withProjectId(), + .withProjectId(), versionStatus); } @@ -94,7 +93,7 @@ public class Upgrader extends ControllerMaintainer { } } - private void updateTargets(VersionStatus versionStatus, DeploymentStatusList deploymentStatuses, UpgradePolicy policy, OptionalInt targetMajorVersion) { + private void updateTargets(VersionStatus versionStatus, DeploymentStatusList deploymentStatuses, UpgradePolicy policy) { InstanceList instances = instances(deploymentStatuses); InstanceList remaining = instances.with(policy); Instant failureThreshold = controller().clock().instant().minus(DeploymentTrigger.maxFailingRevisionTime); @@ -108,7 +107,7 @@ public class Upgrader extends ControllerMaintainer { Map targets = new LinkedHashMap<>(); for (Version version : DeploymentStatus.targetsForPolicy(versionStatus, controller().systemVersion(versionStatus), policy)) { targetAndNewer.add(version); - InstanceList eligible = eligibleForVersion(remaining, version, targetMajorVersion); + InstanceList eligible = eligibleForVersion(remaining, version); InstanceList outdated = cancellationCriterion.apply(eligible); cancelUpgradesOf(outdated.upgrading(), "Upgrading to outdated versions"); @@ -135,11 +134,10 @@ public class Upgrader extends ControllerMaintainer { } } - private InstanceList eligibleForVersion(InstanceList instances, Version version, - OptionalInt targetMajorVersion) { + private InstanceList eligibleForVersion(InstanceList instances, Version version) { Change change = Change.of(version); return instances.not().failingOn(version) - .allowingMajorVersion(version.getMajor(), targetMajorVersion.orElse(version.getMajor())) + .allowingMajorVersion(version.getMajor()) .compatibleWithPlatform(version, controller().applications()::versionCompatibility) .not().hasCompleted(change) // Avoid rescheduling change for instances without production steps. .onLowerVersionThan(version) @@ -182,16 +180,6 @@ public class Upgrader extends ControllerMaintainer { curator.writeUpgradesPerMinute(n); } - /** Returns the target major version for applications not specifying one */ - public OptionalInt targetMajorVersion() { - return controller().applications().targetMajorVersion(); - } - - /** Sets the default target major version. Set to empty to determine target version normally (by confidence) */ - public void setTargetMajorVersion(OptionalInt targetMajorVersion) { - controller().applications().setTargetMajorVersion(targetMajorVersion); - } - /** Override confidence for given version. This will cause the computed confidence to be ignored */ public void overrideConfidence(Version version, Confidence confidence) { if (confidence == Confidence.aborted && !version.isAfter(controller().readSystemVersion())) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 54e98877ba3..e7425c05372 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -269,17 +269,6 @@ public class CuratorDb { curator.set(upgradesPerMinutePath(), ByteBuffer.allocate(Double.BYTES).putDouble(n).array()); } - public Optional readTargetMajorVersion() { - return read(targetMajorVersionPath(), ByteBuffer::wrap).map(ByteBuffer::getInt); - } - - public void writeTargetMajorVersion(OptionalInt targetMajorVersion) { - if (targetMajorVersion.isPresent()) - curator.set(targetMajorVersionPath(), ByteBuffer.allocate(Integer.BYTES).putInt(targetMajorVersion.getAsInt()).array()); - else - curator.delete(targetMajorVersionPath()); - } - public void writeVersionStatus(VersionStatus status) { curator.set(versionStatusPath(), asJson(versionStatusSerializer.toSlime(status))); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java index 9278d030db6..2b4c6411386 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java @@ -156,7 +156,6 @@ public class ControllerApiHandler extends AuditLoggingRequestHandler { private HttpResponse configureUpgrader(HttpRequest request) { String upgradesPerMinuteField = "upgradesPerMinute"; - String targetMajorVersionField = "targetMajorVersion"; byte[] jsonBytes = toJsonBytes(request.getData()); Inspector inspect = SlimeUtils.jsonToSlime(jsonBytes).get(); @@ -164,9 +163,6 @@ public class ControllerApiHandler extends AuditLoggingRequestHandler { if (inspect.field(upgradesPerMinuteField).valid()) { upgrader.setUpgradesPerMinute(inspect.field(upgradesPerMinuteField).asDouble()); - } else if (inspect.field(targetMajorVersionField).valid()) { - int target = (int) inspect.field(targetMajorVersionField).asLong(); - upgrader.setTargetMajorVersion(target == 0 ? OptionalInt.empty() : OptionalInt.of(target)); // 0 is the default value } else { return ErrorResponse.badRequest("No such modifiable field(s)"); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/UpgraderResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/UpgraderResponse.java index 122ea94ab6c..28fa5183e5c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/UpgraderResponse.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/UpgraderResponse.java @@ -19,7 +19,6 @@ public class UpgraderResponse extends SlimeJsonResponse { Slime slime = new Slime(); Cursor root = slime.setObject(); root.setDouble("upgradesPerMinute", upgrader.upgradesPerMinute()); - upgrader.targetMajorVersion().ifPresent(v -> root.setLong("targetMajorVersion", v)); Cursor array = root.setArray("confidenceOverrides"); upgrader.confidenceOverrides().forEach((version, confidence) -> { 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 e078df0267f..4cb10aa6ba9 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 @@ -31,10 +31,9 @@ public record VespaVersion(Version version, public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller) { int thisMajorVersion = statistics.version().getMajor(); - int defaultMajorVersion = controller.applications().targetMajorVersion().orElse(thisMajorVersion); InstanceList all = InstanceList.from(controller.jobController().deploymentStatuses(ApplicationList.from(controller.applications().asList()) .withProductionDeployment())) - .allowingMajorVersion(thisMajorVersion, defaultMajorVersion); + .allowingMajorVersion(thisMajorVersion); // '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() diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index ff0502a2d1f..4c49b0c5c0c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -1150,6 +1150,7 @@ public class ControllerTest { assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.empty())); DeploymentContext legacyApp = tester.newDeploymentContext("avoid", "gc", "default").submit().deploy(); + TenantAndApplicationId newApp = TenantAndApplicationId.from("new", "app"); // A new major is released to the system Version version2 = Version.fromString("8.0"); @@ -1166,10 +1167,14 @@ public class ControllerTest { tester.controllerTester().computeVersionStatus(); assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.empty())); assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(8))); + assertEquals(version2, tester.applications().compileVersion(newApp, OptionalInt.empty())); // The new major is marked as incompatible with older compile versions tester.controllerTester().flagSource().withListFlag(PermanentFlags.INCOMPATIBLE_VERSIONS.id(), List.of("8"), String.class); + assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(7))); + assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.empty())); assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.of(8))); + assertEquals(version2, tester.applications().compileVersion(newApp, OptionalInt.empty())); // The only version on major 8 has low confidence. tester.upgrader().overrideConfidence(version2, Confidence.low); @@ -1180,13 +1185,12 @@ public class ControllerTest { assertThrows(IllegalArgumentException.class, () -> tester.applications().compileVersion(application, OptionalInt.of(8))) .getMessage()); + assertEquals(version1, tester.applications().compileVersion(newApp, OptionalInt.empty())); + assertEquals(version1, tester.applications().compileVersion(newApp, OptionalInt.empty())); - // Version on major 8 has normal confidence. + // Version on major 8 has normal confidence again tester.upgrader().overrideConfidence(version2, Confidence.normal); tester.controllerTester().computeVersionStatus(); - assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.of(7))); - assertEquals(version1, tester.applications().compileVersion(application, OptionalInt.empty())); - assertEquals(version2, tester.applications().compileVersion(application, OptionalInt.of(8))); // Application upgrades to major 8; major version from deployment spec should cause a downgrade. context.submit(new ApplicationPackageBuilder().region("us-west-1").compileVersion(version2).build()).deploy(); 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 fda1f5f0b77..ced1f96b387 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 @@ -164,7 +164,7 @@ public class DeploymentTriggerTest { app.submit(applicationPackage).deploy(); - Change upgrade = Change.of(new Version("7.8.9")); + Change upgrade = Change.of(new Version("6.8.9")); tester.controllerTester().upgradeSystem(upgrade.platform().get()); tester.upgrader().maintain(); app.runJob(systemTest).runJob(stagingTest); @@ -209,7 +209,7 @@ public class DeploymentTriggerTest { app.runJob(productionUsCentral1).runJob(productionUsWest1).runJob(productionUsEast3); assertEquals(Change.empty(), app.instance().change()); - tester.controllerTester().upgradeSystem(new Version("8.9")); + tester.controllerTester().upgradeSystem(new Version("6.9")); tester.upgrader().maintain(); app.runJob(systemTest).runJob(stagingTest); tester.clock().advance(Duration.ofMinutes(1)); @@ -545,7 +545,7 @@ public class DeploymentTriggerTest { .region("us-east-3") .build(); var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); - tester.controllerTester().upgradeSystem(new Version("9.8.7")); + tester.controllerTester().upgradeSystem(new Version("6.8.7")); tester.upgrader().maintain(); tester.deploymentTrigger().pauseJob(app.instanceId(), productionUsWest1, @@ -1118,9 +1118,8 @@ public class DeploymentTriggerTest { DeploymentContext alpha = tester.newDeploymentContext("t", "a", "alpha"); DeploymentContext beta = tester.newDeploymentContext("t", "a", "beta"); alpha.submit(applicationPackage).deploy(); - Optional revision1 = alpha.lastSubmission(); - Version version1 = new Version("7.1"); + Version version1 = new Version("6.2"); tester.controllerTester().upgradeSystem(version1); tester.upgrader().run(); alpha.runJob(systemTest).runJob(stagingTest); @@ -1204,7 +1203,7 @@ public class DeploymentTriggerTest { // Application starts upgrade, but is confidence is broken cancelled after first zone. Tests won't run. Version version0 = app.application().oldestDeployedPlatform().get(); - Version version1 = Version.fromString("7.7"); + Version version1 = Version.fromString("6.7"); tester.controllerTester().upgradeSystem(version1); tester.upgrader().maintain(); @@ -1385,7 +1384,7 @@ public class DeploymentTriggerTest { assertEquals(List.of(), tester.jobs().active()); tester.atMondayMorning().clock().advance(Duration.ofDays(5)); // Inside revision block window for second, conservative instance. - Version version = Version.fromString("8.1"); + Version version = Version.fromString("6.2"); tester.controllerTester().upgradeSystem(version); tester.upgrader().maintain(); assertEquals(Change.of(version), app1.instance().change()); @@ -1495,7 +1494,7 @@ public class DeploymentTriggerTest { // Platform rolls through first production zone. var version0 = tester.controller().readSystemVersion(); - var version1 = new Version("7.1"); + var version1 = new Version("6.2"); tester.controllerTester().upgradeSystem(version1); tester.upgrader().maintain(); app.runJob(systemTest).runJob(stagingTest).runJob(productionUsCentral1); @@ -1529,7 +1528,7 @@ public class DeploymentTriggerTest { assertEquals(Change.empty(), app.instance().change()); // New upgrade fails in staging-test, and revision to fix it is submitted. - var version2 = new Version("7.2"); + var version2 = new Version("6.3"); tester.controllerTester().upgradeSystem(version2); tester.upgrader().maintain(); app.runJob(systemTest).failDeployment(stagingTest); @@ -1561,7 +1560,7 @@ public class DeploymentTriggerTest { // Platform rolls through first production zone. var version0 = tester.controller().readSystemVersion(); - var version1 = new Version("7.1"); + var version1 = new Version("6.2"); tester.controllerTester().upgradeSystem(version1); tester.upgrader().maintain(); app.runJob(systemTest).runJob(stagingTest).runJob(productionUsEast3); @@ -1611,7 +1610,7 @@ public class DeploymentTriggerTest { // Platform rolls through first production zone. var version0 = tester.controller().readSystemVersion(); - var version1 = new Version("7.1"); + var version1 = new Version("6.2"); tester.controllerTester().upgradeSystem(version1); tester.upgrader().maintain(); app.runJob(systemTest).runJob(stagingTest).runJob(productionUsEast3); @@ -1837,8 +1836,8 @@ public class DeploymentTriggerTest { // A version releases, but when the first upgrade has gotten through alpha, beta, and gamma, a newer version has high confidence. var version0 = tester.controller().readSystemVersion(); - var version1 = new Version("7.1"); - var version2 = new Version("7.2"); + var version1 = new Version("6.2"); + var version2 = new Version("6.3"); tester.controllerTester().upgradeSystem(version1); tester.upgrader().maintain(); @@ -1869,7 +1868,7 @@ public class DeploymentTriggerTest { // Platform rolls through first production zone. var version0 = tester.controller().readSystemVersion(); - var version1 = new Version("7.1"); + var version1 = new Version("6.2"); tester.controllerTester().upgradeSystem(version1); tester.upgrader().maintain(); app.runJob(systemTest).runJob(stagingTest).runJob(productionUsCentral1); @@ -1914,7 +1913,7 @@ public class DeploymentTriggerTest { // Platform rolls through first production zone. var version0 = tester.controller().readSystemVersion(); - var version1 = new Version("7.1"); + var version1 = new Version("6.2"); tester.controllerTester().upgradeSystem(version1); tester.upgrader().maintain(); app.runJob(systemTest).runJob(stagingTest).runJob(productionUsCentral1); @@ -1979,7 +1978,7 @@ public class DeploymentTriggerTest { // Manually deploy to east again, then upgrade the system. app.runJob(productionUsEast3, cdPackage); - var version = new Version("7.1"); + var version = new Version("6.2"); tester.controllerTester().upgradeSystem(version); tester.upgrader().maintain(); // System and staging tests both require unknown versions, and are broken. @@ -2039,11 +2038,11 @@ public class DeploymentTriggerTest { conservative.runJob(productionEuWest1) .runJob(testEuWest1); - tester.controllerTester().upgradeSystem(new Version("7.7.7")); + tester.controllerTester().upgradeSystem(new Version("6.7.7")); tester.upgrader().maintain(); canary.runJob(systemTest) - .runJob(stagingTest); + .runJob(stagingTest); tester.upgrader().maintain(); conservative.runJob(productionEuWest1) .runJob(testEuWest1); @@ -2055,7 +2054,7 @@ public class DeploymentTriggerTest { var app = tester.newDeploymentContext().submit().deploy(); // Start upgrade, then receive new submission. - Version version1 = new Version("7.8.9"); + Version version1 = new Version("6.8.9"); RevisionId build1 = app.lastSubmission().get(); tester.controllerTester().upgradeSystem(version1); tester.upgrader().maintain(); @@ -2104,8 +2103,8 @@ public class DeploymentTriggerTest { var app = tester.newDeploymentContext().submit(applicationPackage).deploy(); var appToAvoidVersionGC = tester.newDeploymentContext("g", "c", "default").submit().deploy(); - Version version2 = new Version("7.8.9"); - Version version3 = new Version("8.9.10"); + Version version2 = new Version("6.8.9"); + Version version3 = new Version("6.9.10"); tester.controllerTester().upgradeSystem(version2); tester.deploymentTrigger().forceChange(appToAvoidVersionGC.instanceId(), Change.of(version2)); appToAvoidVersionGC.deployPlatform(version2); @@ -2256,10 +2255,9 @@ public class DeploymentTriggerTest { ApplicationPackage appPackage = ApplicationPackageBuilder.fromDeploymentXml(spec); DeploymentContext tests = tester.newDeploymentContext("tenant", "application", "tests"); DeploymentContext main = tester.newDeploymentContext("tenant", "application", "main"); - Version version1 = new Version("7"); + Version version1 = new Version("6.2"); tester.controllerTester().upgradeSystem(version1); tests.submit(appPackage); - Optional revision1 = tests.lastSubmission(); JobId systemTestJob = new JobId(tests.instanceId(), systemTest); JobId stagingTestJob = new JobId(tests.instanceId(), stagingTest); JobId mainJob = new JobId(main.instanceId(), productionUsEast3); @@ -2286,10 +2284,10 @@ public class DeploymentTriggerTest { assertEquals(Set.of(), tests.deploymentStatus().jobsToRun().keySet()); // Versions 2 and 3 become available. - // Tests instance fails on 2, then update to 3. + // Tests instance fails on 2, then updates to 3. // Version 2 should not be a target for either instance. // Version 2 should also not be possible to set as a forced target for the tests instance. - Version version2 = new Version("8"); + Version version2 = new Version("6.3"); tester.controllerTester().upgradeSystem(version2); tester.upgrader().run(); tester.triggerJobs(); @@ -2299,7 +2297,7 @@ public class DeploymentTriggerTest { assertEquals(Set.of(systemTestJob), tests.deploymentStatus().jobsToRun().keySet()); assertEquals(2, tests.deploymentStatus().jobsToRun().get(systemTestJob).size()); - Version version3 = new Version("9"); + Version version3 = new Version("6.4"); tester.controllerTester().upgradeSystem(version3); tests.runJob(systemTest) // Success in default cloud. .failDeployment(systemTest); // Failure in centauri cloud. diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java index b41c17fcd33..ee6cc11b192 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java @@ -218,11 +218,11 @@ public class MetricsReporterTest { assertFalse(context.instance().change().hasTargets(), "Change deployed"); // New versions is released and upgrade fails in test environments - Version version = Version.fromString("7.1"); + Version version = Version.fromString("6.2"); tester.controllerTester().upgradeSystem(version); tester.upgrader().maintain(); context.failDeployment(systemTest) - .failDeployment(stagingTest); + .failDeployment(stagingTest); reporter.maintain(); assertEquals(2, getDeploymentsFailingUpgrade(context.instanceId())); @@ -529,7 +529,7 @@ public class MetricsReporterTest { context.submit(pkg).completeRollout(); // System is upgraded, triggering upgrade of application - tester.controllerTester().upgradeSystem(Version.fromString("7.0")); + tester.controllerTester().upgradeSystem(Version.fromString("6.2")); tester.upgrader().maintain(); // Start production job for upgrade, without completing it 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 b24b6c8e99b..76f0cacb7c5 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 @@ -16,6 +16,7 @@ import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.deployment.Run; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; +import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence; import org.junit.jupiter.api.Test; import java.time.Duration; @@ -663,116 +664,49 @@ public class UpgraderTest { Version version = Version.fromString("6.2"); tester.controllerTester().upgradeSystem(version); - ApplicationPackage version6ApplicationPackage = new ApplicationPackageBuilder().majorVersion(6) - .region("us-west-1") - .build(); + ApplicationPackageBuilder builder = new ApplicationPackageBuilder().region("us-west-1").majorVersion(7); + ApplicationPackage defaultPackage = new ApplicationPackageBuilder().region("us-west-1").build(); // Setup applications - var canary0 = createAndDeploy("canary0", "canary"); - var default0 = tester.newDeploymentContext().submit(version6ApplicationPackage).deploy(); + var canaryApp = tester.newDeploymentContext("canary", "app", "default").submit(builder.upgradePolicy("canary").build()).deploy(); + var defaultApp = tester.newDeploymentContext("normal", "app", "default").submit(builder.upgradePolicy("default").build()).deploy(); + var conservativeApp = tester.newDeploymentContext("conservative", "app", "default").submit(builder.upgradePolicy("conservative").build()).deploy(); + var lazyApp = tester.newDeploymentContext().submit(defaultPackage).deploy(); - // New major version is released + // New major version is released; more apps upgrade with increasing confidence. version = Version.fromString("7.0"); tester.controllerTester().upgradeSystem(version); - tester.upgrader().maintain(); - assertEquals(version, tester.controller().readVersionStatus().systemVersion().get().versionNumber()); - tester.triggerJobs(); - - // ... canary upgrade to it - assertEquals(2, tester.jobs().active().size()); - canary0.deployPlatform(version); - assertEquals(0, tester.jobs().active().size()); + tester.upgrader().overrideConfidence(version, Confidence.broken); tester.controllerTester().computeVersionStatus(); - - // The other application does not because it has pinned to major version 6 - tester.upgrader().maintain(); - tester.triggerJobs(); - assertEquals(0, tester.jobs().active().size()); - } - - @Test - void testPinningMajorVersionInApplication() { - Version version = Version.fromString("6.2"); - tester.controllerTester().upgradeSystem(version); - - // Setup applications - var canary0 = createAndDeploy("canary", "canary"); - var default0 = tester.newDeploymentContext().submit().deploy(); - tester.applications().lockApplicationOrThrow(default0.application().id(), - a -> tester.applications().store(a.withMajorVersion(6))); - assertEquals(OptionalInt.of(6), default0.application().majorVersion()); - - // New major version is released - version = Version.fromString("7.0"); - tester.controllerTester().upgradeSystem(version); - assertEquals(version, tester.controller().readVersionStatus().systemVersion().get().versionNumber()); tester.upgrader().maintain(); - tester.triggerJobs(); + assertEquals(Change.of(version), canaryApp.instance().change()); + assertEquals(Change.empty(), defaultApp.instance().change()); + assertEquals(Change.empty(), conservativeApp.instance().change()); + assertEquals(Change.empty(), lazyApp.instance().change()); - // ... canary upgrade to it - assertEquals(2, tester.jobs().active().size()); - canary0.deployPlatform(version); - assertEquals(0, tester.jobs().active().size()); + tester.upgrader().overrideConfidence(version, Confidence.low); tester.controllerTester().computeVersionStatus(); - - // The other application does not because it has pinned to major version 6 tester.upgrader().maintain(); - tester.triggerJobs(); - assertEquals(0, tester.jobs().active().size()); - } - - @Test - void testPinningMajorVersionInUpgrader() { - Version version = Version.fromString("6.2"); - tester.controllerTester().upgradeSystem(version); - - ApplicationPackage version7CanaryApplicationPackage = new ApplicationPackageBuilder() - .majorVersion(7) - .upgradePolicy("canary") - .region("us-west-1") - .build(); - ApplicationPackage version7DefaultApplicationPackage = new ApplicationPackageBuilder() - .majorVersion(7) - .upgradePolicy("default") - .region("us-west-1") - .build(); - - // Setup applications - var canary0 = tester.newDeploymentContext("tenant1", "canary0", "default").submit(version7CanaryApplicationPackage).deploy(); - var default0 = tester.newDeploymentContext("tenant1", "default0", "default").submit(version7DefaultApplicationPackage).deploy(); - var default1 = tester.newDeploymentContext("tenant1", "default1", "default").submit(DeploymentContext.applicationPackage()).deploy(); + assertEquals(Change.of(version), canaryApp.instance().change()); + assertEquals(Change.empty(), defaultApp.instance().change()); + assertEquals(Change.empty(), conservativeApp.instance().change()); + assertEquals(Change.empty(), lazyApp.instance().change()); - // New major version is released, but we don't want to upgrade to it yet - tester.upgrader().setTargetMajorVersion(OptionalInt.of(6)); - version = Version.fromString("7.0"); - tester.controllerTester().upgradeSystem(version); - assertEquals(version, tester.controller().readVersionStatus().systemVersion().get().versionNumber()); - tester.upgrader().maintain(); - tester.triggerJobs(); - - // ... canary upgrade to it because it explicitly wants 7 - assertEquals(2, tester.jobs().active().size()); - canary0.deployPlatform(version); - assertEquals(0, tester.jobs().active().size()); + tester.upgrader().overrideConfidence(version, Confidence.normal); tester.controllerTester().computeVersionStatus(); - - // default0 upgrades, but not default1 tester.upgrader().maintain(); - tester.triggerJobs(); - assertEquals(2, tester.jobs().active().size()); - default0.deployPlatform(version); + assertEquals(Change.of(version), canaryApp.instance().change()); + assertEquals(Change.of(version), defaultApp.instance().change()); + assertEquals(Change.empty(), conservativeApp.instance().change()); + assertEquals(Change.empty(), lazyApp.instance().change()); - // Nothing more happens ... - tester.upgrader().maintain(); - tester.triggerJobs(); - assertEquals(0, tester.jobs().active().size()); - - // Now we want to upgrade the latest application - tester.upgrader().setTargetMajorVersion(OptionalInt.empty()); + tester.upgrader().overrideConfidence(version, Confidence.high); + tester.controllerTester().computeVersionStatus(); tester.upgrader().maintain(); - tester.triggerJobs(); - assertEquals(2, tester.jobs().active().size()); - default1.deployPlatform(version); + assertEquals(Change.of(version), canaryApp.instance().change()); + assertEquals(Change.of(version), defaultApp.instance().change()); + assertEquals(Change.of(version), conservativeApp.instance().change()); + assertEquals(Change.empty(), lazyApp.instance().change()); } @Test @@ -955,9 +889,6 @@ public class UpgraderTest { Version version0 = Version.fromString("6.1"); tester.controllerTester().upgradeSystem(version0); - // Apps target 6 by default - tester.upgrader().setTargetMajorVersion(OptionalInt.of(6)); - // All applications deploy on current version var app1 = createAndDeploy("app1", "default"); var app2 = createAndDeploy("app2", "default"); @@ -979,10 +910,10 @@ public class UpgraderTest { Version version2 = Version.fromString("7.1"); tester.controllerTester().upgradeSystem(version2); - // App 2 is allowed on new major and upgrades - tester.controller().applications().lockApplicationIfPresent(app2.application().id(), app -> tester.applications().store(app.withMajorVersion(7))); + // App 2 has upgrade to new platform triggered + tester.deploymentTrigger().forceChange(app2.instanceId(), Change.of(version2)); tester.upgrader().maintain(); - assertEquals(version2, app2.instance().change().platform().orElseThrow()); + assertEquals(Change.of(version2), app2.instance().change()); // App 1 is unpinned and upgrades to latest 6 tester.controller().applications().lockApplicationIfPresent(app1.application().id(), app -> @@ -1004,7 +935,7 @@ public class UpgraderTest { app.failDeployment(systemTest); // New version is not targeted. - Version version1 = new Version("7"); + Version version1 = new Version("6.2"); tester.controllerTester().upgradeSystem(version1); assertEquals(Change.of(revision1.get()), app.instance().change()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java index fa652af18f3..9148ad36d46 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java @@ -93,18 +93,6 @@ public class ControllerApiTest extends ControllerContainerTest { "{\"upgradesPerMinute\":42.0,\"confidenceOverrides\":[]}", 200); - // Set target major version - tester.assertResponse( - operatorRequest("http://localhost:8080/controller/v1/jobs/upgrader", "{\"targetMajorVersion\":6}", Request.Method.PATCH), - "{\"upgradesPerMinute\":42.0,\"targetMajorVersion\":6,\"confidenceOverrides\":[]}", - 200); - - // Clear target major version - tester.assertResponse( - operatorRequest("http://localhost:8080/controller/v1/jobs/upgrader", "{\"targetMajorVersion\":null}", Request.Method.PATCH), - "{\"upgradesPerMinute\":42.0,\"confidenceOverrides\":[]}", - 200); - // Override confidence tester.assertResponse( operatorRequest("http://localhost:8080/controller/v1/jobs/upgrader/confidence/6.42", "broken", Request.Method.POST), diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java index 18776be2dce..6cb7c059b1d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java @@ -384,7 +384,6 @@ public class VersionStatusTest { // A new major version is released and all canaries upgrade Version version4 = new Version("7.1"); - tester.controller().applications().setTargetMajorVersion(OptionalInt.of(version3.getMajor())); // Previous remains the default tester.controllerTester().upgradeSystem(version4); tester.upgrader().maintain(); tester.triggerJobs(); -- cgit v1.2.3 From 3863be5fc10aadef1bcb42bf001d89dc339c77dd Mon Sep 17 00:00:00 2001 From: jonmv Date: Mon, 5 Sep 2022 11:58:47 +0200 Subject: Fix javadoc, and remove unused path --- .../src/main/java/com/yahoo/vespa/hosted/controller/Application.java | 2 +- .../java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) 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 bdb68f655ff..e3a0ad7cb84 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 @@ -173,7 +173,7 @@ public class Application { /** * Returns the oldest platform version this has deployed in a permanent zone (not test or staging). * - * This is unfortunately quite similar to {@link ApplicationController#oldestInstalledPlatform(TenantAndApplicationId)}, + * This is unfortunately quite similar to {@link ApplicationController#oldestInstalledPlatform(Application)}, * but this checks only what the controller has deployed to the production zones, while that checks the node repository * to see what's actually installed on each node. Thus, this is the right choice for, e.g., target Vespa versions for * new deployments, while that is the right choice for version to compile against. diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index e7425c05372..38cea6f8cef 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -686,10 +686,6 @@ public class CuratorDb { return root.append("upgrader").append("upgradesPerMinute"); } - private static Path targetMajorVersionPath() { - return root.append("upgrader").append("targetMajorVersion"); - } - private static Path confidenceOverridesPath() { return root.append("upgrader").append("confidenceOverrides"); } -- cgit v1.2.3