Merge pull request #27394 from vespa-engine/hmusum/log-or-fail-when-upgrading-config-server-between-two-versions-too-far-apart

Log or fail config server startup when upgrading between 2 versions t…

4 files changed, 181 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..19dae614f56 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 skipUpgradeCheck;
 
     @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,
+             Boolean.parseBoolean(Optional.ofNullable(System.getenv("VESPA_SKIP_UPGRADE_CHECK")).orElse("false")));
     }
 
-    public VersionState(File versionFile, Curator curator) {
+    public VersionState(File versionFile, Curator curator, boolean skipUpgradeCheck) {
+        this(versionFile,
+             curator,
+             new Version(VespaVersion.major, VespaVersion.minor, VespaVersion.micro),
+             skipUpgradeCheck);
+    }
+
+    public VersionState(File versionFile,
+                        Curator curator,
+                        Version currentVersion,
+                        boolean skipUpgradeCheck) {
         this.versionFile = versionFile;
         this.curator = curator;
+        this.currentVersion = currentVersion;
+        this.skipUpgradeCheck = skipUpgradeCheck;
     }
 
     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,39 @@ 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;
+
+        String message = "Cannot upgrade from " + storedVersion + " to " + currentVersion();
+        if (storedVersionMajor < latestVersionOnPreviousMajor.getMajor())
+            logOrThrow(message + " (upgrade across 2 major versions not supported). Please upgrade to " +
+                               latestVersionOnPreviousMajor.toFullString() + " first." +
+                               " Setting VESPA_SKIP_UPGRADE_CHECK=true will skip this check at your own risk," +
+                               " see https://vespa.ai/releases.html#versions");
+        else if (sameMajor && (currentVersionMinor - storedVersionMinor > allowedMinorVersionInterval))
+            logOrThrow(message + ". Please upgrade to an older version first, the interval between the two versions is too large (> " + allowedMinorVersionInterval + " releases)." +
+                               " Setting VESPA_SKIP_UPGRADE_CHECK=true will skip this check at your own risk," +
+                               " see https://vespa.ai/releases.html#versions");
+        else if (differentMajor && storedVersionMinor < latestVersionOnPreviousMajor.getMinor())
+            logOrThrow(message + " (new major version). Please upgrade to " + latestVersionOnPreviousMajor.toFullString() + " first." +
+                               " Setting VESPA_SKIP_UPGRADE_CHECK=true will skip this check at your own risk," +
+                               " see https://vespa.ai/releases.html#versions");
+        else if (differentMajor && currentVersionMinor > allowedMinorVersionInterval)
+            logOrThrow(message + ". Please upgrade to an older version first, the interval between the two versions is too large (> " + allowedMinorVersionInterval + " releases)." +
+                               " Setting VESPA_SKIP_UPGRADE_CHECK=true will skip this check at your own risk," +
+                               " see https://vespa.ai/releases.html#versions");
+    }
+
+    private void logOrThrow(String message) {
+        if (skipUpgradeCheck)
+            log.log(WARNING, message);
+        else
+            throw new RuntimeException(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..a60b7babe35 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,10 +179,84 @@ 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);
+
+        assertUpgradeWorks("7,.594.36", "8.110.1", tester, rpcServer);
+        assertUpgradeWorks("8.100.1", "8.110.1", tester, rpcServer);
+
+        assertUpgradeFails("8.100.1", "8.131.1", tester, rpcServer,
+                           "Cannot upgrade from 8.100.1 to 8.131.1. Please upgrade to an older version first, " +
+                                   "the interval between the two versions is too large (> 30 releases)." +
+                                   " Setting VESPA_SKIP_UPGRADE_CHECK=true will skip this check at your own risk," +
+                                   " see https://vespa.ai/releases.html#versions");
+        assertUpgradeFails("7.99.1", "8.100.1", tester, rpcServer,
+                           "Cannot upgrade from 7.99.1 to 8.100.1 (new major version). Please upgrade to 7.594.36 first." +
+                                   " Setting VESPA_SKIP_UPGRADE_CHECK=true will skip this check at your own risk," +
+                                   " see https://vespa.ai/releases.html#versions");
+        assertUpgradeFails("7.594.36", "8.121.1", tester, rpcServer,
+                           "Cannot upgrade from 7.594.36 to 8.121.1. Please upgrade to an older version first," +
+                                   " the interval between the two versions is too large (> 30 releases)." +
+                                   " Setting VESPA_SKIP_UPGRADE_CHECK=true will skip this check at your own risk," +
+                                   " see https://vespa.ai/releases.html#versions");
+        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." +
+                                   " Setting VESPA_SKIP_UPGRADE_CHECK=true will skip this check at your own risk," +
+                                   " see https://vespa.ai/releases.html#versions");
+
+        Version versionToUpgradeTo = Version.fromString("8.131.1");
+        boolean skipUpgradeCheck = true;
+        VersionState versionState = createVersionState(tester.curator(), versionToUpgradeTo, skipUpgradeCheck);
+        assertUpgradeWorks("7.594.36", tester, rpcServer, versionState);
+        assertUpgradeWorks("8.100.1", tester, rpcServer, versionState);
+    }
+
+    private void assertUpgradeWorks(String from, String to, DeployTester tester, RpcServer rpcServer) throws IOException {
+        Version versionToUpgradeTo = Version.fromString(to);
+        VersionState versionState = createVersionState(tester.curator(), versionToUpgradeTo);
+        Bootstrapper bootstrap = createBootstrapper(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY, versionState);
+        bootstrap.versionState().storeVersion(from);
+        bootstrap.doStart();
+    }
+
+    private void assertUpgradeWorks(String from, DeployTester tester, RpcServer rpcServer, VersionState versionState) {
+        Bootstrapper bootstrap = createBootstrapper(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY, versionState);
+        bootstrap.versionState().storeVersion(from);
+        bootstrap.doStart();
+    }
+
+    private void assertUpgradeFails(String from, String to, DeployTester tester, RpcServer rpcServer, String expected) throws IOException {
+        Version versionToUpgradeTo = Version.fromString(to);
+        VersionState versionState = createVersionState(tester.curator(), versionToUpgradeTo);
+        Bootstrapper bootstrap = createBootstrapper(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY, versionState);
+        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());
+        Version versionToUpgradeTo = new Version(VespaVersion.major, VespaVersion.minor, VespaVersion.micro);
+        return createBootstrapper(tester,rpcServer, vipStatusMode, createVersionState(tester.curator(), versionToUpgradeTo));
+    }
 
+    private Bootstrapper createBootstrapper(DeployTester tester,
+                                            RpcServer rpcServer,
+                                            VipStatusMode vipStatusMode,
+                                            VersionState versionState) {
         StateMonitor stateMonitor = StateMonitor.createForTesting();
         VipStatus vipStatus = createVipStatus(stateMonitor);
         return new Bootstrapper(tester.applicationRepository(),
@@ -229,7 +307,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 +322,14 @@ 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, false);
+    }
+
+    private VersionState createVersionState(Curator curator,
+                                            Version vespaVersion,
+                                            boolean throwIfUpgradeBetweenVersionsTooFarApart) throws IOException {
+        return new VersionState(temporaryFolder.newFile(), curator, vespaVersion, throwIfUpgradeBetweenVersionsTooFarApart);
     }
 
     public static class MockRpcServer extends com.yahoo.vespa.config.server.rpc.MockRpcServer {
@@ -286,13 +370,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..917c05ad79b 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, true);
     }
 
 }