From f58cbf1514035af24e37d0d5dcf9e3b5cf88a6d3 Mon Sep 17 00:00:00 2001
From: Julia Kreger <juliaashleykreger@gmail.com>
Date: Tue, 8 Jun 2021 07:57:48 -0700
Subject: [PATCH] Fix ironic-status db index check

When I merged the db status check in for database indexes, I missed
the most improtant line, which is where the object is populated with
the method name to be executed by the upgrade checks framework.

In the rush to try and clean-up after the impact of the Secure RBAC
work, I just didn't manually test the final file I uploaded into
review. I assumed it just worked because the job passed, but didn't
think about the resulting return codes which we *should* experience
on an upgrade from a prior version. Later on, I noticed that because
of the way the status checks are invoked, I also added the code to
do the index check in the wrong order, so I had to restructure things
so the method definition was known by the object on the class which
holds the method names list.

I guess I copied/pasted this over from another file I was testing
in just didn't run the final file. :( Funny enough, the index check
works like a charm now.

Also updates the status check invocation check in the upgrade script
for grenade, *as* warnings *are* permissible and not fatal.

Change-Id: Ifa9da65dc8df5bf9a369d6faeab310386dfd944a
---
 devstack/upgrade/upgrade.sh | 10 +++++++++-
 ironic/cmd/status.py        | 31 ++++++++++++++++---------------
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/devstack/upgrade/upgrade.sh b/devstack/upgrade/upgrade.sh
index 5e70ee4314..8c6096da83 100755
--- a/devstack/upgrade/upgrade.sh
+++ b/devstack/upgrade/upgrade.sh
@@ -74,7 +74,15 @@ 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-status upgrade check && ret_val=$? || ret_val=$?
+if [ $ret_val -gt 1 ] ; then
+    # NOTE(TheJulia): We need to evaluate the return code from the
+    # upgrade status check as the framework defines
+    # Warnings are permissible and returned as status code 1, errors are
+    # returned as greater than 1 which means there is a major upgrade
+    # stopping issue which needs to be addressed.
+    die $LINENO "Ironic DB Status check failed, returned: $ret_val"
+fi
 
 $IRONIC_BIN_DIR/ironic-dbsync --config-file=$IRONIC_CONF_FILE
 
diff --git a/ironic/cmd/status.py b/ironic/cmd/status.py
index 906377994c..37fdbaf7dd 100644
--- a/ironic/cmd/status.py
+++ b/ironic/cmd/status.py
@@ -51,21 +51,6 @@ class Checks(upgradecheck.UpgradeCommands):
         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 = (
-        (_('Object versions'), _check_obj_versions),
-        # Victoria -> Wallaby migration
-        (_('Policy File JSON to YAML Migration'),
-         (common_checks.check_policy_json, {'conf': CONF})),
-    )
-
     def _check_db_indexes(self):
         """Check if indexes exist on heavily used columns.
 
@@ -100,6 +85,22 @@ class Checks(upgradecheck.UpgradeCommands):
         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
+    # 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 = (
+        (_('Object versions'), _check_obj_versions),
+        (_('Database Index Status'), _check_db_indexes),
+        # Victoria -> Wallaby migration
+        (_('Policy File JSON to YAML Migration'),
+         (common_checks.check_policy_json, {'conf': CONF})),
+    )
+
 
 def main():
     return upgradecheck.main(