Add 'server shelve --offload', 'server shelve --wait' options

The '--offload' option allows us to explicitly request that the server
be offloaded once shelved or if already shelved.

The '--wait' option allows us to wait for the shelve and/or offload
operations to complete before returning. It is implied when attempting
to offload a server than is not yet shelved.

Change-Id: Id226831e3c09bc95c34b222151b27391a844b073
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>

5 files changed, 221 insertions, 13 deletions

diff --git a/doc/source/cli/data/nova.csv b/doc/source/cli/data/nova.csv
index 2004007f..c319a4a6 100644
--- a/doc/source/cli/data/nova.csv
+++ b/doc/source/cli/data/nova.csv
@@ -114,7 +114,7 @@ service-force-down,compute service set --force,Force service to down.
 service-list,compute service list,Show a list of all running services.
 set-password,server set --root-password,Change the admin password for a server.
 shelve,server shelve,Shelve a server.
-shelve-offload,,Remove a shelved server from the compute node.
+shelve-offload,shelve --offload,Remove a shelved server from the compute node.
 show,server show,Show details about the given server.
 ssh,server ssh,SSH into a server.
 start,server start,Start the server(s).
diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index 50299d65..33545a74 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -3585,25 +3585,115 @@ class SetServer(command.Command):
 
 
 class ShelveServer(command.Command):
-    _description = _("Shelve server(s)")
+    """Shelve and optionally offload server(s).
+
+    Shelving a server creates a snapshot of the server and stores this
+    snapshot before shutting down the server. This shelved server can then be
+    offloaded or deleted from the host, freeing up remaining resources on the
+    host, such as network interfaces. Shelved servers can be unshelved,
+    restoring the server from the snapshot. Shelving is therefore useful where
+    users wish to retain the UUID and IP of a server, without utilizing other
+    resources or disks.
+
+    Most clouds are configured to automatically offload shelved servers
+    immediately or after a small delay. For clouds where this is not
+    configured, or where the delay is larger, offloading can be manually
+    specified. This is an admin-only operation by default.
+    """
 
     def get_parser(self, prog_name):
         parser = super(ShelveServer, self).get_parser(prog_name)
         parser.add_argument(
-            'server',
+            'servers',
             metavar='<server>',
             nargs='+',
             help=_('Server(s) to shelve (name or ID)'),
         )
+        parser.add_argument(
+            '--offload',
+            action='store_true',
+            default=False,
+            help=_(
+                'Remove the shelved server(s) from the host (admin only). '
+                'Invoking this option on an unshelved server(s) will result '
+                'in the server being shelved first'
+            ),
+        )
+        parser.add_argument(
+            '--wait',
+            action='store_true',
+            default=False,
+            help=_('Wait for shelve and/or offload operation to complete'),
+        )
         return parser
 
     def take_action(self, parsed_args):
+
+        def _show_progress(progress):
+            if progress:
+                self.app.stdout.write('\rProgress: %s' % progress)
+                self.app.stdout.flush()
+
         compute_client = self.app.client_manager.compute
-        for server in parsed_args.server:
-            utils.find_resource(
+
+        for server in parsed_args.servers:
+            server_obj = utils.find_resource(
+                compute_client.servers,
+                server,
+            )
+            if server_obj.status.lower() in ('shelved', 'shelved_offloaded'):
+                continue
+
+            server_obj.shelve()
+
+        # if we don't hav to wait, either because it was requested explicitly
+        # or is required implicitly, then our job is done
+        if not parsed_args.wait and not parsed_args.offload:
+            return
+
+        for server in parsed_args.servers:
+            # TODO(stephenfin): We should wait for these in parallel using e.g.
+            # https://review.opendev.org/c/openstack/osc-lib/+/762503/
+            if not utils.wait_for_status(
+                compute_client.servers.get, server_obj.id,
+                success_status=('shelved', 'shelved_offloaded'),
+                callback=_show_progress,
+            ):
+                LOG.error(_('Error shelving server: %s'), server_obj.id)
+                self.app.stdout.write(
+                    _('Error shelving server: %s\n') % server_obj.id)
+                raise SystemExit
+
+        if not parsed_args.offload:
+            return
+
+        for server in parsed_args.servers:
+            server_obj = utils.find_resource(
                 compute_client.servers,
                 server,
-            ).shelve()
+            )
+            if server_obj.status.lower() == 'shelved_offloaded':
+                continue
+
+            server_obj.shelve_offload()
+
+        if not parsed_args.wait:
+            return
+
+        for server in parsed_args.servers:
+            # TODO(stephenfin): We should wait for these in parallel using e.g.
+            # https://review.opendev.org/c/openstack/osc-lib/+/762503/
+            if not utils.wait_for_status(
+                compute_client.servers.get, server_obj.id,
+                success_status=('shelved_offloaded',),
+                callback=_show_progress,
+            ):
+                LOG.error(
+                    _('Error offloading shelved server %s'), server_obj.id)
+                self.app.stdout.write(
+                    _('Error offloading shelved server: %s\n') % (
+                        server_obj.id))
+                raise SystemExit
 
 
 class ShowServer(command.ShowOne):
diff --git a/openstackclient/tests/functional/common/test_help.py b/openstackclient/tests/functional/common/test_help.py
index 3a9aef9e..c55741f1 100644
--- a/openstackclient/tests/functional/common/test_help.py
+++ b/openstackclient/tests/functional/common/test_help.py
@@ -43,7 +43,7 @@ class HelpTests(base.TestCase):
         ('server resize', 'Scale server to a new flavor'),
         ('server resume', 'Resume server(s)'),
         ('server set', 'Set server properties'),
-        ('server shelve', 'Shelve server(s)'),
+        ('server shelve', 'Shelve and optionally offload server(s)'),
         ('server show', 'Show server details'),
         ('server ssh', 'SSH to server'),
         ('server start', 'Start server(s).'),
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 0f33dd70..9ad6d155 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -6434,16 +6434,126 @@ class TestServerShelve(TestServer):
         # Get the command object to test
         self.cmd = server.ShelveServer(self.app, None)
 
-        # Set shelve method to be tested.
-        self.methods = {
+    def test_shelve(self):
+        server_info = {'status': 'ACTIVE'}
+        server_methods = {
             'shelve': None,
+            'shelve_offload': None,
         }
 
-    def test_shelve_one_server(self):
-        self.run_method_with_servers('shelve', 1)
+        server = compute_fakes.FakeServer.create_one_server(
+            attrs=server_info, methods=server_methods)
+        self.servers_mock.get.return_value = server
 
-    def test_shelve_multi_servers(self):
-        self.run_method_with_servers('shelve', 3)
+        arglist = [server.name]
+        verifylist = [
+            ('servers', [server.name]),
+            ('wait', False),
+            ('offload', False),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+        self.assertIsNone(result)
+
+        self.servers_mock.get.assert_called_once_with(server.name)
+        server.shelve.assert_called_once_with()
+        server.shelve_offload.assert_not_called()
+
+    def test_shelve_already_shelved(self):
+        server_info = {'status': 'SHELVED'}
+        server_methods = {
+            'shelve': None,
+            'shelve_offload': None,
+        }
+
+        server = compute_fakes.FakeServer.create_one_server(
+            attrs=server_info, methods=server_methods)
+        self.servers_mock.get.return_value = server
+
+        arglist = [server.name]
+        verifylist = [
+            ('servers', [server.name]),
+            ('wait', False),
+            ('offload', False),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+        self.assertIsNone(result)
+
+        self.servers_mock.get.assert_called_once_with(server.name)
+        server.shelve.assert_not_called()
+        server.shelve_offload.assert_not_called()
+
+    @mock.patch.object(common_utils, 'wait_for_status', return_value=True)
+    def test_shelve_with_wait(self, mock_wait_for_status):
+        server_info = {'status': 'ACTIVE'}
+        server_methods = {
+            'shelve': None,
+            'shelve_offload': None,
+        }
+
+        server = compute_fakes.FakeServer.create_one_server(
+            attrs=server_info, methods=server_methods)
+        self.servers_mock.get.return_value = server
+
+        arglist = ['--wait', server.name]
+        verifylist = [
+            ('servers', [server.name]),
+            ('wait', True),
+            ('offload', False),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+        self.assertIsNone(result)
+
+        self.servers_mock.get.assert_called_once_with(server.name)
+        server.shelve.assert_called_once_with()
+        server.shelve_offload.assert_not_called()
+        mock_wait_for_status.assert_called_once_with(
+            self.servers_mock.get,
+            server.id,
+            callback=mock.ANY,
+            success_status=('shelved', 'shelved_offloaded'),
+        )
+
+    @mock.patch.object(common_utils, 'wait_for_status', return_value=True)
+    def test_shelve_offload(self, mock_wait_for_status):
+        server_info = {'status': 'ACTIVE'}
+        server_methods = {
+            'shelve': None,
+            'shelve_offload': None,
+        }
+
+        server = compute_fakes.FakeServer.create_one_server(
+            attrs=server_info, methods=server_methods)
+        self.servers_mock.get.return_value = server
+
+        arglist = ['--offload', server.name]
+        verifylist = [
+            ('servers', [server.name]),
+            ('wait', False),
+            ('offload', True),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+        self.assertIsNone(result)
+
+        self.servers_mock.get.assert_has_calls([
+            mock.call(server.name),
+            mock.call(server.name),
+        ])
+        server.shelve.assert_called_once_with()
+        server.shelve_offload.assert_called_once_with()
+        mock_wait_for_status.assert_called_once_with(
+            self.servers_mock.get,
+            server.id,
+            callback=mock.ANY,
+            success_status=('shelved', 'shelved_offloaded'),
+        )
 
 
 class TestServerShow(TestServer):
diff --git a/releasenotes/notes/add-shelve-offload-wait-d0a5c8ba92586f72.yaml b/releasenotes/notes/add-shelve-offload-wait-d0a5c8ba92586f72.yaml
new file mode 100644
index 00000000..ddd32931
--- /dev/null
+++ b/releasenotes/notes/add-shelve-offload-wait-d0a5c8ba92586f72.yaml
@@ -0,0 +1,8 @@
+---
+features:
+  - |
+    Add support for ``--offload`` and ``--wait`` options for ``server shelve``.
+    ``--offload`` allows users to explicitly request offloading of a shelved
+    server in environments where automatic offloading is not configured, while
+    ``--wait`` allows users to wait for the shelve and/or shelve offload
+    operations to complete.