Allow server rebuild --wait for SHUTOFF servers

currently the command is waiting only for ACTIVE server status,
but if the server was SHUTOFF before, it will be SHUTOFF after
rebuild as well, so the command is stuck in waiting forever.

Additionally, we now also pre-validate the server status on client side,
and raise an error if the server to be rebuilt is not in ACTIVE, ERROR
or SHUTOFF state.

Change-Id: If90a4bbba9a7ecd972f8b594c52fee4f75a0ae5e
Co-Authored-By: Oleksiy Molchanov <omolchanov@mirantis.com>
Story: 2010751
Task: 48005

3 files changed, 105 insertions, 0 deletions

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index cde4ab05..73d66d19 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -3536,6 +3536,15 @@ class RebuildServer(command.ShowOne):
                         'future release.'
                     )
 
+        status = getattr(server, 'status', '').lower()
+        if status == 'shutoff':
+            success_status = ['shutoff']
+        elif status in ('error', 'active'):
+            success_status = ['active']
+        else:
+            msg = _("The server status is not ACTIVE, SHUTOFF or ERROR.")
+            raise exceptions.CommandError(msg)
+
         try:
             server = server.rebuild(image, parsed_args.password, **kwargs)
         finally:
@@ -3547,6 +3556,7 @@ class RebuildServer(command.ShowOne):
                 compute_client.servers.get,
                 server.id,
                 callback=_show_progress,
+                success_status=success_status,
             ):
                 self.app.stdout.write(_('Complete\n'))
             else:
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 235e24c6..0b193753 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -6250,6 +6250,7 @@ class TestServerRebuild(TestServer):
 
         # Fake the server to be rebuilt. The IDs of them should be the same.
         attrs['id'] = new_server.id
+        attrs['status'] = 'ACTIVE'
         methods = {
             'rebuild': new_server,
         }
@@ -6448,6 +6449,7 @@ class TestServerRebuild(TestServer):
             self.servers_mock.get,
             self.server.id,
             callback=mock.ANY,
+            success_status=['active'],
             # **kwargs
         )
 
@@ -6473,12 +6475,91 @@ class TestServerRebuild(TestServer):
             self.servers_mock.get,
             self.server.id,
             callback=mock.ANY,
+            success_status=['active'],
         )
 
         self.servers_mock.get.assert_called_with(self.server.id)
         self.get_image_mock.assert_called_with(self.image.id)
         self.server.rebuild.assert_called_with(self.image, None)
 
+    @mock.patch.object(common_utils, 'wait_for_status', return_value=True)
+    def test_rebuild_with_wait_shutoff_status(self, mock_wait_for_status):
+        self.server.status = 'SHUTOFF'
+        arglist = [
+            '--wait',
+            self.server.id,
+        ]
+        verifylist = [
+            ('wait', True),
+            ('server', self.server.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # Get the command object to test.
+        self.cmd.take_action(parsed_args)
+
+        # kwargs = dict(success_status=['active', 'verify_resize'],)
+
+        mock_wait_for_status.assert_called_once_with(
+            self.servers_mock.get,
+            self.server.id,
+            callback=mock.ANY,
+            success_status=['shutoff'],
+            # **kwargs
+        )
+
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.get_image_mock.assert_called_with(self.image.id)
+        self.server.rebuild.assert_called_with(self.image, None)
+
+    @mock.patch.object(common_utils, 'wait_for_status', return_value=True)
+    def test_rebuild_with_wait_error_status(self, mock_wait_for_status):
+        self.server.status = 'ERROR'
+        arglist = [
+            '--wait',
+            self.server.id,
+        ]
+        verifylist = [
+            ('wait', True),
+            ('server', self.server.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # Get the command object to test.
+        self.cmd.take_action(parsed_args)
+
+        # kwargs = dict(success_status=['active', 'verify_resize'],)
+
+        mock_wait_for_status.assert_called_once_with(
+            self.servers_mock.get,
+            self.server.id,
+            callback=mock.ANY,
+            success_status=['active'],
+            # **kwargs
+        )
+
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.get_image_mock.assert_called_with(self.image.id)
+        self.server.rebuild.assert_called_with(self.image, None)
+
+    def test_rebuild_wrong_status_fails(self):
+        self.server.status = 'SHELVED'
+        arglist = [
+            self.server.id,
+        ]
+        verifylist = [
+            ('server', self.server.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            exceptions.CommandError, self.cmd.take_action, parsed_args
+        )
+
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.get_image_mock.assert_called_with(self.image.id)
+        self.server.rebuild.assert_not_called()
+
     def test_rebuild_with_property(self):
         arglist = [
             self.server.id,
@@ -6837,6 +6918,7 @@ class TestServerRebuildVolumeBacked(TestServer):
 
         # Fake the server to be rebuilt. The IDs of them should be the same.
         attrs['id'] = new_server.id
+        attrs['status'] = 'ACTIVE'
         methods = {
             'rebuild': new_server,
         }
diff --git a/releasenotes/notes/story-2010751-server-rebuild-wait-shutoff-c84cddcd3f15e9ce.yaml b/releasenotes/notes/story-2010751-server-rebuild-wait-shutoff-c84cddcd3f15e9ce.yaml
new file mode 100644
index 00000000..58c67e27
--- /dev/null
+++ b/releasenotes/notes/story-2010751-server-rebuild-wait-shutoff-c84cddcd3f15e9ce.yaml
@@ -0,0 +1,13 @@
+---
+features:
+  - |
+    ``openstack server rebuild`` command now fails early if the server is
+    not in a state supported for rebuild - either ``ACTIVE``, ``ERROR`` or
+    ``SHUTOFF``.
+    See `OpenStack Compute API reference for server rebuild action
+    <https://docs.openstack.org/api-ref/compute/?expanded=rebuild-server-rebuild-action-detail#rebuild-server-rebuild-action>`_.
+fixes:
+  - |
+    ``openstack server rebuild --wait`` now properly works for servers in
+    ``SHUTOFF`` state without hanging.
+    [Story `2010751 <https://storyboard.openstack.org/#!/story/2010751>`_]