Merge "Use the SDK for server show"

4 files changed, 109 insertions, 26 deletions

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index bf345e65..480759af 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'))
@@ -4327,32 +4380,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 6a6afa7c..b1afe875 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 846ad8cd..fc8dcd4f 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -7931,20 +7931,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
 
@@ -8045,8 +8040,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',
@@ -8068,8 +8062,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 00000000..c116f6e0
--- /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.