check for active MSses before starting DB upgrade (#12140)

engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java

@@ -123,8 +123,12 @@ import com.cloud.utils.component.SystemIntegrityChecker;
 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;
 
 public class DatabaseUpgradeChecker implements SystemIntegrityChecker {
@@ -247,7 +251,6 @@ public class DatabaseUpgradeChecker implements SystemIntegrityChecker {
             LOGGER.error("Unable to execute upgrade script", e);
             throw new CloudRuntimeException("Unable to execute upgrade script", e);
         }
-
     }
 
     @VisibleForTesting
@@ -448,43 +451,101 @@ public class DatabaseUpgradeChecker implements SystemIntegrityChecker {
                 throw new CloudRuntimeException("Unable to acquire lock to check for database integrity.");
             }
 
-            try {
+            doUpgrades(lock);
-                initializeDatabaseEncryptors();
-
-                final CloudStackVersion dbVersion = CloudStackVersion.parse(_dao.getCurrentVersion());
-                final String currentVersionValue = this.getClass().getPackage().getImplementationVersion();
-
-                if (StringUtils.isBlank(currentVersionValue)) {
-                    return;
-                }
-
-                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());
-
-                LOGGER.info("DB version = " + dbVersion + " Code Version = " + currentVersion);
-
-                if (dbVersion.compareTo(currentVersion) > 0) {
-                    throw new CloudRuntimeException("Database version " + dbVersion + " is higher than management software version " + currentVersionValue);
-                }
-
-                if (dbVersion.compareTo(currentVersion) == 0) {
-                    LOGGER.info("DB version and code version matches so no upgrade needed.");
-                    return;
-                }
-
-                upgrade(dbVersion, currentVersion);
-            } finally {
-                lock.unlock();
-            }
         } finally {
             lock.releaseRef();
         }
     }
 
-    private void initializeDatabaseEncryptors() {
+    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'";
+                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;
+                    }
+                } catch (SQLException e) {
+                    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;
+            }
+        });
+    }
+
+    @VisibleForTesting
+    protected void doUpgrades(GlobalLock lock) {
+        try {
+            initializeDatabaseEncryptors();
+
+            final CloudStackVersion dbVersion = CloudStackVersion.parse(_dao.getCurrentVersion());
+            final String currentVersionValue = getImplementationVersion();
+
+            if (StringUtils.isBlank(currentVersionValue)) {
+                return;
+            }
+
+            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());
+            SystemVmTemplateRegistration.CS_TINY_VERSION = String.valueOf(sysVmVersion.getPatchRelease());
+
+            LOGGER.info("DB version = " + dbVersion + " Code Version = " + currentVersion);
+
+            if (dbVersion.compareTo(currentVersion) > 0) {
+                throw new CloudRuntimeException("Database version " + dbVersion + " is higher than management software version " + currentVersionValue);
+            }
+
+            if (dbVersion.compareTo(currentVersion) == 0) {
+                LOGGER.info("DB version and code version matches so no upgrade needed.");
+                return;
+            }
+
+            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);
+                handleClusteredUpgradeRequired(); // allow tests to override behavior
+            }
+        } finally {
+            lock.unlock();
+        }
+    }
+
+    /**
+     * 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";

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);
+    }
+}

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 java.sql.SQLException;
-import static org.junit.Assert.assertNotNull;
+import java.lang.reflect.Field;
-import static org.junit.Assert.assertTrue;
+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,51 @@ import com.cloud.upgrade.dao.Upgrade471to480;
 import com.cloud.upgrade.dao.Upgrade480to481;
 import com.cloud.upgrade.dao.Upgrade490to4910;
 
+import com.cloud.utils.db.TransactionLegacy;
+
+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 {
 
+    @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 +132,7 @@ public class DatabaseUpgradeCheckerTest {
         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 +157,7 @@ public class DatabaseUpgradeCheckerTest {
         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 +204,12 @@ public class DatabaseUpgradeCheckerTest {
         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 +226,7 @@ public class DatabaseUpgradeCheckerTest {
     }
 
     @Test
-    public void testCalculateUpgradePathFromKownDbVersion() {
+    public void testCalculateUpgradePathFromKnownDbVersion() {
 
         final CloudStackVersion dbVersion = CloudStackVersion.parse("4.17.0.0");
         assertNotNull(dbVersion);
@@ -306,4 +359,25 @@ public class DatabaseUpgradeCheckerTest {
         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());
+    }
+
 }