Log or fail config server startup when upgrading between 2 versions too far apart

Fail on hosted, log on self serve Vespa

4 files changed, 137 insertions, 26 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java
index 65e2b35f954..3defd48e9b4 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java
@@ -59,7 +59,7 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
 
     private final ApplicationRepository applicationRepository;
     private final RpcServer server;
-    private final VersionState versionState;
+    protected final VersionState versionState;
     private final StateMonitor stateMonitor;
     private final VipStatus vipStatus;
     private final ConfigserverConfig configserverConfig;
@@ -133,7 +133,7 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
                         + versionState.currentVersion() + ". Redeploying all applications");
             try {
                 redeployAllApplications();
-                versionState.saveNewVersion();
+                versionState.storeCurrentVersion();
                 log.log(Level.FINE, "All applications redeployed successfully");
             } catch (Exception e) {
                 log.log(Level.SEVERE, "Redeployment of applications failed", e);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/version/VersionState.java b/configserver/src/main/java/com/yahoo/vespa/config/server/version/VersionState.java
index 589d121481a..61c55641d23 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/version/VersionState.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/version/VersionState.java
@@ -1,9 +1,9 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server.version;
 
-import com.yahoo.component.annotation.Inject;
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.component.Version;
+import com.yahoo.component.annotation.Inject;
 import com.yahoo.io.IOUtils;
 import com.yahoo.path.Path;
 import com.yahoo.text.Utf8;
@@ -15,6 +15,9 @@ import java.io.FileReader;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.util.Optional;
+import java.util.logging.Logger;
+
+import static java.util.logging.Level.WARNING;
 
 /**
  *
@@ -22,33 +25,62 @@ import java.util.Optional;
  * data in ZooKeeper if distributeApplicationPackage and data found in ZooKeeper)
  *
  * @author Ulf Lilleengen
+ * @author hmusum
  */
 public class VersionState {
 
+    private static final Logger log = Logger.getLogger(VersionState.class.getName());
+    private static final int allowedMinorVersionInterval = 30; // (2 months of releases => ~30 releases)
+    private static final Version latestVersionOnPreviousMajor = Version.fromString("7.594.36");
     static final Path versionPath = Path.fromString("/config/v2/vespa_version");
 
     private final File versionFile;
     private final Curator curator;
+    private final Version currentVersion;
+    private final boolean throwIfUpgradeBetweenVersionsTooFarApart;
 
     @Inject
     public VersionState(ConfigserverConfig config, Curator curator) {
-        this(new File(Defaults.getDefaults().underVespaHome(config.configServerDBDir()), "vespa_version"), curator);
+        this(new File(Defaults.getDefaults().underVespaHome(config.configServerDBDir()), "vespa_version"),
+             curator,
+             config.hostedVespa());
     }
 
-    public VersionState(File versionFile, Curator curator) {
+    public VersionState(File versionFile, Curator curator, boolean throwIfUpgradeBetweenVersionsTooFarApart) {
+        this(versionFile,
+             curator,
+             new Version(VespaVersion.major, VespaVersion.minor, VespaVersion.micro),
+             throwIfUpgradeBetweenVersionsTooFarApart);
+    }
+
+    public VersionState(File versionFile,
+                        Curator curator,
+                        Version currentVersion,
+                        boolean throwIfUpgradeBetweenVersionsTooFarApart) {
         this.versionFile = versionFile;
         this.curator = curator;
+        this.currentVersion = currentVersion;
+        this.throwIfUpgradeBetweenVersionsTooFarApart = throwIfUpgradeBetweenVersionsTooFarApart;
     }
 
     public boolean isUpgraded() {
-        return currentVersion().compareTo(storedVersion()) > 0;
+        Version storedVersion = storedVersion();
+        if (storedVersion.equals(Version.emptyVersion)) return true;
+
+        // TODO: Also verify version for downgrades?
+        if (currentVersion().compareTo(storedVersion) > 0) {
+            verifyVersionIntervalForUpgrade(storedVersion);
+            return true;
+        } else {
+            return false;
+        }
     }
 
-    public void saveNewVersion() {
-        saveNewVersion(currentVersion().toFullString());
+    public void storeCurrentVersion() {
+        storeVersion(currentVersion().toFullString());
     }
 
-    public void saveNewVersion(String vespaVersion) {
+    public void storeVersion(String vespaVersion) {
         curator.set(versionPath, Utf8.toBytes(vespaVersion));
         try (FileWriter writer = new FileWriter(versionFile)) {
             writer.write(vespaVersion);
@@ -74,7 +106,7 @@ public class VersionState {
     }
 
     public Version currentVersion() {
-        return new Version(VespaVersion.major, VespaVersion.minor, VespaVersion.micro);
+        return currentVersion;
     }
 
     File versionFile() {
@@ -86,4 +118,32 @@ public class VersionState {
         return String.format("Current version:%s, stored version:%s", currentVersion(), storedVersion());
     }
 
+    private void verifyVersionIntervalForUpgrade(Version storedVersion) {
+        int storedVersionMajor = storedVersion.getMajor();
+        int storedVersionMinor = storedVersion.getMinor();
+        int currentVersionMajor = currentVersion.getMajor();
+        int currentVersionMinor = currentVersion.getMinor();
+
+        boolean sameMajor = storedVersionMajor == currentVersionMajor;
+        boolean differentMajor = !sameMajor;
+        if (storedVersionMajor < latestVersionOnPreviousMajor.getMajor())
+            logOrThrow("Cannot upgrade from " + storedVersion + " to " + currentVersion() +
+                               " (upgrade across 2 major versions not supported). Please upgrade to " + latestVersionOnPreviousMajor.toFullString() + " first.");
+        else if (sameMajor && (currentVersionMinor - storedVersionMinor > allowedMinorVersionInterval))
+            logOrThrow("Cannot upgrade from " + storedVersion + " to " + currentVersion() +
+                               ". Please upgrade to an intermediate version first, the interval between the two versions is too large.");
+        else if (differentMajor && storedVersionMinor < latestVersionOnPreviousMajor.getMinor())
+            logOrThrow("Cannot upgrade directly from " + storedVersion + " to " + currentVersion() + " (new major version). Please upgrade to " + latestVersionOnPreviousMajor.toFullString() + " first.");
+        else if (differentMajor && currentVersionMinor > allowedMinorVersionInterval)
+            logOrThrow("Cannot upgrade from " + storedVersion + " to " + currentVersion() +
+                               ". Please upgrade to an intermediate version first, the interval between the two versions is too large.");
+    }
+
+    private void logOrThrow(String message) {
+        if (throwIfUpgradeBetweenVersionsTooFarApart)
+            throw new RuntimeException(message);
+        else
+            log.log(WARNING, message);
+    }
+
 }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
index 2351706659a..df0682b5156 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
@@ -23,17 +23,18 @@ import com.yahoo.vespa.config.server.deploy.DeployTester;
 import com.yahoo.vespa.config.server.filedistribution.FileDirectory;
 import com.yahoo.vespa.config.server.rpc.RpcServer;
 import com.yahoo.vespa.config.server.version.VersionState;
+import com.yahoo.vespa.config.server.version.VespaVersion;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Paths;
 import java.time.Duration;
 import java.time.Instant;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
@@ -47,6 +48,7 @@ import static com.yahoo.vespa.config.server.deploy.DeployTester.createHostedMode
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 /**
  * @author Ulf Lilleengen
@@ -70,6 +72,7 @@ public class ConfigServerBootstrapTest {
         // Take a host away so that there are too few for the application, to verify we can still bootstrap
         provisioner.allocations().values().iterator().next().remove(0);
         Bootstrapper bootstrap = createBootstrapper(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY);
+        assertTrue(bootstrap.isUpgraded());
         assertEquals(List.of("ApplicationPackageMaintainer", "TenantsMaintainer"),
                      bootstrap.configServerMaintenance().maintainers().stream()
                               .map(Maintainer::name)
@@ -105,6 +108,7 @@ public class ConfigServerBootstrapTest {
 
         RpcServer rpcServer = createRpcServer(configserverConfig);
         Bootstrapper bootstrap = createBootstrapper(tester, rpcServer, VIP_STATUS_FILE);
+        assertTrue(bootstrap.isUpgraded());
         assertTrue(bootstrap.vipStatus().isInRotation()); // default is in rotation when using status file
 
         bootstrap.doStart();
@@ -130,6 +134,7 @@ public class ConfigServerBootstrapTest {
 
         RpcServer rpcServer = createRpcServer(configserverConfig);
         Bootstrapper bootstrap = createBootstrapper(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY);
+        assertTrue(bootstrap.isUpgraded());
         assertFalse(bootstrap.vipStatus().isInRotation());
         // Call method directly, to be sure that it is finished redeploying all applications and we can check status
         bootstrap.doStart();
@@ -143,8 +148,6 @@ public class ConfigServerBootstrapTest {
         bootstrap.deconstruct();
     }
 
-    // Tests that we do not try to create the config model version stored in zookeeper when not on hosted vespa, since
-    // we are then only able to create the latest version
     @Test
     public void testBootstrapNonHostedOneConfigModel() throws Exception {
         ConfigserverConfig configserverConfig = createConfigserverConfigNonHosted(temporaryFolder);
@@ -168,6 +171,7 @@ public class ConfigServerBootstrapTest {
 
         RpcServer rpcServer = createRpcServer(configserverConfig);
         Bootstrapper bootstrap = createBootstrapper(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY);
+        assertTrue(bootstrap.isUpgraded());
         bootstrap.doStart();
         waitUntil(rpcServer::isRunning, "failed waiting for Rpc server running");
         assertTrue(rpcServer.isServingConfigRequests());
@@ -175,9 +179,52 @@ public class ConfigServerBootstrapTest {
         waitUntil(() -> bootstrap.vipStatus().isInRotation(), "failed waiting for server to be in rotation");
     }
 
+    @Test
+    public void testBootstrapBetweenVersions() throws Exception {
+        ConfigserverConfig configserverConfig = createConfigserverConfigNonHosted(temporaryFolder);
+        String oldVespaVersion = "8.100.1";
+        Curator curator = new MockCurator();
+        DeployTester tester = new DeployTester.Builder(temporaryFolder)
+                .modelFactory(DeployTester.createModelFactory(Version.fromString(oldVespaVersion)))
+                .configserverConfig(configserverConfig)
+                .curator(curator)
+                .build();
+        RpcServer rpcServer = createRpcServer(configserverConfig);
+
+        // Upgrade between two versions not too far apart, should work
+        Version versionToUpgradeTo = Version.fromString("8.110.1");
+        Bootstrapper bootstrap = createBootstrapper(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY, versionToUpgradeTo);
+        bootstrap.versionState().storeVersion("8.100.1");
+        bootstrap.doStart();
+
+        assertUpgradeFails("8.100.1", "8.131.1", tester, rpcServer,
+                           "Cannot upgrade from 8.100.1 to 8.131.1. Please upgrade to an intermediate version first, the interval between the two versions is too large.");
+        assertUpgradeFails("7.99.1", "8.100.1", tester, rpcServer,
+                           "Cannot upgrade directly from 7.99.1 to 8.100.1 (new major version). Please upgrade to 7.594.36 first.");
+        assertUpgradeFails("7.594.36", "8.121.1", tester, rpcServer,
+                           "Cannot upgrade from 7.594.36 to 8.121.1. Please upgrade to an intermediate version first, the interval between the two versions is too large.");
+        assertUpgradeFails("6.11.11", "8.121.1", tester, rpcServer,
+                           "Cannot upgrade from 6.11.11 to 8.121.1 (upgrade across 2 major versions not supported). Please upgrade to 7.594.36 first.");
+    }
+
+    private void assertUpgradeFails(String from, String to, DeployTester tester, RpcServer rpcServer, String expected) throws IOException {
+        Version versionToUpgradeTo = Version.fromString(to);
+        Bootstrapper bootstrap = createBootstrapper(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY, versionToUpgradeTo);
+        bootstrap.versionState().storeVersion(from);
+        try {
+            bootstrap.doStart();
+            fail("Expected bootstrap to fail");
+        } catch (RuntimeException e) {
+            assertEquals(expected, e.getMessage());
+        }
+    }
+
     private Bootstrapper createBootstrapper(DeployTester tester, RpcServer rpcServer, VipStatusMode vipStatusMode) throws IOException {
-        VersionState versionState = createVersionState(tester.curator());
-        assertTrue(versionState.isUpgraded());
+        return createBootstrapper(tester, rpcServer, vipStatusMode, new Version(VespaVersion.major, VespaVersion.minor, VespaVersion.micro));
+    }
+
+    private Bootstrapper createBootstrapper(DeployTester tester, RpcServer rpcServer, VipStatusMode vipStatusMode, Version version) throws IOException {
+        VersionState versionState = createVersionState(tester.curator(), version);
 
         StateMonitor stateMonitor = StateMonitor.createForTesting();
         VipStatus vipStatus = createVipStatus(stateMonitor);
@@ -229,7 +276,7 @@ public class ConfigServerBootstrapTest {
     }
 
     private List<Host> createHosts(String vespaVersion) {
-        return Arrays.asList(createHost("host1", vespaVersion), createHost("host2", vespaVersion), createHost("host3", vespaVersion));
+        return List.of(createHost("host1", vespaVersion), createHost("host2", vespaVersion), createHost("host3", vespaVersion));
     }
 
     private Host createHost(String hostname, String version) {
@@ -244,8 +291,8 @@ public class ConfigServerBootstrapTest {
                              new NullMetric());
     }
 
-    private VersionState createVersionState(Curator curator) throws IOException {
-        return new VersionState(temporaryFolder.newFile(), curator);
+    private VersionState createVersionState(Curator curator, Version vespaVersion) throws IOException {
+        return new VersionState(temporaryFolder.newFile(), curator, vespaVersion, true);
     }
 
     public static class MockRpcServer extends com.yahoo.vespa.config.server.rpc.MockRpcServer {
@@ -286,13 +333,17 @@ public class ConfigServerBootstrapTest {
 
         @Override
         public void start() {
-            // Do nothing, avoids bootstrapping apps in constructor, use doBootstrap() below to really bootstrap apps
+            // Do nothing, avoids bootstrapping apps in constructor, use doStart() below to really bootstrap apps
         }
 
         public void doStart() {
             super.start();
         }
 
+        public VersionState versionState() { return versionState; }
+
+        public boolean isUpgraded() { return versionState.isUpgraded(); }
+
     }
 
 }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/version/VersionStateTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/version/VersionStateTest.java
index 1f49d496f70..092243bac6d 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/version/VersionStateTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/version/VersionStateTest.java
@@ -33,14 +33,14 @@ public class VersionStateTest {
         VersionState state = createVersionState();
         assertEquals(unknownVersion, state.storedVersion());
         assertTrue(state.isUpgraded());
-        state.saveNewVersion();
+        state.storeCurrentVersion();
         assertFalse(state.isUpgraded());
 
-        state.saveNewVersion("badversion");
+        state.storeVersion("badversion");
         assertEquals(unknownVersion, state.storedVersion());
         assertTrue(state.isUpgraded());
 
-        state.saveNewVersion("5.0.0");
+        state.storeVersion("5.0.0");
         assertEquals(new Version(5, 0, 0), state.storedVersion());
         assertTrue(state.isUpgraded());
 
@@ -50,12 +50,12 @@ public class VersionStateTest {
         assertTrue(state.isUpgraded());
 
         // Save new version, remove version in file, should find version in ZooKeeper
-        state.saveNewVersion("6.0.0");
+        state.storeVersion("6.0.0");
         Files.delete(state.versionFile().toPath());
         assertEquals(new Version(6, 0, 0), state.storedVersion());
         assertTrue(state.isUpgraded());
 
-        state.saveNewVersion();
+        state.storeCurrentVersion();
         assertEquals(state.currentVersion(), state.storedVersion());
         assertFalse(state.isUpgraded());
     }
@@ -64,7 +64,7 @@ public class VersionStateTest {
     public void serverdbfile() throws IOException {
         File dbDir = tempDir.newFolder();
         VersionState state = new VersionState(new ConfigserverConfig.Builder().configServerDBDir(dbDir.getAbsolutePath()).build(), curator);
-        state.saveNewVersion();
+        state.storeCurrentVersion();
         File versionFile = new File(dbDir, "vespa_version");
         assertTrue(versionFile.exists());
         Version stored = Version.fromString(IOUtils.readFile(versionFile));
@@ -72,7 +72,7 @@ public class VersionStateTest {
     }
 
     private VersionState createVersionState() throws IOException {
-        return new VersionState(tempDir.newFile(), curator);
+        return new VersionState(tempDir.newFile(), curator, false);
     }
 
 }