From 9874fa81d7227b9ab41a5448bca55d1ac265f3e7 Mon Sep 17 00:00:00 2001
From: Monty Taylor <mordred@inaugust.com>
Date: Tue, 1 Oct 2019 18:22:57 +0200
Subject: [PATCH] Fix server for later microversion

We updated the server record for 2.72 but missed some bits of inventory
because we weren't actually gettting 2.72. Whoops.

We need to clean the policy/policies fields in ServerGroup because
otherwise they're considered dirty and None values are sent.

Change-Id: Id1720774bc0f6f398d0739a466c7bd5c54182642
---
 openstack/cloud/meta.py                           | 15 +++++++++++----
 openstack/compute/v2/server_group.py              |  4 ++--
 .../tests/functional/cloud/test_inventory.py      | 10 ++++++++--
 .../fix-for-microversion-70cd686b6d6e3fd0.yaml    | 14 ++++++++++++++
 4 files changed, 35 insertions(+), 8 deletions(-)
 create mode 100644 releasenotes/notes/fix-for-microversion-70cd686b6d6e3fd0.yaml

diff --git a/openstack/cloud/meta.py b/openstack/cloud/meta.py
index 0ff6fb1dc..a651d0365 100644
--- a/openstack/cloud/meta.py
+++ b/openstack/cloud/meta.py
@@ -484,10 +484,17 @@ def get_hostvars_from_server(cloud, server, mounts=None):
     """
     server_vars = add_server_interfaces(cloud, server)
 
-    flavor_id = server['flavor']['id']
-    flavor_name = cloud.get_flavor_name(flavor_id)
-    if flavor_name:
-        server_vars['flavor']['name'] = flavor_name
+    flavor_id = server['flavor'].get('id')
+    if flavor_id:
+        # In newer nova, the flavor record can be kept around for flavors
+        # that no longer exist. The id and name are not there.
+        flavor_name = cloud.get_flavor_name(flavor_id)
+        if flavor_name:
+            server_vars['flavor']['name'] = flavor_name
+    elif 'original_name' in server['flavor']:
+        # Users might be have code still expecting name. That name is in
+        # original_name.
+        server_vars['flavor']['name'] = server['flavor']['original_name']
 
     expand_server_security_groups(cloud, server)
 
diff --git a/openstack/compute/v2/server_group.py b/openstack/compute/v2/server_group.py
index 78a577e9a..fe463504e 100644
--- a/openstack/compute/v2/server_group.py
+++ b/openstack/compute/v2/server_group.py
@@ -70,7 +70,7 @@ class ServerGroup(resource.Resource):
                 if self.policies:
                     if not self.policy and isinstance(self.policies, list):
                         self.policy = self.policies[0]
-                    self.policies = None
+                    self._body.clean(only={'policies'})
                 microversion = self._max_microversion
             else:
                 if self.rules:
@@ -80,6 +80,6 @@ class ServerGroup(resource.Resource):
                 if self.policy:
                     if not self.policies:
                         self.policies = [self.policy]
-                    self.policy = None
+                    self._body.clean(only={'policy'})
 
         return microversion
diff --git a/openstack/tests/functional/cloud/test_inventory.py b/openstack/tests/functional/cloud/test_inventory.py
index bfe8b11f2..59e12aa59 100644
--- a/openstack/tests/functional/cloud/test_inventory.py
+++ b/openstack/tests/functional/cloud/test_inventory.py
@@ -49,12 +49,15 @@ class TestInventory(base.BaseFunctionalTest):
     def _test_host_content(self, host):
         self.assertEqual(host['image']['id'], self.image.id)
         self.assertNotIn('links', host['image'])
-        self.assertEqual(host['flavor']['id'], self.flavor.id)
+        # TODO(mordred) Add this back wnen ksa releases
+        # self.assertNotIn('id', host['flavor'])
         self.assertNotIn('links', host['flavor'])
         self.assertNotIn('links', host)
         self.assertIsInstance(host['volumes'], list)
         self.assertIsInstance(host['metadata'], dict)
         self.assertIn('interface_ip', host)
+        # TODO(mordred) Add this back wnen ksa releases
+        # self.assertIn('ram', host['flavor'])
 
     def _test_expanded_host_content(self, host):
         self.assertEqual(host['image']['name'], self.image.name)
@@ -81,9 +84,12 @@ class TestInventory(base.BaseFunctionalTest):
         self.assertEqual(host['image']['id'], self.image.id)
         self.assertNotIn('links', host['image'])
         self.assertNotIn('name', host['name'])
-        self.assertEqual(host['flavor']['id'], self.flavor.id)
+        # TODO(mordred) Add this back wnen ksa releases
+        # self.assertNotIn('id', host['flavor'])
         self.assertNotIn('links', host['flavor'])
         self.assertNotIn('name', host['flavor'])
+        # TODO(mordred) Add this back wnen ksa releases
+        # self.assertIn('ram', host['flavor'])
 
         host_found = False
         for host in self.inventory.list_hosts(expand=False):
diff --git a/releasenotes/notes/fix-for-microversion-70cd686b6d6e3fd0.yaml b/releasenotes/notes/fix-for-microversion-70cd686b6d6e3fd0.yaml
new file mode 100644
index 000000000..be402ef98
--- /dev/null
+++ b/releasenotes/notes/fix-for-microversion-70cd686b6d6e3fd0.yaml
@@ -0,0 +1,14 @@
+---
+fixes:
+  - |
+    In April 2019 the microversion support for the Server resource was increased
+    to ``2.72``. Unfortunately, due to an issue with version discovery documents,
+    this increase never actually became effective. A fix is coming in ``3.17.2`` of
+    ``keystoneauth`` which will unbreak version discovery and cause the microversion
+    support to start working.
+upgrade:
+  - |
+    Due to the fix in microversion support in `keystoneauth`, Servers will be
+    fetched using microversion ``2.72``. Code that assumes the existence of a
+    ``flavor.id`` field in the Server record should be removed, as it does not exist
+    in new microversions and cannot be filled in behind the scenes.