From 70dbb01ea3ed900a41092d46ed5ae1370d5771af Mon Sep 17 00:00:00 2001 From: Daniel Wilson Date: Sat, 12 Nov 2022 11:08:03 -0500 Subject: [PATCH] Use the SDK for server show Use the SDK for the server show command. This change modifies a helper function that is used by server show as well as other commands that print information about an individual server. The helper still uses novaclient APIs when additional OpenStack requests are needed since some of its callers are still using the nova client. Depends-On: https://review.opendev.org/c/openstack/openstacksdk/+/864340 Change-Id: Ic253184ee5f911ec2052419d328260dc4664b273 --- openstackclient/compute/v2/server.py | 83 +++++++++++++++---- .../functional/compute/v2/test_server.py | 28 +++++++ .../tests/unit/compute/v2/test_server.py | 17 ++-- ...h-server-show-to-sdk-44a614aebf2c6da6.yaml | 7 ++ 4 files changed, 109 insertions(+), 26 deletions(-) create mode 100644 releasenotes/notes/switch-server-show-to-sdk-44a614aebf2c6da6.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 5b97814b48..0612871b26 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -77,6 +77,10 @@ class AddressesColumn(cliff_columns.FormattableColumn): except Exception: return 'N/A' + def machine_readable(self): + return {k: [i['addr'] for i in v if 'addr' in i] + for k, v in self._value.items()} + class HostColumn(cliff_columns.FormattableColumn): """Generate a formatted string of a hostname.""" @@ -133,14 +137,61 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True): the latest details of a server after creating it. :rtype: a dict of server details """ + # Note: Some callers of this routine pass a novaclient server, and others + # pass an SDK server. Column names may be different across those cases. info = server.to_dict() if refresh: server = utils.find_resource(compute_client.servers, info['id']) info.update(server.to_dict()) + # Some commands using this routine were originally implemented with the + # nova python wrappers, and were later migrated to use the SDK. Map the + # SDK's property names to the original property names to maintain backward + # compatibility for existing users. Data is duplicated under both the old + # and new name so users can consume the data by either name. + column_map = { + 'access_ipv4': 'accessIPv4', + 'access_ipv6': 'accessIPv6', + 'admin_password': 'adminPass', + 'admin_password': 'adminPass', + 'volumes': 'os-extended-volumes:volumes_attached', + 'availability_zone': 'OS-EXT-AZ:availability_zone', + 'block_device_mapping': 'block_device_mapping_v2', + 'compute_host': 'OS-EXT-SRV-ATTR:host', + 'created_at': 'created', + 'disk_config': 'OS-DCF:diskConfig', + 'flavor_id': 'flavorRef', + 'has_config_drive': 'config_drive', + 'host_id': 'hostId', + 'fault': 'fault', + 'hostname': 'OS-EXT-SRV-ATTR:hostname', + 'hypervisor_hostname': 'OS-EXT-SRV-ATTR:hypervisor_hostname', + 'image_id': 'imageRef', + 'instance_name': 'OS-EXT-SRV-ATTR:instance_name', + 'is_locked': 'locked', + 'kernel_id': 'OS-EXT-SRV-ATTR:kernel_id', + 'launch_index': 'OS-EXT-SRV-ATTR:launch_index', + 'launched_at': 'OS-SRV-USG:launched_at', + 'power_state': 'OS-EXT-STS:power_state', + 'project_id': 'tenant_id', + 'ramdisk_id': 'OS-EXT-SRV-ATTR:ramdisk_id', + 'reservation_id': 'OS-EXT-SRV-ATTR:reservation_id', + 'root_device_name': 'OS-EXT-SRV-ATTR:root_device_name', + 'scheduler_hints': 'OS-SCH-HNT:scheduler_hints', + 'task_state': 'OS-EXT-STS:task_state', + 'terminated_at': 'OS-SRV-USG:terminated_at', + 'updated_at': 'updated', + 'user_data': 'OS-EXT-SRV-ATTR:user_data', + 'vm_state': 'OS-EXT-STS:vm_state', + } + + info.update({ + column_map[column]: data for column, data in info.items() + if column in column_map}) + # Convert the image blob to a name image_info = info.get('image', {}) - if image_info: + if image_info and any(image_info.values()): image_id = image_info.get('id', '') try: image = image_client.get_image(image_id) @@ -188,7 +239,9 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True): # NOTE(dtroyer): novaclient splits these into separate entries... # Format addresses in a useful way - info['addresses'] = format_columns.DictListColumn(server.networks) + info['addresses'] = ( + AddressesColumn(info['addresses']) if 'addresses' in info + else format_columns.DictListColumn(info.get('networks'))) # Map 'metadata' field to 'properties' info['properties'] = format_columns.DictColumn(info.pop('metadata')) @@ -4319,32 +4372,34 @@ class ShowServer(command.ShowOne): 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 + + # Find by name or ID, then get the full details of the server + server = compute_client.find_server( + parsed_args.server, ignore_missing=False) + server = compute_client.get_server(server) if parsed_args.diagnostics: - (resp, data) = server.diagnostics() - if not resp.status_code == 200: - self.app.stderr.write(_( - "Error retrieving diagnostics data\n" - )) - return ({}, {}) + data = compute_client.get_server_diagnostics(server) return zip(*sorted(data.items())) topology = None if parsed_args.topology: - if compute_client.api_version < api_versions.APIVersion('2.78'): + if not sdk_utils.supports_microversion(compute_client, '2.78'): msg = _( '--os-compute-api-version 2.78 or greater is required to ' 'support the --topology option' ) raise exceptions.CommandError(msg) - topology = server.topology() + topology = server.fetch_topology(compute_client) data = _prep_server_detail( - compute_client, self.app.client_manager.image, server, + # TODO(dannosliwcd): Replace these clients with SDK clients after + # all callers of _prep_server_detail() are using the SDK. + self.app.client_manager.compute, + self.app.client_manager.image, + server, refresh=False) if topology: diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py index 830b3543b6..106918d5d9 100644 --- a/openstackclient/tests/functional/compute/v2/test_server.py +++ b/openstackclient/tests/functional/compute/v2/test_server.py @@ -11,6 +11,7 @@ # under the License. import itertools +import json import time import uuid @@ -288,6 +289,33 @@ class ServerTests(common.ComputeTestCase): ) self.assertOutput("", raw_output) + def test_server_show(self): + """Test server show""" + cmd_output = self.server_create() + name = cmd_output['name'] + + # Simple show + cmd_output = json.loads(self.openstack( + f'server show -f json {name}' + )) + self.assertEqual( + name, + cmd_output["name"], + ) + + # Show diagnostics + cmd_output = json.loads(self.openstack( + f'server show -f json {name} --diagnostics' + )) + self.assertIn('driver', cmd_output) + + # Show topology + cmd_output = json.loads(self.openstack( + f'server show -f json {name} --topology ' + f'--os-compute-api-version 2.78' + )) + self.assertIn('topology', cmd_output) + def test_server_actions(self): """Test server action pairs diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 45aff6a370..ba67c706e1 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -7885,20 +7885,15 @@ class TestServerShow(TestServer): 'tenant_id': 'tenant-id-xxx', 'networks': {'public': ['10.20.30.40', '2001:db8::f']}, } - # Fake the server.diagnostics() method. The return value contains http - # response and data. The data is a dict. Sincce this method itself is - # faked, we don't need to fake everything of the return value exactly. - resp = mock.Mock() - resp.status_code = 200 + self.sdk_client.get_server_diagnostics.return_value = {'test': 'test'} server_method = { - 'diagnostics': (resp, {'test': 'test'}), - 'topology': self.topology, + 'fetch_topology': self.topology, } self.server = compute_fakes.FakeServer.create_one_server( attrs=server_info, methods=server_method) # This is the return value for utils.find_resource() - self.servers_mock.get.return_value = self.server + self.sdk_client.get_server.return_value = self.server self.get_image_mock.return_value = self.image self.flavors_mock.get.return_value = self.flavor @@ -7999,8 +7994,7 @@ class TestServerShow(TestServer): self.assertEqual(('test',), data) def test_show_topology(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.78') + self._set_mock_microversion('2.78') arglist = [ '--topology', @@ -8022,8 +8016,7 @@ class TestServerShow(TestServer): self.assertCountEqual(self.data, data) def test_show_topology_pre_v278(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.77') + self._set_mock_microversion('2.77') arglist = [ '--topology', diff --git a/releasenotes/notes/switch-server-show-to-sdk-44a614aebf2c6da6.yaml b/releasenotes/notes/switch-server-show-to-sdk-44a614aebf2c6da6.yaml new file mode 100644 index 0000000000..c116f6e0cb --- /dev/null +++ b/releasenotes/notes/switch-server-show-to-sdk-44a614aebf2c6da6.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + The ``server show`` command now uses the OpenStack SDK instead of the + Python nova bindings. The command prints data fields both by their + novaclient names used in previous releases as well as the names used in the + SDK.