From ce8171bad90f15ac8a753f50bb3792862b66cc7c Mon Sep 17 00:00:00 2001 From: suneethravi Date: Sat, 27 Nov 2021 21:04:25 +0000 Subject: 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 | 44 ++++--- .../tests/unit/compute/v2/test_server.py | 138 ++++++++++----------- 2 files changed, 95 insertions(+), 87 deletions(-) (limited to 'openstackclient') diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 85693e17..c4bf5da9 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 00f9b3a7..01c71b6c 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, - } - - 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(self, sm_mock): + sm_mock.return_value = False + self.run_method_with_sdk_servers('lock_server', 1) - 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_multi_servers(self, sm_mock): + sm_mock.return_value = False + self.run_method_with_sdk_servers('lock_server', 3) - 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(self, sm_mock): + sm_mock.return_value = True arglist = [ - server.id, - '--reason', "blah", + self.server.id, + '--reason', 'blah', ] verifylist = [ - ('reason', "blah"), - ('server', [server.id]) + ('server', [self.server.id]), + ('reason', 'blah'), ] 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) + 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') + 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, - '--reason', "blah", + self.server.id, server2.id, + '--reason', 'choo..choo', ] verifylist = [ - ('reason', "blah"), - ('server', [self.server.id]) + ('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.servers_mock.get.assert_called_with(self.server.id) - self.server.lock.assert_called_with(reason="blah") + 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) - def test_server_lock_multi_servers_with_reason(self): - server2 = compute_fakes.FakeServer.create_one_server( - methods=self.methods) + @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 = [ - self.server.id, server2.id, - '--reason', "choo..choo", + server.id, + '--reason', "blah", ] verifylist = [ - ('reason', "choo..choo"), - ('server', [self.server.id, server2.id]) + ('server', [server.id]), + ('reason', "blah"), ] 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) + 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 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): -- cgit v1.2.1