diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 2e7bc694de..bab303ebd9 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -149,16 +149,13 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True): # 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. + # compatibility for existing users. column_map = { 'access_ipv4': 'accessIPv4', 'access_ipv6': 'accessIPv6', 'admin_password': 'adminPass', - 'admin_password': 'adminPass', - 'volumes': 'os-extended-volumes:volumes_attached', + 'attached_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', @@ -168,7 +165,6 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True): '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', @@ -179,21 +175,56 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True): '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', } + # Some columns returned by openstacksdk should not be shown because they're + # either irrelevant or duplicates + ignored_columns = { + # computed columns + 'interface_ip', + 'location', + 'private_v4', + 'private_v6', + 'public_v4', + 'public_v6', + # create-only columns + 'block_device_mapping', + 'image_id', + 'max_count', + 'min_count', + 'scheduler_hints', + # aliases + 'volumes', + # unnecessary + 'links', + } + # Some columns are only present in certain responses and should not be + # shown otherwise. + optional_columns = { + 'admin_password', # removed in 2.14 + 'fault', # only present in errored servers + 'flavor_id', # removed in 2.47 + 'networks', # only present in create responses + 'security_groups', # only present in create, detail responses + } - info.update( - { - column_map[column]: data - for column, data in info.items() - if column in column_map - } - ) + data = {} + for key, value in info.items(): + if key in ignored_columns: + continue + + if key in optional_columns: + if info[key] is None: + continue + + alias = column_map.get(key) + data[alias or key] = value + + info = data # Convert the image blob to a name image_info = info.get('image', {}) @@ -214,46 +245,57 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True): # Convert the flavor blob to a name flavor_info = info.get('flavor', {}) # Microversion 2.47 puts the embedded flavor into the server response - # body but omits the id, so if not present we just expose the flavor - # dict in the server output. - if 'id' in flavor_info: + # body. The presence of the 'original_name' attribute indicates this. + if flavor_info.get('original_name') is None: # microversion < 2.47 flavor_id = flavor_info.get('id', '') try: flavor = utils.find_resource(compute_client.flavors, flavor_id) info['flavor'] = "%s (%s)" % (flavor.name, flavor_id) except Exception: info['flavor'] = flavor_id - else: + else: # microversion >= 2.47 info['flavor'] = format_columns.DictColumn(flavor_info) - if 'os-extended-volumes:volumes_attached' in info: + # there's a lot of redundant information in BDMs - strip it + if 'volumes_attached' in info: info.update( { 'volumes_attached': format_columns.ListDictColumn( - info.pop('os-extended-volumes:volumes_attached') + [ + { + k: v + for k, v in volume.items() + if v is not None and k != 'location' + } + for volume in info.pop('volumes_attached') or [] + ] ) } ) + if 'security_groups' in info: info.update( { 'security_groups': format_columns.ListDictColumn( - info.pop('security_groups') + info.pop('security_groups'), ) } ) + if 'tags' in info: info.update({'tags': format_columns.ListColumn(info.pop('tags'))}) - # NOTE(dtroyer): novaclient splits these into separate entries... - # Format addresses in a useful way - info['addresses'] = ( - AddressesColumn(info['addresses']) - if 'addresses' in info - else format_columns.DictListColumn(info.get('networks')) - ) + # Map 'networks' to 'addresses', if present. Note that the 'networks' key + # is used for create responses, otherwise it's 'addresses'. We know it'll + # be set because this is one of our optional columns. + if 'networks' in info: + info['addresses'] = format_columns.DictListColumn( + info.pop('networks', {}), + ) + else: + info['addresses'] = AddressesColumn(info.get('addresses', {})) - # Map 'metadata' field to 'properties' + # Map 'metadata' field to 'properties' and format info['properties'] = format_columns.DictColumn(info.pop('metadata')) # Migrate tenant_id to project_id naming @@ -266,9 +308,6 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True): info['OS-EXT-STS:power_state'] ) - # Remove values that are long and not too useful - info.pop('links', None) - return info diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 5257dd45ac..ea944f9b33 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -1313,7 +1313,6 @@ class TestServerCreate(TestServer): 'id', 'image', 'name', - 'networks', 'properties', ) @@ -1327,7 +1326,6 @@ class TestServerCreate(TestServer): self.new_server.id, self.image.name + ' (' + self.new_server.image.get('id') + ')', self.new_server.name, - self.new_server.networks, format_columns.DictColumn(self.new_server.metadata), ) return datalist @@ -2310,7 +2308,7 @@ class TestServerCreate(TestServer): self.new_server.name, self.image, self.flavor, **kwargs ) self.assertEqual(self.columns, columns) - self.assertEqual(self.datalist(), data) + self.assertTupleEqual(self.datalist(), data) @mock.patch.object(common_utils, 'wait_for_status', return_value=False) def test_server_create_with_wait_fails(self, mock_wait_for_status): @@ -8209,7 +8207,7 @@ class TestServerShow(TestServer): 'image': {'id': self.image.id}, 'flavor': {'id': self.flavor.id}, 'tenant_id': 'tenant-id-xxx', - 'networks': {'public': ['10.20.30.40', '2001:db8::f']}, + 'addresses': {'public': ['10.20.30.40', '2001:db8::f']}, } self.compute_sdk_client.get_server_diagnostics.return_value = { 'test': 'test' @@ -8236,7 +8234,6 @@ class TestServerShow(TestServer): 'id', 'image', 'name', - 'networks', 'project_id', 'properties', ) @@ -8245,12 +8242,11 @@ class TestServerShow(TestServer): server.PowerStateColumn( getattr(self.server, 'OS-EXT-STS:power_state') ), - format_columns.DictListColumn(self.server.networks), self.flavor.name + " (" + self.flavor.id + ")", self.server.id, self.image.name + " (" + self.image.id + ")", self.server.name, - {'public': ['10.20.30.40', '2001:db8::f']}, + server.AddressesColumn({'public': ['10.20.30.40', '2001:db8::f']}), 'tenant-id-xxx', format_columns.DictColumn({}), ) @@ -9059,7 +9055,7 @@ class TestServerGeneral(TestServer): 'image': {u'id': _image.id}, 'flavor': {u'id': _flavor.id}, 'tenant_id': u'tenant-id-xxx', - 'networks': {u'public': [u'10.20.30.40', u'2001:db8::f']}, + 'addresses': {u'public': [u'10.20.30.40', u'2001:db8::f']}, 'links': u'http://xxx.yyy.com', 'properties': '', 'volumes_attached': [{"id": "6344fe9d-ef20-45b2-91a6"}], @@ -9079,7 +9075,7 @@ class TestServerGeneral(TestServer): ), 'properties': '', 'volumes_attached': [{"id": "6344fe9d-ef20-45b2-91a6"}], - 'addresses': format_columns.DictListColumn(_server.networks), + 'addresses': format_columns.DictListColumn(_server.addresses), 'project_id': 'tenant-id-xxx', } @@ -9089,8 +9085,6 @@ class TestServerGeneral(TestServer): self.image_client, _server, ) - # 'networks' is used to create _server. Remove it. - server_detail.pop('networks') # Check the results. self.assertCountEqual(info, server_detail)