-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
base: 4.20
Are you sure you want to change the base?
check for active MSses before starting DB upgrade #12140
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12140 +/- ##
=============================================
- Coverage 16.18% 4.00% -12.18%
=============================================
Files 5657 402 -5255
Lines 498470 32665 -465805
Branches 60493 5808 -54685
=============================================
- Hits 80660 1309 -79351
+ Misses 408830 31203 -377627
+ Partials 8980 153 -8827
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a check to prevent database upgrades from running when multiple management servers are active. The implementation verifies that only one management server with status 'UP' exists in the database before proceeding with the upgrade process, addressing issue #11973.
- Extracts the upgrade logic into a new
doUpgrades()method for better code organization - Introduces
checkIfStandalone()to query the management_server table and enforce single-server mode - Throws an exception if multiple active management servers are detected, preventing concurrent upgrade attempts
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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; | ||
| } | ||
| }); |
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
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.
| 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); | |
| } |
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Show resolved
Hide resolved
| 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; | ||
| } | ||
| }); | ||
| 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); | ||
| } | ||
| } |
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
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:
- The behavior when no management servers are running (count = 0)
- The behavior when exactly one management server is running (count = 1)
- The behavior when multiple management servers are running (count > 1)
- 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.
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 15845 |
Description
This PR...
Fixes: #11973
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?