Change nova-manage unexpected error return code to 255

If any nova-manage command fails in an unexpected way and
it bubbles back up to main() the return code will be 1.
There are some commands like archive_deleted_rows,
map_instances and heal_allocations which return 1 for flow
control with automation systems. As a result, those tools
could be calling the command repeatedly getting rc=1 thinking
there is more work to do when really something is failing.

This change makes the unexpected error code 255, updates the
relevant nova-manage command docs that already mention return
codes in some kind of list/table format, and adds an upgrade
release note just to cover our bases in case someone was for
some weird reason relying on 1 specifically for failures rather
than anything greater than 0.

Change-Id: I2937c9ef00f1d1699427f9904cb86fe2f03d9205
Closes-Bug: #1840978

4 files changed, 19 insertions, 8 deletions

diff --git a/doc/source/cli/nova-manage.rst b/doc/source/cli/nova-manage.rst
index f4ae60f813..241d1fc72f 100644
--- a/doc/source/cli/nova-manage.rst
+++ b/doc/source/cli/nova-manage.rst
@@ -99,6 +99,8 @@ Nova Database
            :oslo.config:option:`api_database.connection`.
        * - 4
          - Invalid value for ``--before``.
+       * - 255
+         - An unexpected error occurred.
 
     If automating, this should be run continuously while the result is 1,
     stopping at 0, or use the ``--until-complete`` option.
@@ -267,6 +269,8 @@ Nova Cells v2
     mapped in batches of 50. If you have a large number of instances, consider
     specifying a custom value and run the command until it exits with 0.
 
+    .. todo document return codes since 1 is used for flow control
+
 ``nova-manage cell_v2 map_cell_and_hosts [--name <cell_name>] [--transport-url <transport_url>] [--verbose]``
     Create a cell mapping to the database connection and message queue
     transport url, and map hosts to that cell. The database connection
@@ -483,6 +487,7 @@ Placement
         $ openstack port unset <port_uuid> --binding-profile allocation
 
     * 127: Invalid input.
+    * 255: An unexpected error occurred.
 
 ``nova-manage placement sync_aggregates [--verbose]``
     Mirrors compute host aggregates to resource provider aggregates
@@ -508,6 +513,7 @@ Placement
     * 4: Host mappings not found for one or more host aggregate members
     * 5: Compute node records not found for one or more hosts
     * 6: Resource provider not found by uuid for a given host
+    * 255: An unexpected error occurred.
 
 
 See Also
diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py
index 20056e0ff5..5a76c126b9 100644
--- a/nova/cmd/manage.py
+++ b/nova/cmd/manage.py
@@ -2622,9 +2622,4 @@ def main():
             pdb.post_mortem()
         else:
             print(_("An error has occurred:\n%s") % traceback.format_exc())
-        # FIXME(mriedem): Some commands, like archive_deleted_rows, return 1
-        # for providing flow control to the caller, e.g. continue while rc=1
-        # until you get rc=0. If we fail in some unexpected way and return 1
-        # here, automation tools could continue indefinitely. This should
-        # probably be 255 instead.
-        return 1
+        return 255
diff --git a/nova/tests/unit/test_nova_manage.py b/nova/tests/unit/test_nova_manage.py
index 62e493b2e4..2446d74ac7 100644
--- a/nova/tests/unit/test_nova_manage.py
+++ b/nova/tests/unit/test_nova_manage.py
@@ -2920,7 +2920,7 @@ class TestNovaManageMain(test.NoDBTestCase):
         with mock.patch.object(manage.cmd_common, 'get_action_fn',
                                side_effect=test.TestingException('oops')):
             mock_conf.post_mortem = False
-            self.assertEqual(1, manage.main())
+            self.assertEqual(255, manage.main())
             # assert the traceback is dumped to stdout
             output = self.output.getvalue()
             self.assertIn('An error has occurred', output)
@@ -2934,5 +2934,5 @@ class TestNovaManageMain(test.NoDBTestCase):
         with mock.patch.object(manage.cmd_common, 'get_action_fn',
                                side_effect=test.TestingException('oops')):
             mock_conf.post_mortem = True
-            self.assertEqual(1, manage.main())
+            self.assertEqual(255, manage.main())
             self.assertTrue(mock_pm.called)
diff --git a/releasenotes/notes/bug-1840978-nova-manage-255-88df61a0b69c21c7.yaml b/releasenotes/notes/bug-1840978-nova-manage-255-88df61a0b69c21c7.yaml
new file mode 100644
index 0000000000..9b13466826
--- /dev/null
+++ b/releasenotes/notes/bug-1840978-nova-manage-255-88df61a0b69c21c7.yaml
@@ -0,0 +1,10 @@
+---
+upgrade:
+  - |
+    The ``nova-manage`` set of commands would previously exit with return
+    code 1 due to any unexpected error. However, some commands, such as
+    ``nova-manage db archive_deleted_rows``,
+    ``nova-manage cell_v2 map_instances`` and
+    ``nova-manage placement heal_allocations`` use return code 1 for flow
+    control with automation. As a result, the unexpected error return code
+    has been changed from 1 to 255 for all ``nova-manage`` commands.