From da7572a5ff9f54dbab4f8328632c562a2816a4fb Mon Sep 17 00:00:00 2001
From: Matt Riedemann <mriedem.os@gmail.com>
Date: Fri, 8 Jun 2018 14:43:41 -0400
Subject: [PATCH] Fix server show for microversion 2.47

Compute API version 2.47 embeds the server's internal
flavor in the response. The original flavor id is not
preserved since it could have changed if the flavor
was deleted and re-created after the server was created,
which was the dreaded Horizon "Edit Flavor" issue.
So the flavor dict in the server response is a dict of
information about the flavor representing the server
"right now" excluding the id. The original flavor name
is shown though along with the ram/disk/vcpu etc
information.

The server list command has a similar issue which
will be fixed in a follow up change.

Change-Id: I1a92999758006d02567c542b6be8902a049899cc
Task: 13864
Story: 1751104
---
 openstackclient/compute/v2/server.py          | 26 +++++++++++++-----
 .../tests/unit/compute/v2/test_server.py      | 27 +++++++++++++++++++
 ...104-compute-api-2.47-4bfa21cfaa13f408.yaml |  6 +++++
 3 files changed, 52 insertions(+), 7 deletions(-)
 create mode 100644 releasenotes/notes/bug-1751104-compute-api-2.47-4bfa21cfaa13f408.yaml

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index c2d9cf3af2..a7b99306d2 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -148,12 +148,18 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True):
 
     # Convert the flavor blob to a name
     flavor_info = info.get('flavor', {})
-    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
+    # 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:
+        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:
+        info['flavor'] = utils.format_dict(flavor_info)
 
     if 'os-extended-volumes:volumes_attached' in info:
         info.update(
@@ -1257,6 +1263,10 @@ class ListServer(command.Lister):
                     s.flavor_name = flavor.name
                 s.flavor_id = s.flavor['id']
             else:
+                # TODO(mriedem): Fix this for microversion >= 2.47 where the
+                # flavor is embedded in the server response without the id.
+                # We likely need to drop the Flavor ID column in that case if
+                # --long is specified.
                 s.flavor_name = ''
                 s.flavor_id = ''
 
@@ -1994,7 +2004,9 @@ class ShelveServer(command.Command):
 
 
 class ShowServer(command.ShowOne):
-    _description = _("Show server details")
+    _description = _(
+        "Show server details. Specify ``--os-compute-api-version 2.47`` "
+        "or higher to see the embedded flavor information for the server.")
 
     def get_parser(self, prog_name):
         parser = super(ShowServer, self).get_parser(prog_name)
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 61c81132a0..a53c6c8193 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -3217,6 +3217,33 @@ class TestServerShow(TestServer):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, data)
 
+    def test_show_embedded_flavor(self):
+        # Tests using --os-compute-api-version >= 2.47 where the flavor
+        # details are embedded in the server response body excluding the id.
+        arglist = [
+            self.server.name,
+        ]
+        verifylist = [
+            ('diagnostics', False),
+            ('server', self.server.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.server.info['flavor'] = {
+            'ephemeral': 0,
+            'ram': 512,
+            'original_name': 'm1.tiny',
+            'vcpus': 1,
+            'extra_specs': {},
+            'swap': 0,
+            'disk': 1
+        }
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.assertEqual(self.columns, columns)
+        # Since the flavor details are in a dict we can't be sure of the
+        # ordering so just assert that one of the keys is in the output.
+        self.assertIn('original_name', data[2])
+
     def test_show_diagnostics(self):
         arglist = [
             '--diagnostics',
diff --git a/releasenotes/notes/bug-1751104-compute-api-2.47-4bfa21cfaa13f408.yaml b/releasenotes/notes/bug-1751104-compute-api-2.47-4bfa21cfaa13f408.yaml
new file mode 100644
index 0000000000..ef89c785ef
--- /dev/null
+++ b/releasenotes/notes/bug-1751104-compute-api-2.47-4bfa21cfaa13f408.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    The ``openstack server show`` command will now properly show the server's
+    flavor information when using ``--os-compute-api-version 2.47`` or higher.
+    See: https://storyboard.openstack.org/#!/story/1751104