From 2ab57aec0a53924fae3291d9f89af8aecc3aa8e9 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Fri, 17 Dec 2021 14:48:49 +0100 Subject: Support aborting and rolling back incomplete upgrade --- .../vespa/hosted/controller/ControllerTester.java | 2 +- .../controller/integration/NodeRepositoryMock.java | 8 +- .../maintenance/MetricsReporterTest.java | 4 +- .../controller/maintenance/OsUpgraderTest.java | 17 +++-- .../maintenance/OsVersionStatusUpdaterTest.java | 4 +- .../controller/maintenance/SystemUpgraderTest.java | 88 +++++++++++++++++++--- .../hosted/controller/restapi/os/OsApiTest.java | 4 +- 7 files changed, 103 insertions(+), 24 deletions(-) (limited to 'controller-server/src/test/java') diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index 6697ddac808..99c97b3bdd6 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -251,7 +251,7 @@ public final class ControllerTester { for (ZoneApi zone : zoneRegistry().zones().all().zones()) { for (SystemApplication application : systemApplications) { if (!application.hasApplicationPackage()) { - configServer().nodeRepository().upgrade(zone.getId(), application.nodeType(), version); + configServer().nodeRepository().upgrade(zone.getId(), application.nodeType(), version, false); } configServer().setVersion(version, application.id(), zone.getId()); configServer().convergeServices(application.id(), zone.getId()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java index ef99183ecde..a2a1b4ba0a1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java @@ -125,11 +125,17 @@ public class NodeRepositoryMock implements NodeRepository { } @Override - public void upgrade(ZoneId zone, NodeType type, Version version) { + public void upgrade(ZoneId zone, NodeType type, Version version, boolean allowDowngrade) { this.targetVersions.compute(zone, (ignored, targetVersions) -> { if (targetVersions == null) { targetVersions = TargetVersions.EMPTY; } + Optional current = targetVersions.vespaVersion(type); + if (current.isPresent() && version.isBefore(current.get()) && !allowDowngrade) { + throw new IllegalArgumentException("Changing wanted version for " + type + " in " + zone + " from " + + current.get() + " to " + version + + ", but downgrade is not allowed"); + } return targetVersions.withVespaVersion(type, version); }); // Bump wanted version of each node. This is done by InfrastructureProvisioner in a real node repository. 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 71a3ce262ad..604a42f3d19 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 @@ -295,7 +295,7 @@ public class MetricsReporterTest { var tester = new ControllerTester(); var reporter = createReporter(tester.controller()); var zone = ZoneId.from("prod.eu-west-1"); - tester.zoneRegistry().setUpgradePolicy(UpgradePolicy.create().upgrade(ZoneApiMock.from(zone))); + tester.zoneRegistry().setUpgradePolicy(UpgradePolicy.builder().upgrade(ZoneApiMock.from(zone)).build()); var systemUpgrader = new SystemUpgrader(tester.controller(), Duration.ofDays(1) ); tester.configServer().bootstrap(List.of(zone), SystemApplication.configServer); @@ -352,7 +352,7 @@ public class MetricsReporterTest { var reporter = createReporter(tester.controller()); var zone = ZoneId.from("prod.eu-west-1"); var cloud = CloudName.defaultName(); - tester.zoneRegistry().setOsUpgradePolicy(cloud, UpgradePolicy.create().upgrade(ZoneApiMock.from(zone))); + tester.zoneRegistry().setOsUpgradePolicy(cloud, UpgradePolicy.builder().upgrade(ZoneApiMock.from(zone)).build()); var osUpgrader = new OsUpgrader(tester.controller(), Duration.ofDays(1), CloudName.defaultName()); var statusUpdater = new OsVersionStatusUpdater(tester.controller(), Duration.ofDays(1) ); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java index ec1a5455413..3c3f0053e91 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java @@ -42,12 +42,13 @@ public class OsUpgraderTest { ZoneApi zone3 = zone("prod.us-central-1", cloud1); ZoneApi zone4 = zone("prod.us-east-3", cloud1); ZoneApi zone5 = zone("prod.us-north-1", cloud2); - UpgradePolicy upgradePolicy = UpgradePolicy.create() + UpgradePolicy upgradePolicy = UpgradePolicy.builder() .upgrade(zone0) .upgrade(zone1) .upgradeInParallel(zone2, zone3) .upgrade(zone5) // Belongs to a different cloud and is ignored by this upgrader - .upgrade(zone4); + .upgrade(zone4) + .build(); OsUpgrader osUpgrader = osUpgrader(upgradePolicy, cloud1, false); // Bootstrap system @@ -125,11 +126,12 @@ public class OsUpgraderTest { ZoneApi zone2 = zone("prod.us-west-1", cloud); ZoneApi zone3 = zone("prod.us-central-1", cloud); ZoneApi zone4 = zone("prod.eu-west-1", cloud); - UpgradePolicy upgradePolicy = UpgradePolicy.create() + UpgradePolicy upgradePolicy = UpgradePolicy.builder() .upgrade(zone0) .upgrade(zone1) .upgradeInParallel(zone2, zone3) - .upgrade(zone4); + .upgrade(zone4) + .build(); OsUpgrader osUpgrader = osUpgrader(upgradePolicy, cloud, true); // Bootstrap system @@ -189,9 +191,10 @@ public class OsUpgraderTest { CloudName cloud = CloudName.from("cloud"); ZoneApi zone1 = zone("dev.us-east-1", cloud); ZoneApi zone2 = zone("prod.us-west-1", cloud); - UpgradePolicy upgradePolicy = UpgradePolicy.create() + UpgradePolicy upgradePolicy = UpgradePolicy.builder() .upgrade(zone1) - .upgrade(zone2); + .upgrade(zone2) + .build(); OsUpgrader osUpgrader = osUpgrader(upgradePolicy, cloud, false); // Bootstrap system @@ -299,7 +302,7 @@ public class OsUpgraderTest { } private OsUpgrader osUpgrader(UpgradePolicy upgradePolicy, CloudName cloud, boolean reprovisionToUpgradeOs) { - var zones = upgradePolicy.asList().stream().flatMap(Collection::stream).collect(Collectors.toList()); + var zones = upgradePolicy.steps().stream().flatMap(Collection::stream).collect(Collectors.toList()); tester.zoneRegistry() .setZones(zones) .setOsUpgradePolicy(cloud, upgradePolicy); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java index f5ae6bafc65..f2d738bd2e1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java @@ -28,11 +28,11 @@ public class OsVersionStatusUpdaterTest { OsVersionStatusUpdater statusUpdater = new OsVersionStatusUpdater(tester.controller(), Duration.ofDays(1) ); // Add all zones to upgrade policy - UpgradePolicy upgradePolicy = UpgradePolicy.create(); + UpgradePolicy.Builder upgradePolicy = UpgradePolicy.builder(); for (ZoneApi zone : tester.zoneRegistry().zones().controllerUpgraded().zones()) { upgradePolicy = upgradePolicy.upgrade(zone); } - tester.zoneRegistry().setOsUpgradePolicy(CloudName.defaultName(), upgradePolicy); + tester.zoneRegistry().setOsUpgradePolicy(CloudName.defaultName(), upgradePolicy.build()); // Initially empty assertSame(OsVersionStatus.empty, tester.controller().osVersionStatus()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java index ff47b6cc231..c09d3ec3a92 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java @@ -21,6 +21,7 @@ import java.util.stream.Stream; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author mpolden @@ -37,10 +38,11 @@ public class SystemUpgraderTest { @Test public void upgrade_system() { SystemUpgrader systemUpgrader = systemUpgrader( - UpgradePolicy.create() + UpgradePolicy.builder() .upgrade(zone1) .upgradeInParallel(zone2, zone3) .upgrade(zone4) + .build() ); Version version1 = Version.fromString("6.5"); @@ -137,7 +139,7 @@ public class SystemUpgraderTest { @Test public void upgrade_controller_with_non_converging_application() { - SystemUpgrader systemUpgrader = systemUpgrader(UpgradePolicy.create().upgrade(zone1)); + SystemUpgrader systemUpgrader = systemUpgrader(UpgradePolicy.builder().upgrade(zone1).build()); // Bootstrap system tester.configServer().bootstrap(List.of(zone1.getId()), SystemApplication.configServer, @@ -173,10 +175,11 @@ public class SystemUpgraderTest { @Test public void upgrade_system_containing_host_applications() { SystemUpgrader systemUpgrader = systemUpgrader( - UpgradePolicy.create() + UpgradePolicy.builder() .upgrade(zone1) .upgradeInParallel(zone2, zone3) .upgrade(zone4) + .build() ); Version version1 = Version.fromString("6.5"); @@ -223,7 +226,7 @@ public class SystemUpgraderTest { @Test public void downgrading_controller_never_downgrades_system() { - SystemUpgrader systemUpgrader = systemUpgrader(UpgradePolicy.create().upgrade(zone1)); + SystemUpgrader systemUpgrader = systemUpgrader(UpgradePolicy.builder().upgrade(zone1).build()); Version version = Version.fromString("6.5"); tester.upgradeSystem(version); @@ -242,7 +245,7 @@ public class SystemUpgraderTest { @Test public void upgrade_halts_on_broken_version() { - SystemUpgrader systemUpgrader = systemUpgrader(UpgradePolicy.create().upgrade(zone1).upgrade(zone2)); + SystemUpgrader systemUpgrader = systemUpgrader(UpgradePolicy.builder().upgrade(zone1).upgrade(zone2).build()); // Initial system version Version version1 = Version.fromString("6.5"); @@ -267,9 +270,7 @@ public class SystemUpgraderTest { convergeServices(SystemApplication.proxy, zone1); // Confidence is reduced to broken and next zone is not scheduled for upgrade - new Upgrader(tester.controller(), Duration.ofDays(1)) - .overrideConfidence(version2, VespaVersion.Confidence.broken); - tester.computeVersionStatus(); + overrideConfidence(version2, VespaVersion.Confidence.broken); systemUpgrader.maintain(); assertWantedVersion(List.of(SystemApplication.configServerHost, SystemApplication.proxyHost, SystemApplication.configServer, SystemApplication.proxy), version1, zone2); @@ -282,7 +283,7 @@ public class SystemUpgraderTest { tester.configServer().bootstrap(List.of(zone1.getId()), applications); tester.configServer().disallowConvergenceCheck(SystemApplication.proxy.id()); tester.zoneRegistry().exclusiveRoutingIn(zone1); - var systemUpgrader = systemUpgrader(UpgradePolicy.create().upgrade(zone1)); + var systemUpgrader = systemUpgrader(UpgradePolicy.builder().upgrade(zone1).build()); // System begins upgrade var version1 = Version.fromString("6.5"); @@ -299,6 +300,75 @@ public class SystemUpgraderTest { assertEquals(version1, tester.controller().readSystemVersion()); } + @Test + public void downgrade_from_aborted_version() { + SystemUpgrader systemUpgrader = systemUpgrader(UpgradePolicy.builder().upgrade(zone1).upgrade(zone2).upgrade(zone3).build()); + + Version version1 = Version.fromString("6.5"); + tester.configServer().bootstrap(List.of(zone1.getId(), zone2.getId(), zone3.getId()), SystemApplication.notController()); + tester.upgradeSystem(version1); + systemUpgrader.maintain(); + assertCurrentVersion(SystemApplication.notController(), version1, zone1, zone2, zone3); + + // Controller upgrades + Version version2 = Version.fromString("6.6"); + tester.upgradeController(version2); + assertControllerVersion(version2); + + // 2/3 zones upgrade + for (var zone : List.of(zone1, zone2)) { + systemUpgrader.maintain(); + completeUpgrade(List.of(SystemApplication.tenantHost, + SystemApplication.proxyHost, + SystemApplication.configServerHost), + version2, zone); + completeUpgrade(SystemApplication.configServer, version2, zone); + systemUpgrader.maintain(); + completeUpgrade(SystemApplication.proxy, version2, zone); + convergeServices(SystemApplication.proxy, zone); + } + + // Upgrade is aborted + overrideConfidence(version2, VespaVersion.Confidence.aborted); + + // Dependency graph is inverted and applications without dependencies downgrade first. Upgrade policy is + // also followed in inverted order + for (var zone : List.of(zone2, zone1)) { + systemUpgrader.maintain(); + completeUpgrade(List.of(SystemApplication.tenantHost, + SystemApplication.configServerHost, + SystemApplication.proxy), + version1, zone); + convergeServices(SystemApplication.proxy, zone); + List lastToDowngrade = List.of(SystemApplication.configServer, + SystemApplication.proxyHost); + assertWantedVersion(lastToDowngrade, version2, zone); + + // ... and then configserver and proxyhost + systemUpgrader.maintain(); + completeUpgrade(lastToDowngrade, version1, zone); + } + assertSystemVersion(version1); + + // Another version is released and system upgrades + Version version3 = Version.fromString("6.7"); + tester.upgradeSystem(version3); + assertEquals(version3, tester.controller().readSystemVersion()); + + // Attempt to abort current system version is rejected + try { + overrideConfidence(version3, VespaVersion.Confidence.aborted); + fail("Expected exception"); + } catch (IllegalArgumentException ignored) {} + systemUpgrader.maintain(); + assertWantedVersion(SystemApplication.notController(), version3, zone1, zone2, zone3); + } + + private void overrideConfidence(Version version, VespaVersion.Confidence confidence) { + new Upgrader(tester.controller(), Duration.ofDays(1)).overrideConfidence(version, confidence); + tester.computeVersionStatus(); + } + /** Simulate upgrade of nodes allocated to given application. In a real system this is done by the node itself */ private void completeUpgrade(SystemApplication application, Version version, ZoneApi first, ZoneApi... rest) { assertWantedVersion(application, version, first, rest); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java index 6b6ced68b0a..7d17e97e66b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java @@ -55,8 +55,8 @@ public class OsApiTest extends ControllerContainerTest { zoneRegistryMock().setSystemName(SystemName.cd) .setZones(zone1, zone2, zone3) .reprovisionToUpgradeOsIn(zone3) - .setOsUpgradePolicy(cloud1, UpgradePolicy.create().upgrade(zone1).upgrade(zone2)) - .setOsUpgradePolicy(cloud2, UpgradePolicy.create().upgrade(zone3)); + .setOsUpgradePolicy(cloud1, UpgradePolicy.builder().upgrade(zone1).upgrade(zone2).build()) + .setOsUpgradePolicy(cloud2, UpgradePolicy.builder().upgrade(zone3).build()); osUpgraders = List.of( new OsUpgrader(tester.controller(), Duration.ofDays(1), cloud1), -- cgit v1.2.3