From 9555ad281c95b10707992cd3640e536e7fbab4cc Mon Sep 17 00:00:00 2001
From: Kaifeng Wang <kaifeng.w@gmail.com>
Date: Wed, 23 Jan 2019 11:30:34 +0800
Subject: [PATCH] Fix listing nodes with conductor could raise

In the case of conductor service goes offline, some hardware types
may be not available as well, get_conductor_for is not protected for
such case when querying node with conductor.

This patch adds exception handling and bypasses orphaned nodes.

Change-Id: I381585240057a989ab269ea35ea9b2124527bf8e
Story: 2004834
Task: 29028
---
 ironic/api/controllers/v1/node.py             | 12 +++++++---
 .../unit/api/controllers/v1/test_node.py      | 24 +++++++++++++++++++
 ...conductor-list-raise-131ac76719b74032.yaml |  6 +++++
 3 files changed, 39 insertions(+), 3 deletions(-)
 create mode 100644 releasenotes/notes/fix-conductor-list-raise-131ac76719b74032.yaml

diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py
index d18452cb8b..ef98ac988d 100644
--- a/ironic/api/controllers/v1/node.py
+++ b/ironic/api/controllers/v1/node.py
@@ -1587,9 +1587,15 @@ class NodesController(rest.RestController):
     def _filter_by_conductor(self, nodes, conductor):
         filtered_nodes = []
         for n in nodes:
-            host = pecan.request.rpcapi.get_conductor_for(n)
-            if host == conductor:
-                filtered_nodes.append(n)
+            try:
+                host = pecan.request.rpcapi.get_conductor_for(n)
+                if host == conductor:
+                    filtered_nodes.append(n)
+            except (exception.NoValidHost, exception.TemporaryFailure):
+                # NOTE(kaifeng) Node gets orphaned in case some conductor
+                # offline or all conductors are offline.
+                pass
+
         return filtered_nodes
 
     def _get_nodes_collection(self, chassis_uuid, instance_uuid, associated,
diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py
index 27f13d7dcf..f5b717710c 100644
--- a/ironic/tests/unit/api/controllers/v1/test_node.py
+++ b/ironic/tests/unit/api/controllers/v1/test_node.py
@@ -1636,6 +1636,30 @@ class TestListNodes(test_api_base.BaseApiTest):
         self.assertNotIn(node1.uuid, uuids)
         self.assertIn(node2.uuid, uuids)
 
+    def test_get_nodes_by_conductor_no_valid_host(self):
+        obj_utils.create_test_node(self.context,
+                                   uuid=uuidutils.generate_uuid())
+
+        self.mock_get_conductor_for.side_effect = exception.NoValidHost(
+            reason='hey a conductor just goes vacation')
+        response = self.get_json('/nodes?conductor=like.shadows',
+                                 headers={api_base.Version.string: "1.49"})
+        self.assertEqual([], response['nodes'])
+
+        self.mock_get_conductor_for.side_effect = exception.TemporaryFailure(
+            reason='this must be conductor strike')
+        response = self.get_json('/nodes?conductor=like.shadows',
+                                 headers={api_base.Version.string: "1.49"})
+        self.assertEqual([], response['nodes'])
+
+        self.mock_get_conductor_for.side_effect = exception.IronicException(
+            'Some unexpected thing happened')
+        response = self.get_json('/nodes?conductor=fake.conductor',
+                                 headers={api_base.Version.string: "1.49"},
+                                 expect_errors=True)
+        self.assertIn('Some unexpected thing happened',
+                      response.json['error_message'])
+
     def test_get_nodes_by_owner(self):
         node1 = obj_utils.create_test_node(self.context,
                                            uuid=uuidutils.generate_uuid(),
diff --git a/releasenotes/notes/fix-conductor-list-raise-131ac76719b74032.yaml b/releasenotes/notes/fix-conductor-list-raise-131ac76719b74032.yaml
new file mode 100644
index 0000000000..fb0d748b20
--- /dev/null
+++ b/releasenotes/notes/fix-conductor-list-raise-131ac76719b74032.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes an issue that node list with conductor fails if any of the nodes
+    has an invalid hardware type, which may happen when some conductor is
+    out of service.