Microversion 2.73: Support adding the reason behind a server lock

This patch adds a new parameter ``--reason`` to ``openstack server lock``
command and ``--locked``, ``unlocked`` filtering parameters to
``openstack server list`` command. This can help users to provide a
reason when locking the server and to filter instances based on their
locked value from 2.73 microversion.

Implements blueprint add-locked-reason
Depends-On: https://review.opendev.org/#/c/661785/

Change-Id: Ib2714f98b24d47e570da8a6c231e765acd2ff595

Author: tssurya

lower-constraints.txt

@@ -100,7 +100,7 @@ python-mimeparse==1.6.0
 python-mistralclient==3.1.0
 python-muranoclient==0.8.2
 python-neutronclient==6.7.0
-python-novaclient==10.0.0
+python-novaclient==14.1.0
 python-octaviaclient==1.3.0
 python-rsdclient==0.1.0
 python-saharaclient==1.4.0

openstackclient/compute/v2/server.py

@@ -1136,6 +1136,21 @@ def get_parser(self, prog_name):
                    " The provided time should be an ISO 8061 formatted time."
                    " ex 2016-03-04T06:27:59Z .")
         )
+        lock_group = parser.add_mutually_exclusive_group()
+        lock_group.add_argument(
+            '--locked',
+            action='store_true',
+            default=False,
+            help=_('Only display locked servers. '
+                   'Requires ``--os-compute-api-version`` 2.73 or greater.'),
+        )
+        lock_group.add_argument(
+            '--unlocked',
+            action='store_true',
+            default=False,
+            help=_('Only display unlocked servers. '
+                   'Requires ``--os-compute-api-version`` 2.73 or greater.'),
+        )
         return parser
 
     def take_action(self, parsed_args):
@@ -1190,6 +1205,18 @@ def take_action(self, parsed_args):
             'deleted': parsed_args.deleted,
             'changes-since': parsed_args.changes_since,
         }
+        support_locked = (compute_client.api_version >=
+                          api_versions.APIVersion('2.73'))
+        if not support_locked and (parsed_args.locked or parsed_args.unlocked):
+            msg = _('--os-compute-api-version 2.73 or greater is required to '
+                    'use the (un)locked filter option.')
+            raise exceptions.CommandError(msg)
+        elif support_locked:
+            # Only from 2.73.
+            if parsed_args.locked:
+                search_opts['locked'] = True
+            if parsed_args.unlocked:
+                search_opts['locked'] = False
         LOG.debug('search options: %s', search_opts)
 
         if search_opts['changes-since']:
@@ -1374,16 +1401,28 @@ def get_parser(self, prog_name):
             nargs='+',
             help=_('Server(s) to lock (name or ID)'),
         )
+        parser.add_argument(
+            '--reason',
+            metavar='<reason>',
+            default=None,
+            help=_("Reason for locking the server(s). Requires "
+                   "``--os-compute-api-version`` 2.73 or greater.")
+        )
         return parser
 
     def take_action(self, parsed_args):
 
         compute_client = self.app.client_manager.compute
+        support_reason = compute_client.api_version >= api_versions.APIVersion(
+            '2.73')
+        if not support_reason and parsed_args.reason:
+            msg = _('--os-compute-api-version 2.73 or greater is required to '
+                    'use the --reason option.')
+            raise exceptions.CommandError(msg)
         for server in parsed_args.server:
-            utils.find_resource(
-                compute_client.servers,
-                server,
-            ).lock()
+            serv = utils.find_resource(compute_client.servers, server)
+            (serv.lock(reason=parsed_args.reason) if support_reason
+                else serv.lock())
 
 
 # FIXME(dtroyer): Here is what I want, how with argparse/cliff?

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

@@ -90,7 +90,14 @@ def run_method_with_servers(self, method_name, server_count):
 
         for s in servers:
             method = getattr(s, method_name)
-            method.assert_called_with()
+            if method_name == 'lock':
+                version = self.app.client_manager.compute.api_version
+                if version >= api_versions.APIVersion('2.73'):
+                    method.assert_called_with(reason=None)
+                else:
+                    method.assert_called_with()
+            else:
+                method.assert_called_with()
         self.assertIsNone(result)
 
 
@@ -2210,6 +2217,80 @@ def test_server_list_with_image(self):
         self.assertEqual(self.columns, columns)
         self.assertEqual(tuple(self.data), tuple(data))
 
+    def test_server_list_with_locked_pre_v273(self):
+
+        arglist = [
+            '--locked'
+        ]
+        verifylist = [
+            ('locked', True)
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        ex = self.assertRaises(exceptions.CommandError,
+                               self.cmd.take_action,
+                               parsed_args)
+        self.assertIn(
+            '--os-compute-api-version 2.73 or greater is required', str(ex))
+
+    def test_server_list_with_locked_v273(self):
+
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.73')
+        arglist = [
+            '--locked'
+        ]
+        verifylist = [
+            ('locked', True)
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.search_opts['locked'] = True
+        self.servers_mock.list.assert_called_with(**self.kwargs)
+
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(tuple(self.data), tuple(data))
+
+    def test_server_list_with_unlocked_v273(self):
+
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.73')
+        arglist = [
+            '--unlocked'
+        ]
+        verifylist = [
+            ('unlocked', True)
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.search_opts['locked'] = False
+        self.servers_mock.list.assert_called_with(**self.kwargs)
+
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(tuple(self.data), tuple(data))
+
+    def test_server_list_with_locked_and_unlocked_v273(self):
+
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.73')
+        arglist = [
+            '--locked',
+            '--unlocked'
+        ]
+        verifylist = [
+            ('locked', True),
+            ('unlocked', True)
+        ]
+
+        ex = self.assertRaises(
+            utils.ParserException,
+            self.check_parser, self.cmd, arglist, verifylist)
+        self.assertIn('Argument parse failed', str(ex))
+
     def test_server_list_with_flavor(self):
 
         arglist = [
@@ -2336,6 +2417,72 @@ def test_server_lock_one_server(self):
     def test_server_lock_multi_servers(self):
         self.run_method_with_servers('lock', 3)
 
+    def test_server_lock_with_reason(self):
+        server = compute_fakes.FakeServer.create_one_server()
+        arglist = [
+            server.id,
+            '--reason', "blah",
+        ]
+        verifylist = [
+            ('reason', "blah"),
+            ('server', [server.id])
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        ex = self.assertRaises(exceptions.CommandError,
+                               self.cmd.take_action,
+                               parsed_args)
+        self.assertIn(
+            '--os-compute-api-version 2.73 or greater is required', str(ex))
+
+
+class TestServerLockV273(TestServerLock):
+
+    def setUp(self):
+        super(TestServerLockV273, self).setUp()
+
+        self.server = compute_fakes.FakeServer.create_one_server(
+            methods=self.methods)
+
+        # This is the return value for utils.find_resource()
+        self.servers_mock.get.return_value = self.server
+
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.73')
+
+        # Get the command object to test
+        self.cmd = server.LockServer(self.app, None)
+
+    def test_server_lock_with_reason(self):
+        arglist = [
+            self.server.id,
+            '--reason', "blah",
+        ]
+        verifylist = [
+            ('reason', "blah"),
+            ('server', [self.server.id])
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.cmd.take_action(parsed_args)
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.server.lock.assert_called_with(reason="blah")
+
+    def test_server_lock_multi_servers_with_reason(self):
+        server2 = compute_fakes.FakeServer.create_one_server(
+            methods=self.methods)
+        arglist = [
+            self.server.id, server2.id,
+            '--reason', "choo..choo",
+        ]
+        verifylist = [
+            ('reason', "choo..choo"),
+            ('server', [self.server.id, server2.id])
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.cmd.take_action(parsed_args)
+        self.assertEqual(2, self.servers_mock.get.call_count)
+        self.server.lock.assert_called_with(reason="choo..choo")
+        self.assertEqual(2, self.server.lock.call_count)
+
 
 class TestServerMigrate(TestServer):
 

releasenotes/notes/bp-add-locked-reason-425efd2def1144f1.yaml

@@ -0,0 +1,13 @@
+---
+features:
+  - Add ``--reason`` option to the ``server lock`` command to specify a reason
+    when locking a server.
+    Requires ``–os-compute-api-version`` 2.73 or greater.
+  - Add ``--locked`` option to the ``server list`` command to list only locked
+    servers.
+    Requires ``–os-compute-api-version`` 2.73 or greater.
+  - Add ``--unlocked`` option to the ``server list`` command list only unlocked
+    servers.
+    Requires ``–os-compute-api-version`` 2.73 or greater.
+    [Blueprint `add-locked-reason <https://blueprints.launchpad.net/nova/+spec/add-locked-reason>`_]
+ 

requirements.txt

@@ -13,5 +13,5 @@ oslo.i18n>=3.15.3 # Apache-2.0
 oslo.utils>=3.33.0 # Apache-2.0
 python-glanceclient>=2.8.0 # Apache-2.0
 python-keystoneclient>=3.17.0 # Apache-2.0
-python-novaclient>=10.0.0 # Apache-2.0
+python-novaclient>=14.1.0 # Apache-2.0
 python-cinderclient>=3.3.0 # Apache-2.0