Merge "Allow server rebuild --wait for SHUTOFF servers"

Author: Zuul

openstackclient/compute/v2/server.py

@@ -3536,6 +3536,15 @@ def _show_progress(progress):
                         '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 @@ def _show_progress(progress):
                 compute_client.servers.get,
                 server.id,
                 callback=_show_progress,
+                success_status=success_status,
             ):
                 self.app.stdout.write(_('Complete\n'))
             else:

openstackclient/tests/unit/compute/v2/test_server.py

@@ -6241,6 +6241,7 @@ def setUp(self):
 
         # 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,
         }
@@ -6439,6 +6440,7 @@ def test_rebuild_with_wait_ok(self, mock_wait_for_status):
             self.servers_mock.get,
             self.server.id,
             callback=mock.ANY,
+            success_status=['active'],
             # **kwargs
         )
 
@@ -6464,12 +6466,91 @@ def test_rebuild_with_wait_fails(self, mock_wait_for_status):
             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,
@@ -6828,6 +6909,7 @@ def setUp(self):
 
         # 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,
         }

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>`_]