From 7b097f016baeddcaca920f02daed3c4fdfe599db Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Wed, 4 Aug 2021 17:07:52 -0700 Subject: [PATCH] Fix upgrade logic to allow for bundled changes The upgrade path logic was built to force a developer pattern to break things such as new tables and features across multiple patches, and the status check *can* explicitly fail if we don't explicitly go hint to it that we've added table. Yes, we have a hard coded list... Anyway, a better pattern is allow the db sync process to do the appropriate needful. Run the status check, if it fails, fallback and update the schema. Also handles the explicit failure error and tries to return a human friendly error message for when the table is not present. In the end this allows us to merge schema changes such as additional tables with their underlying objects and properly handle things as long as the schema update works as expected. This allows us to leverage an operational model of performing upgrades. Change-Id: Id5f2a8068bc064e1ed1e376524850e4739f79ef2 --- devstack/upgrade/upgrade.sh | 7 ++++++- ironic/cmd/status.py | 11 ++++++++++- ...issing-table-in-status-check-512c1732dec56f62.yaml | 6 ++++++ 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/handle-missing-table-in-status-check-512c1732dec56f62.yaml diff --git a/devstack/upgrade/upgrade.sh b/devstack/upgrade/upgrade.sh index 8c6096da83..7801ccd269 100755 --- a/devstack/upgrade/upgrade.sh +++ b/devstack/upgrade/upgrade.sh @@ -81,7 +81,12 @@ if [ $ret_val -gt 1 ] ; then # 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" + echo "WARNING: Status check failed, we're going to attempt to apply the schema update and then re-evaluate." + $IRONIC_BIN_DIR/ironic-dbsync --config-file=$IRONIC_CONF_FILE upgrade + $IRONIC_BIN_DIR/ironic-status upgrade check && ret_val=$? || ret_val=$? + if [ $ret_val -gt 1 ] ; then + die $LINENO "Ironic DB Status check failed, returned: $ret_val" + fi fi $IRONIC_BIN_DIR/ironic-dbsync --config-file=$IRONIC_CONF_FILE diff --git a/ironic/cmd/status.py b/ironic/cmd/status.py index 37fdbaf7dd..10c8a5bfd8 100644 --- a/ironic/cmd/status.py +++ b/ironic/cmd/status.py @@ -19,6 +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 from ironic.cmd import dbsync from ironic.common.i18n import _ @@ -44,7 +45,15 @@ class Checks(upgradecheck.UpgradeCommands): .version field in the database, with the expected versions of these objects. """ - msg = dbsync.DBCommand().check_obj_versions(ignore_missing_tables=True) + try: + # NOTE(TheJulia): Seems an exception is raised by sqlalchemy + # 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: + msg = ('Database table missing. Please ensure you have ' + 'updated the database schema. Not Found: %s' % e) + return upgradecheck.Result(upgradecheck.Code.FAILURE, details=msg) if not msg: return upgradecheck.Result(upgradecheck.Code.SUCCESS) diff --git a/releasenotes/notes/handle-missing-table-in-status-check-512c1732dec56f62.yaml b/releasenotes/notes/handle-missing-table-in-status-check-512c1732dec56f62.yaml new file mode 100644 index 0000000000..026b4386aa --- /dev/null +++ b/releasenotes/notes/handle-missing-table-in-status-check-512c1732dec56f62.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Handles excessively long errors when the status upgrade check is executed, + and simply indicates now if a table is missing, suggesting to update the + database schema before proceeding.