From edcbbdef0967461b3c35c74d6e66ec7e3b612876 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Wed, 26 Nov 2025 15:14:07 +0100 Subject: [PATCH 1/9] check for active MSses before starting DB upgrade --- .../cloud/upgrade/DatabaseUpgradeChecker.java | 83 +++++++++++++------ 1 file changed, 59 insertions(+), 24 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 abf860439375..e99cd894a9c3 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java @@ -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; @@ -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; + } + }); + 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); + } + } - 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(); } } From 48af60a81a1c6895fb8d5b35bd75dc9de3689711 Mon Sep 17 00:00:00 2001 From: dahn Date: Thu, 27 Nov 2025 16:41:58 +0100 Subject: [PATCH 2/9] fix sql --- .../src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 e99cd894a9c3..5bd117a34eb2 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java @@ -462,7 +462,7 @@ 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'"; + String sql = "SELECT COUNT(*) FROM `cloud`.`mshosts` WHERE `state` = 'UP'"; try (Connection conn = TransactionLegacy.getStandaloneConnection(); PreparedStatement pstmt = conn.prepareStatement(sql); ResultSet rs = pstmt.executeQuery()) { From 46a6f7a0fcf907385daeb85c6cd233a74f452dc9 Mon Sep 17 00:00:00 2001 From: dahn Date: Thu, 27 Nov 2025 16:43:12 +0100 Subject: [PATCH 3/9] none may be up! --- .../src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5bd117a34eb2..2b7266ae75a5 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java @@ -468,7 +468,7 @@ public Boolean doInTransaction(TransactionStatus status) { ResultSet rs = pstmt.executeQuery()) { if (rs.next()) { int count = rs.getInt(1); - return count <= 1; + return count = 0; } } catch (SQLException e) { String errorMessage = "Unable to check if the management server is running in standalone mode."; From 2135e1ef70b7b922b68f21e82425bdfa25e1f97d Mon Sep 17 00:00:00 2001 From: dahn Date: Thu, 27 Nov 2025 17:05:20 +0100 Subject: [PATCH 4/9] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../java/com/cloud/upgrade/DatabaseUpgradeChecker.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 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 2b7266ae75a5..1e473b14063f 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java @@ -462,13 +462,13 @@ private void checkIfStandalone() throws CloudRuntimeException { boolean standalone = Transaction.execute(new TransactionCallback<>() { @Override public Boolean doInTransaction(TransactionStatus status) { - String sql = "SELECT COUNT(*) FROM `cloud`.`mshosts` WHERE `state` = 'UP'"; - try (Connection conn = TransactionLegacy.getStandaloneConnection(); + String sql = "SELECT COUNT(*) FROM `cloud`.`mshost` WHERE `state` = '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 = 0; + return count == 0; } } catch (SQLException e) { String errorMessage = "Unable to check if the management server is running in standalone mode."; @@ -479,7 +479,7 @@ public Boolean doInTransaction(TransactionStatus status) { } }); 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."; + String msg = "CloudStack is running multiple management servers while attempting to upgrade. Upgrades can only be run in standalone mode. Aborting."; LOGGER.info(msg); throw new CloudRuntimeException(msg); } From 42414f441ec257300141ac24801a384f37c7278c Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 1 Dec 2025 14:09:18 +0100 Subject: [PATCH 5/9] make condition not fatal --- .../cloud/upgrade/DatabaseUpgradeChecker.java | 17 ++++++----------- 1 file changed, 6 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 1e473b14063f..82d0dbcd347f 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java @@ -451,15 +451,15 @@ public void check() { throw new CloudRuntimeException("Unable to acquire lock to check for database integrity."); } - // not sure about the right moment to do this yet - checkIfStandalone(); - doUpgrades(lock); + if (isStandalone()) { + doUpgrades(lock); + } } finally { lock.releaseRef(); } } - private void checkIfStandalone() throws CloudRuntimeException { - boolean standalone = Transaction.execute(new TransactionCallback<>() { + private boolean isStandalone() throws CloudRuntimeException { + return Transaction.execute(new TransactionCallback<>() { @Override public Boolean doInTransaction(TransactionStatus status) { String sql = "SELECT COUNT(*) FROM `cloud`.`mshost` WHERE `state` = 'UP'"; @@ -473,16 +473,11 @@ public Boolean doInTransaction(TransactionStatus status) { } 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 false; } return true; } }); - if (! standalone) { - String msg = "CloudStack is running multiple management servers while attempting to upgrade. Upgrades can only be run in standalone mode. Aborting."; - LOGGER.info(msg); - throw new CloudRuntimeException(msg); - } } private void doUpgrades(GlobalLock lock) { From 6d9a6c56a0d234b2125dc85ac3b0357a010d0025 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Tue, 2 Dec 2025 10:19:21 +0100 Subject: [PATCH 6/9] change moment of abort --- .../cloud/upgrade/DatabaseUpgradeChecker.java | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 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 82d0dbcd347f..264b1400a533 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java @@ -250,7 +250,6 @@ protected void runScript(Connection conn, InputStream file) { LOGGER.error("Unable to execute upgrade script", e); throw new CloudRuntimeException("Unable to execute upgrade script", e); } - } @VisibleForTesting @@ -451,13 +450,12 @@ public void check() { throw new CloudRuntimeException("Unable to acquire lock to check for database integrity."); } - if (isStandalone()) { - doUpgrades(lock); - } + doUpgrades(lock); } finally { lock.releaseRef(); } } + private boolean isStandalone() throws CloudRuntimeException { return Transaction.execute(new TransactionCallback<>() { @Override @@ -474,6 +472,10 @@ public Boolean doInTransaction(TransactionStatus status) { String errorMessage = "Unable to check if the management server is running in standalone mode."; LOGGER.error(errorMessage, e); return false; + } catch (NullPointerException npe) { + String errorMessage = "Unable to check if the management server is running in standalone mode. Not able to get a Database connection."; + LOGGER.error(errorMessage, npe); + return false; } return true; } @@ -508,7 +510,14 @@ private void doUpgrades(GlobalLock lock) { return; } - upgrade(dbVersion, currentVersion); + if (isStandalone()) { + upgrade(dbVersion, currentVersion); + } 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."; + LOGGER.error(errorMessage); + throw new CloudRuntimeException(errorMessage); + } } finally { lock.unlock(); } From 03150051bc51989034039821d60cbfb5b9bf1354 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Tue, 2 Dec 2025 15:31:02 +0100 Subject: [PATCH 7/9] more hard exit --- .../main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 264b1400a533..47fd7ac82f34 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java @@ -128,6 +128,7 @@ import com.cloud.utils.db.TransactionLegacy; import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.exception.CloudRuntimeException; + import com.google.common.annotations.VisibleForTesting; public class DatabaseUpgradeChecker implements SystemIntegrityChecker { @@ -516,7 +517,7 @@ private void doUpgrades(GlobalLock lock) { 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."; LOGGER.error(errorMessage); - throw new CloudRuntimeException(errorMessage); + System.exit(5); // I would prefer ServerDaemon.abort(errorMessage) but that would create a dependency hell } } finally { lock.unlock(); From 335f1445952882626611e7edc7e28c8e90bd0543 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Tue, 2 Dec 2025 16:18:34 +0100 Subject: [PATCH 8/9] some unit tests --- .../cloud/upgrade/DatabaseUpgradeChecker.java | 33 +++- .../DatabaseUpgradeCheckerDoUpgradesTest.java | 173 ++++++++++++++++++ .../upgrade/DatabaseUpgradeCheckerTest.java | 87 ++++++++- 3 files changed, 278 insertions(+), 15 deletions(-) create mode 100644 engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerDoUpgradesTest.java 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 47fd7ac82f34..afb7a8d69e6d 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java @@ -457,7 +457,7 @@ public void check() { } } - private boolean isStandalone() throws CloudRuntimeException { + boolean isStandalone() throws CloudRuntimeException { return Transaction.execute(new TransactionCallback<>() { @Override public Boolean doInTransaction(TransactionStatus status) { @@ -483,18 +483,19 @@ public Boolean doInTransaction(TransactionStatus status) { }); } - private void doUpgrades(GlobalLock lock) { + @VisibleForTesting + protected void doUpgrades(GlobalLock lock) { try { initializeDatabaseEncryptors(); final CloudStackVersion dbVersion = CloudStackVersion.parse(_dao.getCurrentVersion()); - final String currentVersionValue = this.getClass().getPackage().getImplementationVersion(); + final String currentVersionValue = getImplementationVersion(); if (StringUtils.isBlank(currentVersionValue)) { return; } - String csVersion = SystemVmTemplateRegistration.parseMetadataFile(); + String csVersion = parseSystemVmMetadata(); final CloudStackVersion sysVmVersion = CloudStackVersion.parse(csVersion); final CloudStackVersion currentVersion = CloudStackVersion.parse(currentVersionValue); SystemVmTemplateRegistration.CS_MAJOR_VERSION = String.valueOf(sysVmVersion.getMajorRelease()) + "." + String.valueOf(sysVmVersion.getMinorRelease()); @@ -517,14 +518,34 @@ private void doUpgrades(GlobalLock lock) { 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."; LOGGER.error(errorMessage); - System.exit(5); // I would prefer ServerDaemon.abort(errorMessage) but that would create a dependency hell + handleClusteredUpgradeRequired(); // allow tests to override behavior } } finally { lock.unlock(); } } - private void initializeDatabaseEncryptors() { + /** + * 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. + */ + @VisibleForTesting + protected void handleClusteredUpgradeRequired() { + System.exit(5); // I would prefer ServerDaemon.abort(errorMessage) but that would create a dependency hell + } + + @VisibleForTesting + protected String getImplementationVersion() { + return this.getClass().getPackage().getImplementationVersion(); + } + + @VisibleForTesting + protected String parseSystemVmMetadata() { + return SystemVmTemplateRegistration.parseMetadataFile(); + } + + // Make this protected so tests can noop it out + protected void initializeDatabaseEncryptors() { TransactionLegacy txn = TransactionLegacy.open("initializeDatabaseEncryptors"); txn.start(); String errorMessage = "Unable to get the database connections"; diff --git a/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerDoUpgradesTest.java b/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerDoUpgradesTest.java new file mode 100644 index 000000000000..6241bd7cede0 --- /dev/null +++ b/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerDoUpgradesTest.java @@ -0,0 +1,173 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.upgrade; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import com.cloud.upgrade.dao.VersionDao; +import com.cloud.upgrade.dao.VersionDaoImpl; +import com.cloud.upgrade.dao.VersionVO; +import com.cloud.utils.db.GlobalLock; +import org.junit.Test; + +public class DatabaseUpgradeCheckerDoUpgradesTest { + + static class StubVersionDao extends VersionDaoImpl implements VersionDao { + private final String currentVersion; + + StubVersionDao(String currentVersion) { + this.currentVersion = currentVersion; + } + + @Override + public VersionVO findByVersion(String version, VersionVO.Step step) { + return null; + } + + @Override + public String getCurrentVersion() { + return currentVersion; + } + + } + + private static class TestableChecker extends DatabaseUpgradeChecker { + boolean initializeCalled = false; + boolean upgradeCalled = false; + boolean clusterHandlerCalled = false; + String implVersionOverride = null; + String sysVmMetadataOverride = "4.8.0"; + boolean standaloneOverride = true; + + TestableChecker(String daoVersion) { + // set a stub DAO + this._dao = new StubVersionDao(daoVersion); + } + + @Override + protected void initializeDatabaseEncryptors() { + initializeCalled = true; + // noop instead of doing DB work + } + + @Override + protected String getImplementationVersion() { + return implVersionOverride; + } + + @Override + protected String parseSystemVmMetadata() { + return sysVmMetadataOverride; + } + + @Override + boolean isStandalone() { + return standaloneOverride; + } + + @Override + protected void upgrade(org.apache.cloudstack.utils.CloudStackVersion dbVersion, org.apache.cloudstack.utils.CloudStackVersion currentVersion) { + upgradeCalled = true; + } + + @Override + protected void handleClusteredUpgradeRequired() { + clusterHandlerCalled = true; + } + } + + @Test + public void testDoUpgrades_noImplementationVersion_returnsEarly() { + TestableChecker checker = new TestableChecker("4.8.0"); + checker.implVersionOverride = ""; // blank -> should return early + + GlobalLock lock = GlobalLock.getInternLock("test-noimpl"); + try { + // acquire lock so doUpgrades can safely call unlock in finally + lock.lock(1); + checker.doUpgrades(lock); + } finally { + // ensure lock released if still held + lock.releaseRef(); + } + + assertTrue("initializeDatabaseEncryptors should be called before returning", checker.initializeCalled); + assertFalse("upgrade should not be called when implementation version is blank", checker.upgradeCalled); + assertFalse("cluster handler should not be called", checker.clusterHandlerCalled); + } + + @Test + public void testDoUpgrades_dbUpToDate_noUpgrade() { + // DB version = code version -> no upgrade + TestableChecker checker = new TestableChecker("4.8.1"); + checker.implVersionOverride = "4.8.1"; + checker.sysVmMetadataOverride = "4.8.1"; + + GlobalLock lock = GlobalLock.getInternLock("test-uptodate"); + try { + lock.lock(1); + checker.doUpgrades(lock); + } finally { + lock.releaseRef(); + } + + assertTrue(checker.initializeCalled); + assertFalse(checker.upgradeCalled); + assertFalse(checker.clusterHandlerCalled); + } + + @Test + public void testDoUpgrades_requiresUpgrade_standalone_invokesUpgrade() { + TestableChecker checker = new TestableChecker("4.8.0"); + checker.implVersionOverride = "4.8.2"; // code is newer than DB + checker.sysVmMetadataOverride = "4.8.2"; + checker.standaloneOverride = true; + + GlobalLock lock = GlobalLock.getInternLock("test-upgrade-standalone"); + try { + lock.lock(1); + checker.doUpgrades(lock); + } finally { + lock.releaseRef(); + } + + assertTrue(checker.initializeCalled); + assertTrue("upgrade should be invoked in standalone mode", checker.upgradeCalled); + assertFalse(checker.clusterHandlerCalled); + } + + @Test + public void testDoUpgrades_requiresUpgrade_clustered_invokesHandler() { + TestableChecker checker = new TestableChecker("4.8.0"); + checker.implVersionOverride = "4.8.2"; // code is newer than DB + checker.sysVmMetadataOverride = "4.8.2"; + checker.standaloneOverride = false; + + GlobalLock lock = GlobalLock.getInternLock("test-upgrade-clustered"); + try { + lock.lock(1); + checker.doUpgrades(lock); + } finally { + lock.releaseRef(); + } + + assertTrue(checker.initializeCalled); + assertFalse("upgrade should not be invoked in clustered mode", checker.upgradeCalled); + assertTrue("cluster handler should be invoked in clustered mode", 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 1a9372f73eaf..c01e7b9c9fdf 100644 --- a/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java +++ b/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java @@ -16,14 +16,24 @@ // under the License. package com.cloud.upgrade; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; +import java.sql.SQLException; +import java.lang.reflect.Field; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; -import java.util.Arrays; +import javax.sql.DataSource; import org.apache.cloudstack.utils.CloudStackVersion; import org.junit.Test; +import org.junit.Before; +import org.junit.After; +import org.junit.runner.RunWith; + +import org.mockito.ArgumentMatchers; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; import com.cloud.upgrade.DatabaseUpgradeChecker.NoopDbUpgrade; import com.cloud.upgrade.dao.DbUpgrade; @@ -43,8 +53,46 @@ import com.cloud.upgrade.dao.Upgrade480to481; import com.cloud.upgrade.dao.Upgrade490to4910; +import com.cloud.utils.db.TransactionLegacy; + +import static org.junit.Assert.*; + +@RunWith(MockitoJUnitRunner.class) public class DatabaseUpgradeCheckerTest { + @Mock + DataSource dataSource; + + @Mock + Connection connection; + + @Mock + PreparedStatement preparedStatement; + + @Mock + ResultSet resultSet; + + private DataSource backupDataSource; + + @Before + public void setup() throws Exception { + Field dsField = TransactionLegacy.class.getDeclaredField("s_ds"); + dsField.setAccessible(true); + backupDataSource = (DataSource) dsField.get(null); + dsField.set(null, dataSource); + + Mockito.when(dataSource.getConnection()).thenReturn(connection); + Mockito.when(connection.prepareStatement(ArgumentMatchers.anyString())).thenReturn(preparedStatement); + Mockito.when(preparedStatement.executeQuery()).thenReturn(resultSet); + } + + @After + public void cleanup() throws Exception { + Field dsField = TransactionLegacy.class.getDeclaredField("s_ds"); + dsField.setAccessible(true); + dsField.set(null, backupDataSource); + } + @Test public void testCalculateUpgradePath480to481() { @@ -79,7 +127,7 @@ public void testCalculateUpgradePath490to4910() { assertTrue(upgrades.length >= 1); assertTrue(upgrades[0] instanceof Upgrade490to4910); - assertTrue(Arrays.equals(new String[] {"4.9.0", currentVersion.toString()}, upgrades[0].getUpgradableVersionRange())); + assertArrayEquals(new String[]{"4.9.0", currentVersion.toString()}, upgrades[0].getUpgradableVersionRange()); assertEquals(currentVersion.toString(), upgrades[0].getUpgradedVersion()); } @@ -104,7 +152,7 @@ public void testCalculateUpgradePath410to412() { assertTrue(upgrades[3] instanceof Upgrade41120to41130); assertTrue(upgrades[4] instanceof Upgrade41120to41200); - assertTrue(Arrays.equals(new String[] {"4.11.0.0", "4.11.1.0"}, upgrades[1].getUpgradableVersionRange())); + assertArrayEquals(new String[]{"4.11.0.0", "4.11.1.0"}, upgrades[1].getUpgradableVersionRange()); assertEquals(currentVersion.toString(), upgrades[4].getUpgradedVersion()); } @@ -151,12 +199,12 @@ public void testFindUpgradePath452to490() { assertTrue(upgrades[5] instanceof Upgrade471to480); assertTrue(upgrades[6] instanceof Upgrade480to481); - assertTrue(Arrays.equals(new String[] {"4.8.1", currentVersion.toString()}, upgrades[upgrades.length - 1].getUpgradableVersionRange())); + assertArrayEquals(new String[]{"4.8.1", currentVersion.toString()}, upgrades[upgrades.length - 1].getUpgradableVersionRange()); assertEquals(currentVersion.toString(), upgrades[upgrades.length - 1].getUpgradedVersion()); } @Test - public void testCalculateUpgradePathUnkownDbVersion() { + public void testCalculateUpgradePathUnknownDbVersion() { final CloudStackVersion dbVersion = CloudStackVersion.parse("4.99.0.0"); assertNotNull(dbVersion); @@ -173,7 +221,7 @@ public void testCalculateUpgradePathUnkownDbVersion() { } @Test - public void testCalculateUpgradePathFromKownDbVersion() { + public void testCalculateUpgradePathFromKnownDbVersion() { final CloudStackVersion dbVersion = CloudStackVersion.parse("4.17.0.0"); assertNotNull(dbVersion); @@ -306,4 +354,25 @@ public void testCalculateUpgradePathFromSecurityReleaseToNextSecurityRelease() { assertEquals(upgrades.length + 1, upgradesFromSecurityReleaseToNext.length); assertTrue(upgradesFromSecurityReleaseToNext[upgradesFromSecurityReleaseToNext.length - 1] instanceof NoopDbUpgrade); } + + @Test + public void isStandalone() throws SQLException { + // simulate zero 'UP' hosts -> standalone + Mockito.when(resultSet.next()).thenReturn(true); + Mockito.when(resultSet.getInt(1)).thenReturn(0); + + final DatabaseUpgradeChecker checker = new DatabaseUpgradeChecker(); + assertTrue("DatabaseUpgradeChecker should be a standalone component", checker.isStandalone()); + } + + @Test + public void isNotStandalone() throws SQLException { + // simulate at least one 'UP' host -> not standalone + Mockito.when(resultSet.next()).thenReturn(true); + Mockito.when(resultSet.getInt(1)).thenReturn(1); + + final DatabaseUpgradeChecker checker = new DatabaseUpgradeChecker(); + assertFalse("DatabaseUpgradeChecker should not be a standalone component", checker.isStandalone()); + } + } From e79dacb78eeaf2c1223d9f93071141a336efaee7 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Tue, 2 Dec 2025 16:29:55 +0100 Subject: [PATCH 9/9] imports --- .../java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 c01e7b9c9fdf..27995eb179af 100644 --- a/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java +++ b/engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java @@ -55,7 +55,12 @@ import com.cloud.utils.db.TransactionLegacy; -import static org.junit.Assert.*; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertArrayEquals; + @RunWith(MockitoJUnitRunner.class) public class DatabaseUpgradeCheckerTest {