Skip to content

check for active MSses before starting DB upgrade #12140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: 4.20
Choose a base branch
from
+59 −24
Draft

check for active MSses before starting DB upgrade #12140

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
edcbbde
check for active MSses before starting DB upgrade
Nov 26, 2025
48af60a
fix sql
DaanHoogland Nov 27, 2025
46a6f7a
none may be up!
DaanHoogland Nov 27, 2025
2135e1e
Apply suggestions from code review
DaanHoogland Nov 27, 2025
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
83 changes: 59 additions & 24 deletions engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,10 @@
import com.cloud.utils.crypt.DBEncryptionUtil;
import com.cloud.utils.db.GlobalLock;
import com.cloud.utils.db.ScriptRunner;
import com.cloud.utils.db.Transaction;
import com.cloud.utils.db.TransactionCallback;
import com.cloud.utils.db.TransactionLegacy;
import com.cloud.utils.db.TransactionStatus;
import com.cloud.utils.exception.CloudRuntimeException;
import com.google.common.annotations.VisibleForTesting;

Expand Down Expand Up @@ -448,39 +451,71 @@ public void check() {
throw new CloudRuntimeException("Unable to acquire lock to check for database integrity.");
}

try {
initializeDatabaseEncryptors();
// not sure about the right moment to do this yet
checkIfStandalone();
doUpgrades(lock);
} finally {
lock.releaseRef();
}
}
private void checkIfStandalone() throws CloudRuntimeException {
boolean standalone = Transaction.execute(new TransactionCallback<>() {
@Override
public Boolean doInTransaction(TransactionStatus status) {
String sql = "SELECT COUNT(*) FROM `cloud`.`management_server` WHERE `status` = 'UP'";
try (Connection conn = TransactionLegacy.getStandaloneConnection();
PreparedStatement pstmt = conn.prepareStatement(sql);
ResultSet rs = pstmt.executeQuery()) {
if (rs.next()) {
int count = rs.getInt(1);
return count <= 1;
}
} catch (SQLException e) {
String errorMessage = "Unable to check if the management server is running in standalone mode.";
LOGGER.error(errorMessage, e);
throw new CloudRuntimeException(errorMessage, e);
}
return true;
}
});
Comment on lines 462 to 480
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Transaction.execute() wrapper is unnecessary here. The code inside the callback is obtaining its own standalone connection via TransactionLegacy.getStandaloneConnection(), which means the transaction context provided by Transaction.execute() is not being used.

Consider simplifying this to directly execute the query without the Transaction.execute wrapper, or use the TransactionStatus parameter properly if transaction management is needed.

Suggested change
boolean standalone = Transaction.execute(new TransactionCallback<>() {
@Override
public Boolean doInTransaction(TransactionStatus status) {
String sql = "SELECT COUNT(*) FROM `cloud`.`management_server` WHERE `status` = 'UP'";
try (Connection conn = TransactionLegacy.getStandaloneConnection();
PreparedStatement pstmt = conn.prepareStatement(sql);
ResultSet rs = pstmt.executeQuery()) {
if (rs.next()) {
int count = rs.getInt(1);
return count <= 1;
}
} catch (SQLException e) {
String errorMessage = "Unable to check if the management server is running in standalone mode.";
LOGGER.error(errorMessage, e);
throw new CloudRuntimeException(errorMessage, e);
}
return true;
}
});
String sql = "SELECT COUNT(*) FROM `cloud`.`management_server` WHERE `status` = 'UP'";
boolean standalone = true;
try (Connection conn = TransactionLegacy.getStandaloneConnection();
PreparedStatement pstmt = conn.prepareStatement(sql);
ResultSet rs = pstmt.executeQuery()) {
if (rs.next()) {
int count = rs.getInt(1);
standalone = count <= 1;
}
} catch (SQLException e) {
String errorMessage = "Unable to check if the management server is running in standalone mode.";
LOGGER.error(errorMessage, e);
throw new CloudRuntimeException(errorMessage, e);
}

Copilot uses AI. Check for mistakes.
if (! standalone) {
String msg = "CloudStack is running multiple management servers and attempting to upgrade. Upgrades can only be run in standalone mode. Skipping database upgrade check.";
LOGGER.info(msg);
throw new CloudRuntimeException(msg);
}
}
Comment on lines 461 to 486
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new checkIfStandalone() method lacks test coverage. Consider adding unit tests to verify:

  1. The behavior when no management servers are running (count = 0)
  2. The behavior when exactly one management server is running (count = 1)
  3. The behavior when multiple management servers are running (count > 1)
  4. The behavior when a SQLException occurs during the check

This is particularly important since this check prevents upgrades from proceeding when multiple servers are detected.

Copilot uses AI. Check for mistakes.

final CloudStackVersion dbVersion = CloudStackVersion.parse(_dao.getCurrentVersion());
final String currentVersionValue = this.getClass().getPackage().getImplementationVersion();
private void doUpgrades(GlobalLock lock) {
try {
initializeDatabaseEncryptors();

if (StringUtils.isBlank(currentVersionValue)) {
return;
}
final CloudStackVersion dbVersion = CloudStackVersion.parse(_dao.getCurrentVersion());
final String currentVersionValue = this.getClass().getPackage().getImplementationVersion();

String csVersion = SystemVmTemplateRegistration.parseMetadataFile();
final CloudStackVersion sysVmVersion = CloudStackVersion.parse(csVersion);
final CloudStackVersion currentVersion = CloudStackVersion.parse(currentVersionValue);
SystemVmTemplateRegistration.CS_MAJOR_VERSION = String.valueOf(sysVmVersion.getMajorRelease()) + "." + String.valueOf(sysVmVersion.getMinorRelease());
SystemVmTemplateRegistration.CS_TINY_VERSION = String.valueOf(sysVmVersion.getPatchRelease());
if (StringUtils.isBlank(currentVersionValue)) {
return;
}

LOGGER.info("DB version = " + dbVersion + " Code Version = " + currentVersion);
String csVersion = SystemVmTemplateRegistration.parseMetadataFile();
final CloudStackVersion sysVmVersion = CloudStackVersion.parse(csVersion);
final CloudStackVersion currentVersion = CloudStackVersion.parse(currentVersionValue);
SystemVmTemplateRegistration.CS_MAJOR_VERSION = String.valueOf(sysVmVersion.getMajorRelease()) + "." + String.valueOf(sysVmVersion.getMinorRelease());
SystemVmTemplateRegistration.CS_TINY_VERSION = String.valueOf(sysVmVersion.getPatchRelease());

if (dbVersion.compareTo(currentVersion) > 0) {
throw new CloudRuntimeException("Database version " + dbVersion + " is higher than management software version " + currentVersionValue);
}
LOGGER.info("DB version = " + dbVersion + " Code Version = " + currentVersion);

if (dbVersion.compareTo(currentVersion) == 0) {
LOGGER.info("DB version and code version matches so no upgrade needed.");
return;
}
if (dbVersion.compareTo(currentVersion) > 0) {
throw new CloudRuntimeException("Database version " + dbVersion + " is higher than management software version " + currentVersionValue);
}

upgrade(dbVersion, currentVersion);
} finally {
lock.unlock();
if (dbVersion.compareTo(currentVersion) == 0) {
LOGGER.info("DB version and code version matches so no upgrade needed.");
return;
}

upgrade(dbVersion, currentVersion);
} finally {
lock.releaseRef();
lock.unlock();
}
}

Expand Down
Loading