This is an automated email from the ASF dual-hosted git repository. DaanHoogland pushed a commit to branch dis13919-upgradeDbNoop in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit ba363db53712881155905f43d4e925bfd6a3e1b4 Author: Daan Hoogland <[email protected]> AuthorDate: Thu Aug 20 09:40:02 2026 +0200 allow MS to start when DB version is behind by a NoOp upgrade step --- .../com/cloud/upgrade/DatabaseUpgradeChecker.java | 32 +++++++++++++++------- .../DatabaseUpgradeCheckerDoUpgradesTest.java | 26 +++++++++++++++++- .../cloud/upgrade/DatabaseUpgradeCheckerTest.java | 24 ++++++++++++++++ 3 files changed, 71 insertions(+), 11 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java b/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java index 0d9807dcb2d..954289784b1 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java @@ -307,9 +307,9 @@ public class DatabaseUpgradeChecker implements SystemIntegrityChecker { } } - protected void upgrade(CloudStackVersion dbVersion, CloudStackVersion currentVersion) { + protected void upgrade(DbUpgrade[] upgrades) { executeProcedureScripts(); - final DbUpgrade[] upgrades = executeUpgrades(dbVersion, currentVersion); + executeUpgrades(upgrades); executeViewScripts(); updateSystemVmTemplates(upgrades); @@ -337,16 +337,11 @@ public class DatabaseUpgradeChecker implements SystemIntegrityChecker { } } - private DbUpgrade[] executeUpgrades(CloudStackVersion dbVersion, CloudStackVersion currentVersion) { - LOGGER.info("Database upgrade must be performed from " + dbVersion + " to " + currentVersion); - - final DbUpgrade[] upgrades = calculateUpgradePath(dbVersion, currentVersion); - + private void executeUpgrades(DbUpgrade[] upgrades) { for (DbUpgrade upgrade : upgrades) { VersionVO version = executeUpgrade(upgrade); executeUpgradeCleanup(upgrade, version); } - return upgrades; } private VersionVO executeUpgrade(DbUpgrade upgrade) { @@ -516,8 +511,11 @@ public class DatabaseUpgradeChecker implements SystemIntegrityChecker { return; } - if (isStandalone()) { - upgrade(dbVersion, currentVersion); + LOGGER.info("Database upgrade must be performed from " + dbVersion + " to " + currentVersion); + final DbUpgrade[] upgrades = calculateUpgradePath(dbVersion, currentVersion); + + if (isStandalone() || isNoopOnlyUpgradePath(upgrades)) { + upgrade(upgrades); } else { String errorMessage = "Database upgrade is required but the management server is running in a clustered environment. " + "Please perform the database upgrade when the management server is not running in a clustered environment."; @@ -529,6 +527,20 @@ public class DatabaseUpgradeChecker implements SystemIntegrityChecker { } } + /** + * A noop-only path means the DB is only missing its version stamp (e.g. a hotfix release with + * no schema/data changes); it is safe to apply on any node regardless of cluster state, which + * also avoids deadlocking a fresh multi-node cluster bring-up where every node runs the same code. + */ + boolean isNoopOnlyUpgradePath(DbUpgrade[] upgrades) { + for (DbUpgrade upgrade : upgrades) { + if (!(upgrade instanceof NoopDbUpgrade)) { + return false; + } + } + return true; + } + /** * Hook that is called when an upgrade is required but the management server is clustered. * Default behavior is to exit the JVM, tests can override to throw instead. diff --git a/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerDoUpgradesTest.java b/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerDoUpgradesTest.java index 6241bd7cede..b0ae5e9e478 100644 --- a/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerDoUpgradesTest.java +++ b/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerDoUpgradesTest.java @@ -19,6 +19,7 @@ package com.cloud.upgrade; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import com.cloud.upgrade.dao.DbUpgrade; import com.cloud.upgrade.dao.VersionDao; import com.cloud.upgrade.dao.VersionDaoImpl; import com.cloud.upgrade.dao.VersionVO; @@ -81,7 +82,7 @@ public class DatabaseUpgradeCheckerDoUpgradesTest { } @Override - protected void upgrade(org.apache.cloudstack.utils.CloudStackVersion dbVersion, org.apache.cloudstack.utils.CloudStackVersion currentVersion) { + protected void upgrade(DbUpgrade[] upgrades) { upgradeCalled = true; } @@ -170,4 +171,27 @@ public class DatabaseUpgradeCheckerDoUpgradesTest { assertFalse("upgrade should not be invoked in clustered mode", checker.upgradeCalled); assertTrue("cluster handler should be invoked in clustered mode", checker.clusterHandlerCalled); } + + @Test + public void testDoUpgrades_noopOnlyPath_clustered_stillUpgrades() { + // DB is one hotfix release behind the code (e.g. 4.20.4.0 -> 4.20.4.1) with no real schema/data + // migration between them, so it must be allowed even though another MS is already reported up - + // this is the normal case when bringing up a fresh multi-node cluster on identical code. + TestableChecker checker = new TestableChecker("4.20.4.0"); + checker.implVersionOverride = "4.20.4.1"; + checker.sysVmMetadataOverride = "4.20.4.1"; + checker.standaloneOverride = false; + + GlobalLock lock = GlobalLock.getInternLock("test-upgrade-noop-clustered"); + try { + lock.lock(1); + checker.doUpgrades(lock); + } finally { + lock.releaseRef(); + } + + assertTrue(checker.initializeCalled); + assertTrue("a noop-only upgrade path must be applied even in clustered mode", checker.upgradeCalled); + assertFalse("cluster handler should not be invoked for a noop-only upgrade path", checker.clusterHandlerCalled); + } } diff --git a/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java b/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java index 1b84fc3a9e3..deec1a415b0 100644 --- a/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java +++ b/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java @@ -226,6 +226,30 @@ public class DatabaseUpgradeCheckerTest { } + @Test + public void testIsNoopOnlyUpgradePathTrueForNoopOnlyPath() { + final CloudStackVersion dbVersion = CloudStackVersion.parse("4.99.0.0"); + final CloudStackVersion currentVersion = CloudStackVersion.parse("4.99.1.0"); + + final DatabaseUpgradeChecker checker = new DatabaseUpgradeChecker(); + final DbUpgrade[] upgrades = checker.calculateUpgradePath(dbVersion, currentVersion); + + assertTrue("a path made up of only version-stamp noop upgrades should be safe on any node", + checker.isNoopOnlyUpgradePath(upgrades)); + } + + @Test + public void testIsNoopOnlyUpgradePathFalseWhenRealUpgradePresent() { + final CloudStackVersion dbVersion = CloudStackVersion.parse("4.8.0"); + final CloudStackVersion currentVersion = CloudStackVersion.parse("4.8.1"); + + final DatabaseUpgradeChecker checker = new DatabaseUpgradeChecker(); + final DbUpgrade[] upgrades = checker.calculateUpgradePath(dbVersion, currentVersion); + + assertFalse("a path containing a real schema/data migration must not be treated as noop-only", + checker.isNoopOnlyUpgradePath(upgrades)); + } + @Test public void testCalculateUpgradePathFromKnownDbVersion() {
