Switch server lock, unlock to sdk

Switch server lock commands from novaclient to SDK.

Change-Id: I042db99c9d7a0d8d207f8cdf69d786bd4fe904e7
Depends-On: https://review.opendev.org/c/openstack/openstacksdk/+/867890

openstackclient/compute/v2/server.py

@@ -2815,24 +2815,33 @@ A non-admin user will not be able to execute actions.""")
             '--reason',
             metavar='<reason>',
             default=None,
-            help=_("Reason for locking the server(s). Requires "
+            help=_(
-                   "``--os-compute-api-version`` 2.73 or greater.")
+                'Reason for locking the server(s) '
+                '(supported by --os-compute-api-version 2.73 or above)'
+            ),
         )
         return parser
 
     def take_action(self, parsed_args):
+        compute_client = self.app.client_manager.sdk_connection.compute
 
-        compute_client = self.app.client_manager.compute
+        kwargs = {}
-        support_reason = compute_client.api_version >= api_versions.APIVersion(
+        if parsed_args.reason:
-            '2.73')
+            if not sdk_utils.supports_microversion(compute_client, '2.73'):
-        if not support_reason and parsed_args.reason:
+                msg = _(
-            msg = _('--os-compute-api-version 2.73 or greater is required to '
+                    '--os-compute-api-version 2.73 or greater is required to '
-                    'use the --reason option.')
+                    'use the --reason option'
+                )
                 raise exceptions.CommandError(msg)
+
+            kwargs['locked_reason'] = parsed_args.reason
+
         for server in parsed_args.server:
-            serv = utils.find_resource(compute_client.servers, server)
+            server_id = compute_client.find_server(
-            (serv.lock(reason=parsed_args.reason) if support_reason
+                server,
-                else serv.lock())
+                ignore_missing=False
+            ).id
+            compute_client.lock_server(server_id, **kwargs)
 
 
 # FIXME(dtroyer): Here is what I want, how with argparse/cliff?
@@ -4660,7 +4669,7 @@ class UnlockServer(command.Command):
     _description = _("Unlock server(s)")
 
     def get_parser(self, prog_name):
-        parser = super(UnlockServer, self).get_parser(prog_name)
+        parser = super().get_parser(prog_name)
         parser.add_argument(
             'server',
             metavar='<server>',
@@ -4670,12 +4679,13 @@ class UnlockServer(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.compute
+        compute_client = self.app.client_manager.sdk_connection.compute
         for server in parsed_args.server:
-            utils.find_resource(
+            server_id = compute_client.find_server(
-                compute_client.servers,
                 server,
-            ).unlock()
+                ignore_missing=False,
+            ).id
+            compute_client.unlock_server(server_id)
 
 
 class UnpauseServer(command.Command):

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

@@ -196,11 +196,7 @@ class TestServer(compute_fakes.TestComputev2):
 
         for s in servers:
             method = getattr(s, method_name)
-            if method_name == 'lock':
+            if method_name == 'unshelve':
-                version = self.app.client_manager.compute.api_version
-                if version >= api_versions.APIVersion('2.73'):
-                    method.assert_called_with(reason=None)
-            elif method_name == 'unshelve':
                 version = self.app.client_manager.compute.api_version
                 if version >= api_versions.APIVersion('2.91'):
                     method.assert_called_with(availability_zone=_sentinel,
@@ -5441,87 +5437,94 @@ class TestServerListV273(_TestServerList):
 class TestServerLock(TestServer):
 
     def setUp(self):
-        super(TestServerLock, self).setUp()
+        super().setUp()
+
+        self.server = compute_fakes.FakeServer.create_one_sdk_server()
+
+        self.app.client_manager.sdk_connection = mock.Mock()
+        self.app.client_manager.sdk_connection.compute = mock.Mock()
+        self.sdk_client = self.app.client_manager.sdk_connection.compute
+
+        self.sdk_client.find_server.return_value = self.server
+        self.sdk_client.lock_server.return_value = None
 
         # Get the command object to test
         self.cmd = server.LockServer(self.app, None)
 
-        # Set methods to be tested.
+    @mock.patch.object(sdk_utils, 'supports_microversion')
-        self.methods = {
+    def test_server_lock(self, sm_mock):
-            'lock': None,
+        sm_mock.return_value = False
-        }
+        self.run_method_with_sdk_servers('lock_server', 1)
 
-    def test_server_lock_one_server(self):
+    @mock.patch.object(sdk_utils, 'supports_microversion')
-        self.run_method_with_servers('lock', 1)
+    def test_server_lock_multi_servers(self, sm_mock):
+        sm_mock.return_value = False
+        self.run_method_with_sdk_servers('lock_server', 3)
 
-    def test_server_lock_multi_servers(self):
+    @mock.patch.object(sdk_utils, 'supports_microversion')
-        self.run_method_with_servers('lock', 3)
+    def test_server_lock_with_reason(self, sm_mock):
+        sm_mock.return_value = True
+        arglist = [
+            self.server.id,
+            '--reason', 'blah',
+        ]
+        verifylist = [
+            ('server', [self.server.id]),
+            ('reason', 'blah'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.cmd.take_action(parsed_args)
+        self.sdk_client.find_server.assert_called_with(
+            self.server.id,
+            ignore_missing=False,
+        )
+        self.sdk_client.lock_server.assert_called_with(
+            self.server.id,
+            locked_reason="blah",
+        )
 
-    def test_server_lock_with_reason(self):
+    @mock.patch.object(sdk_utils, 'supports_microversion')
-        server = compute_fakes.FakeServer.create_one_server()
+    def test_server_lock_with_reason_multi_servers(self, sm_mock):
+        sm_mock.return_value = True
+        server2 = compute_fakes.FakeServer.create_one_sdk_server()
+        arglist = [
+            self.server.id, server2.id,
+            '--reason', 'choo..choo',
+        ]
+        verifylist = [
+            ('server', [self.server.id, server2.id]),
+            ('reason', 'choo..choo'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.cmd.take_action(parsed_args)
+        self.assertEqual(2, self.sdk_client.find_server.call_count)
+        self.sdk_client.lock_server.assert_called_with(
+            self.server.id,
+            locked_reason="choo..choo",
+        )
+        self.assertEqual(2, self.sdk_client.lock_server.call_count)
+
+    @mock.patch.object(sdk_utils, 'supports_microversion')
+    def test_server_lock_with_reason_pre_v273(self, sm_mock):
+        sm_mock.return_value = False
+        server = compute_fakes.FakeServer.create_one_sdk_server()
         arglist = [
             server.id,
             '--reason', "blah",
         ]
         verifylist = [
+            ('server', [server.id]),
             ('reason', "blah"),
-            ('server', [server.id])
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-        ex = self.assertRaises(exceptions.CommandError,
+        ex = self.assertRaises(
+            exceptions.CommandError,
             self.cmd.take_action,
-                               parsed_args)
+            parsed_args,
+        )
         self.assertIn(
-            '--os-compute-api-version 2.73 or greater is required', str(ex))
+            '--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):
@@ -8318,21 +8321,16 @@ class TestServerSuspend(TestServer):
 class TestServerUnlock(TestServer):
 
     def setUp(self):
-        super(TestServerUnlock, self).setUp()
+        super().setUp()
 
         # Get the command object to test
         self.cmd = server.UnlockServer(self.app, None)
 
-        # Set methods to be tested.
-        self.methods = {
-            'unlock': None,
-        }
-
     def test_server_unlock_one_server(self):
-        self.run_method_with_servers('unlock', 1)
+        self.run_method_with_sdk_servers('unlock_server', 1)
 
     def test_server_unlock_multi_servers(self):
-        self.run_method_with_servers('unlock', 3)
+        self.run_method_with_sdk_servers('unlock_server', 3)
 
 
 class TestServerUnpause(TestServer):

releasenotes/notes/switch-server-lock-to-sdk-d5dd17e4987233a5.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    The ``server lock`` and ``server unlock`` commands now use SDK.