diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 85693e1703..c4bf5da94f 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -2815,24 +2815,33 @@ A non-admin user will not be able to execute actions.""") '--reason', metavar='', default=None, - help=_("Reason for locking the server(s). Requires " - "``--os-compute-api-version`` 2.73 or greater.") + help=_( + '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 + + kwargs = {} + if parsed_args.reason: + if not sdk_utils.supports_microversion(compute_client, '2.73'): + msg = _( + '--os-compute-api-version 2.73 or greater is required to ' + 'use the --reason option' + ) + raise exceptions.CommandError(msg) + + kwargs['locked_reason'] = parsed_args.reason - 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: - serv = utils.find_resource(compute_client.servers, server) - (serv.lock(reason=parsed_args.reason) if support_reason - else serv.lock()) + server_id = compute_client.find_server( + server, + 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='', @@ -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( - compute_client.servers, + server_id = compute_client.find_server( server, - ).unlock() + ignore_missing=False, + ).id + compute_client.unlock_server(server_id) class UnpauseServer(command.Command): diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 00f9b3a75f..01c71b6c96 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/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': - 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': + if 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. - self.methods = { - 'lock': None, - } + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_lock(self, sm_mock): + sm_mock.return_value = False + self.run_method_with_sdk_servers('lock_server', 1) - def test_server_lock_one_server(self): - self.run_method_with_servers('lock', 1) + @mock.patch.object(sdk_utils, 'supports_microversion') + 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): - self.run_method_with_servers('lock', 3) + @mock.patch.object(sdk_utils, 'supports_microversion') + 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): - server = compute_fakes.FakeServer.create_one_server() + @mock.patch.object(sdk_utils, 'supports_microversion') + 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, - self.cmd.take_action, - parsed_args) + 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) + '--os-compute-api-version 2.73 or greater is required', str(ex), + ) 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): diff --git a/releasenotes/notes/switch-server-lock-to-sdk-d5dd17e4987233a5.yaml b/releasenotes/notes/switch-server-lock-to-sdk-d5dd17e4987233a5.yaml new file mode 100644 index 0000000000..8e9a519af4 --- /dev/null +++ b/releasenotes/notes/switch-server-lock-to-sdk-d5dd17e4987233a5.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + The ``server lock`` and ``server unlock`` commands now use SDK.