From 71e26d17283bfce7deebb36e0980b0d92f285abf Mon Sep 17 00:00:00 2001
From: Mark Goddard <mark@stackhpc.com>
Date: Thu, 26 Nov 2020 15:34:01 +0000
Subject: [PATCH] Fix missing node facts with BIFROST_INVENTORY_SOURCE=ironic

In Ussuri and earlier releases, the Bifrost custom inventory returns all
node fields as facts. Currently, most of these are returned as null.
This happened following the switch to openstacksdk. The plugin checks
for the presence of a properties field, and gets detailed node
information if it is missing. This worked with shade, but with
openstacksdk, the returned node object has most fields present but set
to null.

One of the most obvious consequences is that provisioning fails, since
the 'Deploy to hardware - Using custom instance_info.' task tries to use
the instance_info value of null.

This change fixes the issue by removing the check and always getting the
detailed node information.

Change-Id: Ia87c8332994f3b0f037ada953a299987bba246e5
Story: 2008394
Task: 41321
---
 bifrost/inventory.py                          |  3 +-
 bifrost/tests/unit/test_inventory.py          | 40 +++++++++++++------
 .../fix-story-2008394-9a77486a838a1f2c.yaml   |  7 ++++
 3 files changed, 36 insertions(+), 14 deletions(-)
 create mode 100644 releasenotes/notes/fix-story-2008394-9a77486a838a1f2c.yaml

diff --git a/bifrost/inventory.py b/bifrost/inventory.py
index 1b785e28f..fede26bf2 100755
--- a/bifrost/inventory.py
+++ b/bifrost/inventory.py
@@ -231,8 +231,7 @@ def _process_sdk(groups, hostvars):
         node_names = node_names.split(',')
 
     for machine in machines:
-        if 'properties' not in machine:
-            machine = cloud.get_machine(machine['uuid'])
+        machine = cloud.get_machine(machine['uuid'])
         if machine['name'] is None:
             name = machine['uuid']
         else:
diff --git a/bifrost/tests/unit/test_inventory.py b/bifrost/tests/unit/test_inventory.py
index dfefd2615..54470a93e 100644
--- a/bifrost/tests/unit/test_inventory.py
+++ b/bifrost/tests/unit/test_inventory.py
@@ -44,18 +44,26 @@ class TestBifrostInventoryUnit(base.TestCase):
         mock_cloud = mock_sdk.return_value
         mock_cloud.list_machines.return_value = [
             {
-                'driver_info': {
-                    'ipmi_address': '1.2.3.4',
-                },
+                'driver_info': None,
                 'links': [],
                 'name': 'node1',
                 'ports': [],
-                'properties': {
-                    'cpus': 42,
-                },
+                'properties': None,
                 'uuid': 'f3fbf7c6-b4e9-4dd2-8ca0-c74a50f8be45',
             },
         ]
+        mock_cloud.get_machine.return_value = {
+            'driver_info': {
+                'ipmi_address': '1.2.3.4',
+            },
+            'links': [],
+            'name': 'node1',
+            'ports': [],
+            'properties': {
+                'cpus': 42,
+            },
+            'uuid': 'f3fbf7c6-b4e9-4dd2-8ca0-c74a50f8be45',
+        }
         mock_cloud.list_nics_for_machine.return_value = [
             {
                 'address': '00:11:22:33:44:55',
@@ -94,18 +102,26 @@ class TestBifrostInventoryUnit(base.TestCase):
         mock_cloud = mock_sdk.return_value
         mock_cloud.list_machines.return_value = [
             {
-                'driver_info': {
-                    'ipmi_address': '1.2.3.4',
-                },
+                'driver_info': None,
                 'links': [],
                 'name': 'node1',
                 'ports': [],
-                'properties': {
-                    'cpus': 42,
-                },
+                'properties': None,
                 'uuid': 'f3fbf7c6-b4e9-4dd2-8ca0-c74a50f8be45',
             },
         ]
+        mock_cloud.get_machine.return_value = {
+            'driver_info': {
+                'ipmi_address': '1.2.3.4',
+            },
+            'links': [],
+            'name': 'node1',
+            'ports': [],
+            'properties': {
+                'cpus': 42,
+            },
+            'uuid': 'f3fbf7c6-b4e9-4dd2-8ca0-c74a50f8be45',
+        }
         mock_cloud.list_nics_for_machine.return_value = [
             {
                 'address': '00:11:22:33:44:55',
diff --git a/releasenotes/notes/fix-story-2008394-9a77486a838a1f2c.yaml b/releasenotes/notes/fix-story-2008394-9a77486a838a1f2c.yaml
new file mode 100644
index 000000000..cfc4487a7
--- /dev/null
+++ b/releasenotes/notes/fix-story-2008394-9a77486a838a1f2c.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes an issue with the Bifrost inventory plugin when used with
+    ``BIFROST_INVENTORY_SOURCE=ironic``. All node fields are now returned as
+    facts, as in Ussuri and earlier releases. See `story 2008394
+    <https://storyboard.openstack.org/#!/story/2008394>`__ for details.