Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,9 @@
}
}

protected void upgrade(CloudStackVersion dbVersion, CloudStackVersion currentVersion) {
protected void upgrade(DbUpgrade[] upgrades) {
executeProcedureScripts();
final DbUpgrade[] upgrades = executeUpgrades(dbVersion, currentVersion);
executeUpgrades(upgrades);

executeViewScripts();
updateSystemVmTemplates(upgrades);
Expand Down Expand Up @@ -337,16 +337,11 @@
}
}

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) {
Expand Down Expand Up @@ -516,8 +511,11 @@
return;
}

if (isStandalone()) {
upgrade(dbVersion, currentVersion);
LOGGER.info("Database upgrade must be performed from " + dbVersion + " to " + currentVersion);

Check warning on line 514 in engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Format specifiers should be used instead of string concatenation.

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaAeVwKeQazf8tr0t2N7&open=AaAeVwKeQazf8tr0t2N7&pullRequest=13928

Check warning on line 514 in engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use the built-in formatting to construct this argument.

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaAeVwKeQazf8tr0t2N6&open=AaAeVwKeQazf8tr0t2N6&pullRequest=13928
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.";
Expand All @@ -529,6 +527,20 @@
}
}

/**
* 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
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;
Expand Down Expand Up @@ -81,7 +82,7 @@ boolean isStandalone() {
}

@Override
protected void upgrade(org.apache.cloudstack.utils.CloudStackVersion dbVersion, org.apache.cloudstack.utils.CloudStackVersion currentVersion) {
protected void upgrade(DbUpgrade[] upgrades) {
upgradeCalled = true;
}

Expand Down Expand Up @@ -170,4 +171,27 @@ public void testDoUpgrades_requiresUpgrade_clustered_invokesHandler() {
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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,30 @@ public void testCalculateUpgradePathUnknownDbVersion() {

}

@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() {

Expand Down
Loading