From bcaf2ab559e82d571cf46ed50aba60abf0b41637 Mon Sep 17 00:00:00 2001 From: Stephen Finucane <stephenfin@redhat.com> Date: Wed, 8 May 2024 18:36:53 +0100 Subject: [PATCH] compute: Migrate 'server set', 'server unset' commands Change-Id: I2c249e9ca3952100dcf7f97fcafa879b733d34c6 Signed-off-by: Stephen Finucane <stephenfin@redhat.com> --- openstackclient/compute/v2/server.py | 95 +++--- .../tests/unit/compute/v2/test_server.py | 287 +++++++++++------- ...ver-set-unset-to-sdk-ae32ebcced845b06.yaml | 4 + requirements.txt | 2 +- 4 files changed, 247 insertions(+), 141 deletions(-) create mode 100644 releasenotes/notes/migrate-server-set-unset-to-sdk-ae32ebcced845b06.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index dc85e21f74..026feaccd4 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -4415,14 +4415,13 @@ class SetServer(command.Command): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute - server = utils.find_resource( - compute_client.servers, - parsed_args.server, + compute_client = self.app.client_manager.sdk_connection.compute + server = compute_client.find_server( + parsed_args.server, ignore_missing=False ) if parsed_args.description: - if compute_client.api_version < api_versions.APIVersion("2.19"): + if not sdk_utils.supports_microversion(compute_client, '2.19'): msg = _( '--os-compute-api-version 2.19 or greater is required to ' 'support the --description option' @@ -4430,7 +4429,7 @@ class SetServer(command.Command): raise exceptions.CommandError(msg) if parsed_args.tags: - if compute_client.api_version < api_versions.APIVersion('2.26'): + if not sdk_utils.supports_microversion(compute_client, '2.26'): msg = _( '--os-compute-api-version 2.26 or greater is required to ' 'support the --tag option' @@ -4438,7 +4437,7 @@ class SetServer(command.Command): raise exceptions.CommandError(msg) if parsed_args.hostname: - if compute_client.api_version < api_versions.APIVersion('2.90'): + if not sdk_utils.supports_microversion(compute_client, '2.90'): msg = _( '--os-compute-api-version 2.90 or greater is required to ' 'support the --hostname option' @@ -4457,30 +4456,32 @@ class SetServer(command.Command): update_kwargs['hostname'] = parsed_args.hostname if update_kwargs: - server.update(**update_kwargs) + compute_client.update_server(server, **update_kwargs) if parsed_args.properties: - compute_client.servers.set_meta(server, parsed_args.properties) + compute_client.set_server_metadata( + server, **parsed_args.properties + ) if parsed_args.state: - server.reset_state(state=parsed_args.state) + compute_client.reset_server_state(server, state=parsed_args.state) if parsed_args.root_password: p1 = getpass.getpass(_('New password: ')) p2 = getpass.getpass(_('Retype new password: ')) if p1 == p2: - server.change_password(p1) + compute_client.change_server_password(server, p1) else: msg = _("Passwords do not match, password unchanged") raise exceptions.CommandError(msg) elif parsed_args.password: - server.change_password(parsed_args.password) + compute_client.change_server_password(server, parsed_args.password) elif parsed_args.no_password: - server.clear_password() + compute_client.clear_server_password(server) if parsed_args.tags: for tag in parsed_args.tags: - server.add_tag(tag=tag) + compute_client.add_tag_to_server(server, tag=tag) class ShelveServer(command.Command): @@ -4995,7 +4996,8 @@ class UnsetServer(command.Command): metavar='<server>', help=_('Server (name or ID)'), ) - parser.add_argument( + property_group = parser.add_mutually_exclusive_group() + property_group.add_argument( '--property', metavar='<key>', action='append', @@ -5006,16 +5008,22 @@ class UnsetServer(command.Command): '(repeat option to remove multiple values)' ), ) + property_group.add_argument( + '--all-properties', + action='store_true', + help=_('Remove all properties'), + ) parser.add_argument( '--description', dest='description', action='store_true', help=_( - 'Unset server description (supported by ' - '--os-compute-api-version 2.19 or above)' + 'Unset server description ' + '(supported by --os-compute-api-version 2.19 or above)' ), ) - parser.add_argument( + tag_group = parser.add_mutually_exclusive_group() + tag_group.add_argument( '--tag', metavar='<tag>', action='append', @@ -5027,32 +5035,40 @@ class UnsetServer(command.Command): '(supported by --os-compute-api-version 2.26 or above)' ), ) + tag_group.add_argument( + '--all-tags', + action='store_true', + help=_( + 'Remove all tags ' + '(supported by --os-compute-api-version 2.26 or above)' + ), + ) return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute - server = utils.find_resource( - compute_client.servers, - parsed_args.server, + compute_client = self.app.client_manager.sdk_connection.compute + + server = compute_client.find_server( + parsed_args.server, ignore_missing=False ) - if parsed_args.properties: - compute_client.servers.delete_meta(server, parsed_args.properties) - - if parsed_args.description: - if compute_client.api_version < api_versions.APIVersion("2.19"): - msg = _( - '--os-compute-api-version 2.19 or greater is ' - 'required to support the --description option' - ) - raise exceptions.CommandError(msg) - compute_client.servers.update( - server, - description="", + if parsed_args.properties or parsed_args.all_properties: + compute_client.delete_server_metadata( + server, parsed_args.properties or None ) - if parsed_args.tags: - if compute_client.api_version < api_versions.APIVersion('2.26'): + if parsed_args.description: + if not sdk_utils.supports_microversion(compute_client, '2.19'): + msg = _( + '--os-compute-api-version 2.19 or greater is required to ' + 'support the --description option' + ) + raise exceptions.CommandError(msg) + + compute_client.update_server(server, description="") + + if parsed_args.tags or parsed_args.all_tags: + if not sdk_utils.supports_microversion(compute_client, '2.26'): msg = _( '--os-compute-api-version 2.26 or greater is required to ' 'support the --tag option' @@ -5060,7 +5076,10 @@ class UnsetServer(command.Command): raise exceptions.CommandError(msg) for tag in parsed_args.tags: - compute_client.servers.delete_tag(server, tag=tag) + compute_client.remove_tag_from_server(server, tag) + + if parsed_args.all_tags: + compute_client.remove_tags_from_server(server) class UnshelveServer(command.Command): diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index f460c0513b..1384dfe8ad 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -7843,62 +7843,60 @@ class TestServerSet(TestServer): def setUp(self): super().setUp() - self.attrs = { - 'api_version': None, - } - - self.methods = { - 'update': None, - 'reset_state': None, - 'change_password': None, - 'clear_password': None, - 'add_tag': None, - 'set_tags': None, - } - - self.fake_servers = self.setup_servers_mock(2) + self.server = compute_fakes.create_one_sdk_server() + self.compute_sdk_client.find_server.return_value = self.server # Get the command object to test self.cmd = server.SetServer(self.app, None) def test_server_set_no_option(self): - arglist = ['foo_vm'] - verifylist = [('server', 'foo_vm')] + arglist = [self.server.id] + verifylist = [('server', self.server.id)] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.assertNotCalled(self.fake_servers[0].update) - self.assertNotCalled(self.fake_servers[0].reset_state) - self.assertNotCalled(self.fake_servers[0].change_password) - self.assertNotCalled(self.servers_mock.set_meta) + + self.compute_sdk_client.update_server.assert_not_called() + self.compute_sdk_client.set_server_metadata.assert_not_called() + self.compute_sdk_client.reset_server_state.assert_not_called() + self.compute_sdk_client.change_server_password.assert_not_called() + self.compute_sdk_client.clear_server_password.assert_not_called() + self.compute_sdk_client.add_tag_to_server.assert_not_called() self.assertIsNone(result) def test_server_set_with_state(self): - for index, state in enumerate(['active', 'error']): - arglist = [ - '--state', - state, - 'foo_vm', - ] - verifylist = [ - ('state', state), - ('server', 'foo_vm'), - ] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) - self.fake_servers[index].reset_state.assert_called_once_with( - state=state - ) - self.assertIsNone(result) + arglist = [ + '--state', + 'active', + self.server.id, + ] + verifylist = [ + ('state', 'active'), + ('server', self.server.id), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) + + self.compute_sdk_client.reset_server_state.assert_called_once_with( + self.server, state='active' + ) + self.compute_sdk_client.update_server.assert_not_called() + self.compute_sdk_client.set_server_metadata.assert_not_called() + self.compute_sdk_client.change_server_password.assert_not_called() + self.compute_sdk_client.clear_server_password.assert_not_called() + self.compute_sdk_client.add_tag_to_server.assert_not_called() + self.assertIsNone(result) def test_server_set_with_invalid_state(self): arglist = [ '--state', 'foo_state', - 'foo_vm', + self.server.id, ] verifylist = [ ('state', 'foo_state'), - ('server', 'foo_vm'), + ('server', self.server.id), ] self.assertRaises( test_utils.ParserException, @@ -7912,15 +7910,24 @@ class TestServerSet(TestServer): arglist = [ '--name', 'foo_name', - 'foo_vm', + self.server.id, ] verifylist = [ ('name', 'foo_name'), - ('server', 'foo_vm'), + ('server', self.server.id), ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.fake_servers[0].update.assert_called_once_with(name='foo_name') + + self.compute_sdk_client.update_server.assert_called_once_with( + self.server, name='foo_name' + ) + self.compute_sdk_client.set_server_metadata.assert_not_called() + self.compute_sdk_client.reset_server_state.assert_not_called() + self.compute_sdk_client.change_server_password.assert_not_called() + self.compute_sdk_client.clear_server_password.assert_not_called() + self.compute_sdk_client.add_tag_to_server.assert_not_called() self.assertIsNone(result) def test_server_set_with_property(self): @@ -7929,49 +7936,72 @@ class TestServerSet(TestServer): 'key1=value1', '--property', 'key2=value2', - 'foo_vm', + self.server.id, ] verifylist = [ ('properties', {'key1': 'value1', 'key2': 'value2'}), - ('server', 'foo_vm'), + ('server', self.server.id), ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.servers_mock.set_meta.assert_called_once_with( - self.fake_servers[0], parsed_args.properties + + self.compute_sdk_client.set_server_metadata.assert_called_once_with( + self.server, key1='value1', key2='value2' ) + self.compute_sdk_client.update_server.assert_not_called() + self.compute_sdk_client.reset_server_state.assert_not_called() + self.compute_sdk_client.change_server_password.assert_not_called() + self.compute_sdk_client.clear_server_password.assert_not_called() + self.compute_sdk_client.add_tag_to_server.assert_not_called() self.assertIsNone(result) def test_server_set_with_password(self): arglist = [ '--password', 'foo', - 'foo_vm', + self.server.id, ] verifylist = [ ('password', 'foo'), - ('server', 'foo_vm'), + ('server', self.server.id), ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) - self.cmd.take_action(parsed_args) - - self.fake_servers[0].change_password.assert_called_once_with('foo') + self.compute_sdk_client.change_server_password.assert_called_once_with( + self.server, 'foo' + ) + self.compute_sdk_client.update_server.assert_not_called() + self.compute_sdk_client.set_server_metadata.assert_not_called() + self.compute_sdk_client.reset_server_state.assert_not_called() + self.compute_sdk_client.clear_server_password.assert_not_called() + self.compute_sdk_client.add_tag_to_server.assert_not_called() + self.assertIsNone(result) def test_server_set_with_no_password(self): arglist = [ '--no-password', - 'foo_vm', + self.server.id, ] verifylist = [ ('no_password', True), - ('server', 'foo_vm'), + ('server', self.server.id), ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) - self.cmd.take_action(parsed_args) - - self.fake_servers[0].clear_password.assert_called_once_with() + self.compute_sdk_client.clear_server_password.assert_called_once_with( + self.server + ) + self.compute_sdk_client.update_server.assert_not_called() + self.compute_sdk_client.set_server_metadata.assert_not_called() + self.compute_sdk_client.reset_server_state.assert_not_called() + self.compute_sdk_client.change_server_password.assert_not_called() + self.compute_sdk_client.add_tag_to_server.assert_not_called() + self.assertIsNone(result) # TODO(stephenfin): Remove this in a future major version @mock.patch.object( @@ -7980,17 +8010,24 @@ class TestServerSet(TestServer): def test_server_set_with_root_password(self, mock_getpass): arglist = [ '--root-password', - 'foo_vm', + self.server.id, ] verifylist = [ ('root_password', True), - ('server', 'foo_vm'), + ('server', self.server.id), ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.fake_servers[0].change_password.assert_called_once_with( - mock.sentinel.fake_pass + + self.compute_sdk_client.change_server_password.assert_called_once_with( + self.server, mock.sentinel.fake_pass ) + self.compute_sdk_client.update_server.assert_not_called() + self.compute_sdk_client.set_server_metadata.assert_not_called() + self.compute_sdk_client.reset_server_state.assert_not_called() + self.compute_sdk_client.clear_server_password.assert_not_called() + self.compute_sdk_client.add_tag_to_server.assert_not_called() self.assertIsNone(result) def test_server_set_with_description(self): @@ -7999,17 +8036,24 @@ class TestServerSet(TestServer): arglist = [ '--description', 'foo_description', - 'foo_vm', + self.server.id, ] verifylist = [ ('description', 'foo_description'), - ('server', 'foo_vm'), + ('server', self.server.id), ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.fake_servers[0].update.assert_called_once_with( - description='foo_description' + + self.compute_sdk_client.update_server.assert_called_once_with( + self.server, description='foo_description' ) + self.compute_sdk_client.set_server_metadata.assert_not_called() + self.compute_sdk_client.reset_server_state.assert_not_called() + self.compute_sdk_client.change_server_password.assert_not_called() + self.compute_sdk_client.clear_server_password.assert_not_called() + self.compute_sdk_client.add_tag_to_server.assert_not_called() self.assertIsNone(result) def test_server_set_with_description_pre_v219(self): @@ -8018,12 +8062,13 @@ class TestServerSet(TestServer): arglist = [ '--description', 'foo_description', - 'foo_vm', + self.server.id, ] verifylist = [ ('description', 'foo_description'), - ('server', 'foo_vm'), + ('server', self.server.id), ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.assertRaises( exceptions.CommandError, self.cmd.take_action, parsed_args @@ -8037,22 +8082,27 @@ class TestServerSet(TestServer): 'tag1', '--tag', 'tag2', - 'foo_vm', + self.server.id, ] verifylist = [ ('tags', ['tag1', 'tag2']), - ('server', 'foo_vm'), + ('server', self.server.id), ] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) + parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.fake_servers[0].add_tag.assert_has_calls( + self.compute_sdk_client.add_tag_to_server.assert_has_calls( [ - mock.call(tag='tag1'), - mock.call(tag='tag2'), + mock.call(self.server, tag='tag1'), + mock.call(self.server, tag='tag2'), ] ) + self.compute_sdk_client.update_server.assert_not_called() + self.compute_sdk_client.set_server_metadata.assert_not_called() + self.compute_sdk_client.reset_server_state.assert_not_called() + self.compute_sdk_client.change_server_password.assert_not_called() + self.compute_sdk_client.clear_server_password.assert_not_called() self.assertIsNone(result) def test_server_set_with_tag_pre_v226(self): @@ -8063,14 +8113,14 @@ class TestServerSet(TestServer): 'tag1', '--tag', 'tag2', - 'foo_vm', + self.server.id, ] verifylist = [ ('tags', ['tag1', 'tag2']), - ('server', 'foo_vm'), + ('server', self.server.id), ] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) + parsed_args = self.check_parser(self.cmd, arglist, verifylist) ex = self.assertRaises( exceptions.CommandError, self.cmd.take_action, parsed_args ) @@ -8084,17 +8134,24 @@ class TestServerSet(TestServer): arglist = [ '--hostname', 'foo-hostname', - 'foo_vm', + self.server.id, ] verifylist = [ ('hostname', 'foo-hostname'), - ('server', 'foo_vm'), + ('server', self.server.id), ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.fake_servers[0].update.assert_called_once_with( - hostname='foo-hostname' + + self.compute_sdk_client.update_server.assert_called_once_with( + self.server, hostname='foo-hostname' ) + self.compute_sdk_client.set_server_metadata.assert_not_called() + self.compute_sdk_client.reset_server_state.assert_not_called() + self.compute_sdk_client.change_server_password.assert_not_called() + self.compute_sdk_client.clear_server_password.assert_not_called() + self.compute_sdk_client.add_tag_to_server.assert_not_called() self.assertIsNone(result) def test_server_set_with_hostname_pre_v290(self): @@ -8103,11 +8160,11 @@ class TestServerSet(TestServer): arglist = [ '--hostname', 'foo-hostname', - 'foo_vm', + self.server.id, ] verifylist = [ ('hostname', 'foo-hostname'), - ('server', 'foo_vm'), + ('server', self.server.id), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.assertRaises( @@ -8718,21 +8775,29 @@ class TestServerUnset(TestServer): def setUp(self): super().setUp() - self.fake_server = self.setup_servers_mock(1)[0] + self.server = compute_fakes.create_one_sdk_server() + self.compute_sdk_client.find_server.return_value = self.server # Get the command object to test self.cmd = server.UnsetServer(self.app, None) def test_server_unset_no_option(self): arglist = [ - 'foo_vm', + self.server.id, ] verifylist = [ - ('server', 'foo_vm'), + ('server', self.server.id), ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.assertNotCalled(self.servers_mock.delete_meta) + + self.compute_sdk_client.find_server( + self.server.id, ignore_missing=False + ) + self.compute_sdk_client.delete_server_metadata.assert_not_called() + self.compute_sdk_client.update_server.assert_not_called() + self.compute_sdk_client.remove_tag_from_server.assert_not_called() self.assertIsNone(result) def test_server_unset_with_property(self): @@ -8741,17 +8806,25 @@ class TestServerUnset(TestServer): 'key1', '--property', 'key2', - 'foo_vm', + self.server.id, ] verifylist = [ ('properties', ['key1', 'key2']), - ('server', 'foo_vm'), + ('server', self.server.id), ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.servers_mock.delete_meta.assert_called_once_with( - self.fake_server, ['key1', 'key2'] + + self.compute_sdk_client.find_server( + self.server.id, ignore_missing=False ) + self.compute_sdk_client.delete_server_metadata.assert_called_once_with( + self.server, + ['key1', 'key2'], + ) + self.compute_sdk_client.update_server.assert_not_called() + self.compute_sdk_client.remove_tag_from_server.assert_not_called() self.assertIsNone(result) def test_server_unset_with_description(self): @@ -8760,19 +8833,24 @@ class TestServerUnset(TestServer): arglist = [ '--description', - 'foo_vm', + self.server.id, ] verifylist = [ ('description', True), - ('server', 'foo_vm'), + ('server', self.server.id), ] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) + parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.servers_mock.update.assert_called_once_with( - self.fake_server, description="" + self.compute_sdk_client.find_server( + self.server.id, ignore_missing=False ) + self.compute_sdk_client.update_server.assert_called_once_with( + self.server, description='' + ) + self.compute_sdk_client.delete_server_metadata.assert_not_called() + self.compute_sdk_client.remove_tag_from_server.assert_not_called() self.assertIsNone(result) def test_server_unset_with_description_pre_v219(self): @@ -8781,11 +8859,11 @@ class TestServerUnset(TestServer): arglist = [ '--description', - 'foo_vm', + self.server.id, ] verifylist = [ ('description', True), - ('server', 'foo_vm'), + ('server', self.server.id), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -8804,23 +8882,28 @@ class TestServerUnset(TestServer): 'tag1', '--tag', 'tag2', - 'foo_vm', + self.server.id, ] verifylist = [ ('tags', ['tag1', 'tag2']), - ('server', 'foo_vm'), + ('server', self.server.id), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) self.assertIsNone(result) - self.servers_mock.delete_tag.assert_has_calls( + self.compute_sdk_client.find_server( + self.server.id, ignore_missing=False + ) + self.compute_sdk_client.remove_tag_from_server.assert_has_calls( [ - mock.call(self.fake_server, tag='tag1'), - mock.call(self.fake_server, tag='tag2'), + mock.call(self.server, 'tag1'), + mock.call(self.server, 'tag2'), ] ) + self.compute_sdk_client.delete_server_metadata.assert_not_called() + self.compute_sdk_client.update_server.assert_not_called() def test_server_unset_with_tag_pre_v226(self): self.set_compute_api_version('2.25') @@ -8830,11 +8913,11 @@ class TestServerUnset(TestServer): 'tag1', '--tag', 'tag2', - 'foo_vm', + self.server.id, ] verifylist = [ ('tags', ['tag1', 'tag2']), - ('server', 'foo_vm'), + ('server', self.server.id), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) diff --git a/releasenotes/notes/migrate-server-set-unset-to-sdk-ae32ebcced845b06.yaml b/releasenotes/notes/migrate-server-set-unset-to-sdk-ae32ebcced845b06.yaml new file mode 100644 index 0000000000..d01c09a909 --- /dev/null +++ b/releasenotes/notes/migrate-server-set-unset-to-sdk-ae32ebcced845b06.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + The ``server set`` and ``server unset`` commands have been migrated to SDK. diff --git a/requirements.txt b/requirements.txt index 0cc6c7e880..a20ce69f87 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,7 @@ pbr!=2.1.0,>=2.0.0 # Apache-2.0 cryptography>=2.7 # BSD/Apache-2.0 cliff>=3.5.0 # Apache-2.0 iso8601>=0.1.11 # MIT -openstacksdk>=2.0.0 # Apache-2.0 +openstacksdk>=3.2.0 # Apache-2.0 osc-lib>=2.3.0 # Apache-2.0 oslo.i18n>=3.15.3 # Apache-2.0 python-keystoneclient>=3.22.0 # Apache-2.0