Add upgrade check warning for allocations db

Adding an upgrade check to provide awareness to the state of
the database in regards if an unexpected engine is in use or
if the character set encoding is also not UTF8.

These will raise non-fatal warnings on the upgrade status
check.

Change-Id: Ide0eb4690a056be557e5ea7d5ba5f6be37b50d0a
Story: 2010384

4 files changed, 136 insertions, 2 deletions

diff --git a/devstack/lib/ironic b/devstack/lib/ironic
index 08cccce7a..154dc665d 100644
--- a/devstack/lib/ironic
+++ b/devstack/lib/ironic
@@ -1924,6 +1924,11 @@ function init_ironic {
     # 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_BIN_DIR/ironic-status upgrade check && ret_val=$? || ret_val=$?
+    if [ $ret_val -gt 1 ] ; then
+        die $LINENO "The `ironic-status upgrade check` command returned an error. Cannot proceed."
+    fi
 }
 
 # _ironic_bm_vm_names() - Generates list of names for baremetal VMs.
diff --git a/ironic/cmd/status.py b/ironic/cmd/status.py
index 10c8a5bfd..84a564c17 100644
--- a/ironic/cmd/status.py
+++ b/ironic/cmd/status.py
@@ -19,7 +19,7 @@ from oslo_db.sqlalchemy import enginefacade
 from oslo_db.sqlalchemy import utils
 from oslo_upgradecheck import common_checks
 from oslo_upgradecheck import upgradecheck
-from sqlalchemy import exc as sa_exc
+import sqlalchemy
 
 from ironic.cmd import dbsync
 from ironic.common.i18n import _
@@ -50,7 +50,7 @@ class Checks(upgradecheck.UpgradeCommands):
             # when a table is missing, so lets catch it, since it is fatal.
             msg = dbsync.DBCommand().check_obj_versions(
                 ignore_missing_tables=True)
-        except sa_exc.NoSuchTableError as e:
+        except sqlalchemy.exc.NoSuchTableError as e:
             msg = ('Database table missing. Please ensure you have '
                    'updated the database schema. Not Found: %s' % e)
             return upgradecheck.Result(upgradecheck.Code.FAILURE, details=msg)
@@ -94,6 +94,43 @@ class Checks(upgradecheck.UpgradeCommands):
         else:
             return upgradecheck.Result(upgradecheck.Code.SUCCESS)
 
+    def _check_allocations_table(self):
+        msg = None
+        engine = enginefacade.reader.get_engine()
+        if 'mysql' not in str(engine.url):
+            # This test only applies to mysql and database schema
+            # selection.
+            return upgradecheck.Result(upgradecheck.Code.SUCCESS)
+        res = engine.execute("show create table allocations")
+        results = str(res.all()).lower()
+        print('####################################################33')
+        print(results)
+        if 'utf8' not in results:
+            msg = ('The Allocations table is is not using UTF8 encoding. '
+                   'This is corrected in later versions of Ironic, where '
+                   'the table character set schema is automatically '
+                   'migrated. Continued use of a non-UTF8 character '
+                   'set may produce unexpected results.')
+
+        if 'innodb' not in results:
+            warning = ('The engine used by MySQL for the allocations '
+                       'table is not the intended engine for the Ironic '
+                       'database tables to use. This may have been a result '
+                       'of an error with the table creation schema. This '
+                       'may require Database Administrator intervention '
+                       'and downtime to dump, modify the table engine to '
+                       'utilize InnoDB, and reload the allocations table to '
+                       'utilize the InnoDB engine.')
+            if msg:
+                msg = msg + ' Additionally: ' + warning
+            else:
+                msg = warning
+
+        if msg:
+            return upgradecheck.Result(upgradecheck.Code.WARNING, details=msg)
+        else:
+            return upgradecheck.Result(upgradecheck.Code.SUCCESS)
+
     # 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
@@ -105,6 +142,8 @@ class Checks(upgradecheck.UpgradeCommands):
     _upgrade_checks = (
         (_('Object versions'), _check_obj_versions),
         (_('Database Index Status'), _check_db_indexes),
+        (_('Allocations Name Field Length Check'),
+         _check_allocations_table),
         # Victoria -> Wallaby migration
         (_('Policy File JSON to YAML Migration'),
          (common_checks.check_policy_json, {'conf': CONF})),
diff --git a/ironic/tests/unit/cmd/test_status.py b/ironic/tests/unit/cmd/test_status.py
index f776e2d51..2d044cc13 100644
--- a/ironic/tests/unit/cmd/test_status.py
+++ b/ironic/tests/unit/cmd/test_status.py
@@ -14,6 +14,7 @@
 
 from unittest import mock
 
+from oslo_db import sqlalchemy
 from oslo_upgradecheck.upgradecheck import Code
 
 from ironic.cmd import dbsync
@@ -38,3 +39,84 @@ class TestUpgradeChecks(db_base.DbTestCase):
         check_result = self.cmd._check_obj_versions()
         self.assertEqual(Code.FAILURE, check_result.code)
         self.assertEqual(msg, check_result.details)
+
+    def test__check_allocations_table_ok(self):
+        check_result = self.cmd._check_allocations_table()
+        self.assertEqual(Code.SUCCESS,
+                         check_result.code)
+
+    @mock.patch.object(sqlalchemy.enginefacade.reader,
+                       'get_engine', autospec=True)
+    def test__check_allocations_table_latin1(self, mock_reader):
+        mock_engine = mock.Mock()
+        mock_res = mock.Mock()
+        mock_res.all.return_value = (
+            '... ENGINE=InnoDB DEFAULT CHARSET=latin1',
+        )
+        mock_engine.url = '..mysql..'
+        mock_engine.execute.return_value = mock_res
+        mock_reader.return_value = mock_engine
+        check_result = self.cmd._check_allocations_table()
+        self.assertEqual(Code.WARNING,
+                         check_result.code)
+        expected_msg = ('The Allocations table is is not using UTF8 '
+                        'encoding. This is corrected in later versions '
+                        'of Ironic, where the table character set schema '
+                        'is automatically migrated. Continued use of a '
+                        'non-UTF8 character set may produce unexpected '
+                        'results.')
+        self.assertEqual(expected_msg, check_result.details)
+
+    @mock.patch.object(sqlalchemy.enginefacade.reader,
+                       'get_engine', autospec=True)
+    def test__check_allocations_table_myiasm(self, mock_reader):
+        mock_engine = mock.Mock()
+        mock_res = mock.Mock()
+        mock_engine.url = '..mysql..'
+        mock_res.all.return_value = (
+            '... ENGINE=MyIASM DEFAULT CHARSET=utf8',
+        )
+        mock_engine.execute.return_value = mock_res
+        mock_reader.return_value = mock_engine
+        check_result = self.cmd._check_allocations_table()
+        self.assertEqual(Code.WARNING,
+                         check_result.code)
+        expected_msg = ('The engine used by MySQL for the allocations '
+                        'table is not the intended engine for the Ironic '
+                        'database tables to use. This may have been a '
+                        'result of an error with the table creation schema. '
+                        'This may require Database Administrator '
+                        'intervention and downtime to dump, modify the '
+                        'table engine to utilize InnoDB, and reload the '
+                        'allocations table to utilize the InnoDB engine.')
+        self.assertEqual(expected_msg, check_result.details)
+
+    @mock.patch.object(sqlalchemy.enginefacade.reader,
+                       'get_engine', autospec=True)
+    def test__check_allocations_table_myiasm_both(self, mock_reader):
+        mock_engine = mock.Mock()
+        mock_res = mock.Mock()
+        mock_engine.url = '..mysql..'
+        mock_res.all.return_value = (
+            '... ENGINE=MyIASM DEFAULT CHARSET=latin1',
+        )
+        mock_engine.execute.return_value = mock_res
+        mock_reader.return_value = mock_engine
+        check_result = self.cmd._check_allocations_table()
+        self.assertEqual(Code.WARNING,
+                         check_result.code)
+        expected_msg = ('The Allocations table is is not using UTF8 '
+                        'encoding. This is corrected in later versions '
+                        'of Ironic, where the table character set schema '
+                        'is automatically migrated. Continued use of a '
+                        'non-UTF8 character set may produce unexpected '
+                        'results. Additionally: '
+                        'The engine used by MySQL for the allocations '
+                        'table is not the intended engine for the Ironic '
+                        'database tables to use. This may have been a '
+                        'result of an error with the table creation schema. '
+                        'This may require Database Administrator '
+                        'intervention and downtime to dump, modify the '
+                        'table engine to utilize InnoDB, and reload the '
+                        'allocations table to utilize the InnoDB engine.')
+        self.assertEqual(expected_msg, check_result.details)
diff --git a/releasenotes/notes/add-allocations-table-check-38f1c9eef189b411.yaml b/releasenotes/notes/add-allocations-table-check-38f1c9eef189b411.yaml
new file mode 100644
index 000000000..46046bd2a
--- /dev/null
+++ b/releasenotes/notes/add-allocations-table-check-38f1c9eef189b411.yaml
@@ -0,0 +1,8 @@
+---
+upgrade:
+  - |
+    Adds an upgrade status check for the Allocation table engine and
+    character set encoding on MySQL. This is a result of a missing
+    encoding definition on the table schema when originally created.
+    This issue will be remedied, in part, in a later version of Ironic,
+    but the upgrade status check will provide advance operator visibility.