Merge "Add check for object versions"

devstack/lib/ironic

@@ -1426,6 +1426,10 @@ function init_ironic {
         $IRONIC_BIN_DIR/ironic-dbsync --config-file=$IRONIC_CONF_FILE
     fi
     create_ironic_cache_dir
+
+    # NOTE(rloo): We're not upgrading but want to make sure this command works,
+    # even though we're not parsing the output of this command.
+    $IRONIC_BIN_DIR/ironic-status upgrade check
 }
 
 # _ironic_bm_vm_names() - Generates list of names for baremetal VMs.

devstack/upgrade/upgrade.sh

@@ -71,6 +71,11 @@ stack_install_service ironic
 # calls upgrade-ironic for specific release
 upgrade_project ironic $RUN_DIR $BASE_DEVSTACK_BRANCH $TARGET_DEVSTACK_BRANCH
 
+# NOTE(rloo): make sure it is OK to do an upgrade. Except that we aren't
+# parsing/checking the output of this command because the output could change
+# based on the checks it makes.
+$IRONIC_BIN_DIR/ironic-status upgrade check
+
 $IRONIC_BIN_DIR/ironic-dbsync --config-file=$IRONIC_CONF_FILE
 
 # NOTE(vsaienko) pin_release only on multinode job, for cold upgrade (single node)

doc/source/cli/ironic-status.rst

@@ -75,4 +75,5 @@ Upgrade
 
   **12.0.0 (Stein)**
 
-  * Placeholder to be filled in with checks as they are added in Stein.
+  * Adds a check for compatibility of the object versions with the release
+    of ironic.

ironic/cmd/dbsync.py

@@ -91,14 +91,14 @@ NEW_MODELS = [
 
 class DBCommand(object):
 
-    def _check_versions(self, ignore_missing_tables=False):
+    def check_obj_versions(self, ignore_missing_tables=False):
         """Check the versions of objects.
 
         Check that the object versions are compatible with this release
         of ironic. It does this by comparing the objects' .version field
         in the database, with the expected versions of these objects.
 
-        If it isn't compatible, we exit the program, returning 2.
+        Returns None if compatible; a string describing the issue otherwise.
         """
         if migration.version() is None:
             # no tables, nothing to check
@@ -109,28 +109,35 @@ class DBCommand(object):
         else:
             ignore_models = ()
 
+        msg = None
         try:
             if not dbapi.check_versions(ignore_models=ignore_models):
-                sys.stderr.write(
+                msg = (_('The database is not compatible with this '
-                    _('The database is not compatible with this '
+                         'release of ironic (%s). Please run '
-                      'release of ironic (%s). Please run '
+                         '"ironic-dbsync online_data_migrations" using '
-                      '"ironic-dbsync online_data_migrations" using '
+                         'the previous release.\n')
-                      'the previous release.\n')
+                       % version.version_info.release_string())
-                    % version.version_info.release_string())
-                # NOTE(rloo): We return 1 in online_data_migrations() to
-                # indicate that there are more objects to migrate,
-                # so don't use 1 here.
-                sys.exit(2)
         except exception.DatabaseVersionTooOld:
-            sys.stderr.write(
+            msg = (_('The database version is not compatible with this '
-                _('The database version is not compatible with this '
+                     'release of ironic (%s). This can happen if you are '
-                  'release of ironic (%s). This can happen if you are '
+                     'attempting to upgrade from a version older than '
-                  'attempting to upgrade from a version older than '
+                     'the previous release (skip versions upgrade). '
-                  'the previous release (skip versions upgrade). '
+                     'This is an unsupported upgrade method. '
-                  'This is an unsupported upgrade method. '
+                     'Please run "ironic-dbsync upgrade" using the previous '
-                  'Please run "ironic-dbsync upgrade" using the previous '
+                     'releases for a fast-forward upgrade.\n')
-                  'releases for a fast-forward upgrade.\n')
+                   % version.version_info.release_string())
-                % version.version_info.release_string())
+
+        return msg
+
+    def _check_versions(self, ignore_missing_tables=False):
+        msg = self.check_obj_versions(
+            ignore_missing_tables=ignore_missing_tables)
+        if not msg:
+            return
+        else:
+            sys.stderr.write(msg)
+            # NOTE(rloo): We return 1 in online_data_migrations() to indicate
+            # that there are more objects to migrate, so don't use 1 here.
             sys.exit(2)
 
     def upgrade(self):

ironic/cmd/status.py

@@ -17,6 +17,7 @@ import sys
 from oslo_config import cfg
 from oslo_upgradecheck import upgradecheck
 
+from ironic.cmd import dbsync
 from ironic.common.i18n import _
 
 
@@ -28,21 +29,31 @@ class Checks(upgradecheck.UpgradeCommands):
     and added to _upgrade_checks tuple.
     """
 
-    def _check_placeholder(self):
+    def _check_obj_versions(self):
-        # This is just a placeholder for upgrade checks, it should be
+        """Check that the DB versions of objects are compatible.
-        # removed when the actual checks are added
-        return upgradecheck.Result(upgradecheck.Code.SUCCESS)
 
-    # The format of the check functions is to return an
+        Checks that the object versions are compatible with this
-    # oslo_upgradecheck.upgradecheck.Result
+        release of ironic. It does this by comparing the objects'
-    # object with the appropriate
+        .version field in the database, with the expected versions
-    # oslo_upgradecheck.upgradecheck.Code and details set.
+        of these objects.
-    # If the check hits warnings or failures then those should be stored
+        """
+        msg = dbsync.DBCommand().check_obj_versions(ignore_missing_tables=True)
+
+        if not msg:
+            return upgradecheck.Result(upgradecheck.Code.SUCCESS)
+        else:
+            return upgradecheck.Result(upgradecheck.Code.FAILURE, details=msg)
+
+    # A tuple of check tuples of (<name of check>, <check function>).
+    # The name of the check will be used in the output of this command.
+    # The check function takes no arguments and returns an
+    # oslo_upgradecheck.upgradecheck.Result object with the appropriate
+    # oslo_upgradecheck.upgradecheck.Code and details set. If the
+    # check function hits warnings or failures then those should be stored
     # in the returned Result's "details" attribute. The
     # summary will be rolled up at the end of the check() method.
     _upgrade_checks = (
-        # In the future there should be some real checks added here
+        (_('Object versions'), _check_obj_versions),
-        (_('Placeholder'), _check_placeholder),
     )
 
 

ironic/tests/unit/cmd/test_dbsync.py

@@ -36,29 +36,39 @@ class OnlineMigrationTestCase(db_base.DbTestCase):
         self.context = context.get_admin_context()
         self.db_cmds = dbsync.DBCommand()
 
-    def test__check_versions(self):
+    def test_check_obj_versions(self):
         with mock.patch.object(self.dbapi, 'check_versions',
                                autospec=True) as mock_check_versions:
             mock_check_versions.return_value = True
-            self.db_cmds._check_versions()
+            msg = self.db_cmds.check_obj_versions()
+            self.assertIsNone(msg)
             mock_check_versions.assert_called_once_with(ignore_models=())
 
-    def test__check_versions_bad(self):
+    def test_check_obj_versions_bad(self):
         with mock.patch.object(self.dbapi, 'check_versions',
                                autospec=True) as mock_check_versions:
             mock_check_versions.return_value = False
-            exit = self.assertRaises(SystemExit, self.db_cmds._check_versions)
+            msg = self.db_cmds.check_obj_versions()
+            self.assertIsNotNone(msg)
             mock_check_versions.assert_called_once_with(ignore_models=())
-            self.assertEqual(2, exit.code)
 
-    def test__check_versions_ignore_models(self):
+    def test_check_obj_versions_ignore_models(self):
         with mock.patch.object(self.dbapi, 'check_versions',
                                autospec=True) as mock_check_versions:
             mock_check_versions.return_value = True
-            self.db_cmds._check_versions(True)
+            msg = self.db_cmds.check_obj_versions(ignore_missing_tables=True)
+            self.assertIsNone(msg)
             mock_check_versions.assert_called_once_with(
                 ignore_models=dbsync.NEW_MODELS)
 
+    @mock.patch.object(dbsync.DBCommand, 'check_obj_versions', autospec=True)
+    def test_check_versions_bad(self, mock_check_versions):
+        mock_check_versions.return_value = 'This is bad'
+        exit = self.assertRaises(SystemExit, self.db_cmds._check_versions)
+        mock_check_versions.assert_called_once_with(
+            mock.ANY, ignore_missing_tables=False)
+        self.assertEqual(2, exit.code)
+
     @mock.patch.object(dbsync, 'ONLINE_MIGRATIONS', autospec=True)
     def test__run_migration_functions(self, mock_migrations):
         mock_migrations.__iter__.return_value = ((self.dbapi, 'foo'),)

ironic/tests/unit/cmd/test_status.py

@@ -12,8 +12,10 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
 from oslo_upgradecheck.upgradecheck import Code
 
+from ironic.cmd import dbsync
 from ironic.cmd import status
 from ironic.tests.unit.db import base as db_base
 
@@ -24,7 +26,14 @@ class TestUpgradeChecks(db_base.DbTestCase):
         super(TestUpgradeChecks, self).setUp()
         self.cmd = status.Checks()
 
-    def test__check_placeholder(self):
+    def test__check_obj_versions(self):
-        check_result = self.cmd._check_placeholder()
+        check_result = self.cmd._check_obj_versions()
-        self.assertEqual(
+        self.assertEqual(Code.SUCCESS, check_result.code)
-            Code.SUCCESS, check_result.code)
+
+    @mock.patch.object(dbsync.DBCommand, 'check_obj_versions', autospec=True)
+    def test__check_obj_versions_bad(self, mock_check):
+        msg = 'This is bad'
+        mock_check.return_value = msg
+        check_result = self.cmd._check_obj_versions()
+        self.assertEqual(Code.FAILURE, check_result.code)
+        self.assertEqual(msg, check_result.details)

releasenotes/notes/check_obj_versions-e86d897df673e833.yaml

@@ -0,0 +1,5 @@
+---
+upgrade:
+  - |
+    Adds a check to the ``ironic-status upgrade check`` command, to check for
+    compatibility of the object versions with the release of ironic.