From 592ac2c7d0f098f9da622b976717f74d525beb25 Mon Sep 17 00:00:00 2001
From: Olivier Bourdon <obourdon@opennext.io>
Date: Tue, 6 Mar 2018 09:58:33 +0100
Subject: [PATCH] Add proper return value for validate_node

Change-Id: I361c520e6697750137e86d32dc3890fd8af4a99c
---
 shade/openstackcloud.py                 | 33 +++++++++----
 shade/tests/unit/test_baremetal_node.py | 63 ++++++++++++-------------
 2 files changed, 54 insertions(+), 42 deletions(-)

diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py
index 937d98851..7f73f233c 100644
--- a/shade/openstackcloud.py
+++ b/shade/openstackcloud.py
@@ -9384,20 +9384,33 @@ class OpenStackCloud(
                 )
 
     def validate_node(self, uuid):
-        # TODO(TheJulia): There are soooooo many other interfaces
-        # that we can support validating, while these are essential,
-        # we should support more.
-        # TODO(TheJulia): Add a doc string :(
+        """Returns node validation information
+
+        :param string uuid: A UUID value representing the baremetal node.
+
+        :raises: OpenStackCloudException on operation error or
+                  if deploy and power informations are not present.
+
+        :returns: dict containing validation information for each
+                  interface: boot, console, deploy, inspect, management,
+                  network, power, raid, rescue, storage, ...
+        """
         msg = ("Failed to query the API for validation status of "
                "node {node_id}").format(node_id=uuid)
         url = '/nodes/{node_id}/validate'.format(node_id=uuid)
-        ifaces = self._baremetal_client.get(url, error_message=msg)
-
-        if not ifaces['deploy'] or not ifaces['power']:
+        validate_resp = self._baremetal_client.get(url, error_message=msg)
+        is_deploy_valid = validate_resp.get(
+            'deploy', {'result': False}).get('result', False)
+        is_power_valid = validate_resp.get(
+            'power', {'result': False}).get('result', False)
+        if not is_deploy_valid or not is_power_valid:
             raise OpenStackCloudException(
-                "ironic node %s failed to validate. "
-                "(deploy: %s, power: %s)" % (ifaces['deploy'],
-                                             ifaces['power']))
+                "ironic node {} failed to validate. "
+                "(deploy: {}, power: {})".format(
+                    uuid,
+                    validate_resp.get('deploy'),
+                    validate_resp.get('power')))
+        return validate_resp
 
     def node_set_provision_state(self,
                                  name_or_id,
diff --git a/shade/tests/unit/test_baremetal_node.py b/shade/tests/unit/test_baremetal_node.py
index 9a8795745..e65d5e80c 100644
--- a/shade/tests/unit/test_baremetal_node.py
+++ b/shade/tests/unit/test_baremetal_node.py
@@ -115,40 +115,39 @@ class TestBaremetalNode(base.IronicTestCase):
                              'validate']),
                  json=validate_return),
         ])
-        self.op_cloud.validate_node(self.fake_baremetal_node['uuid'])
-
+        validate_resp = self.op_cloud.validate_node(
+            self.fake_baremetal_node['uuid'])
+        self.assertDictEqual(validate_return, validate_resp)
         self.assert_calls()
 
-    # FIXME(TheJulia): So, this doesn't presently fail, but should fail.
-    # Placing the test here, so we can sort out the issue in the actual
-    # method later.
-    # def test_validate_node_raises_exception(self):
-    #    validate_return = {
-    #        'deploy': {
-    #            'result': False,
-    #            'reason': 'error!',
-    #        },
-    #        'power': {
-    #            'result': False,
-    #            'reason': 'meow!',
-    #        },
-    #        'foo': {
-    #            'result': True
-    #        }}
-    #    self.register_uris([
-    #        dict(method='GET',
-    #             uri=self.get_mock_url(
-    #                 resource='nodes',
-    #                 append=[self.fake_baremetal_node['uuid'],
-    #                         'validate']),
-    #             json=validate_return),
-    #    ])
-    #    self.assertRaises(
-    #        Exception,
-    #        self.op_cloud.validate_node,
-    #        self.fake_baremetal_node['uuid'])
-    #
-    #    self.assert_calls()
+    def test_validate_node_raises_exception(self):
+        validate_return = {
+            'deploy': {
+                'result': False,
+                'reason': 'error!',
+            },
+            'power': {
+                'result': False,
+                'reason': 'meow!',
+            },
+            'foo': {
+                'result': True
+            }}
+        self.register_uris([
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     resource='nodes',
+                     append=[self.fake_baremetal_node['uuid'],
+                             'validate']),
+                 json=validate_return),
+        ])
+        self.assertRaisesRegexp(
+            exc.OpenStackCloudException,
+            '^ironic node .* failed to validate.*',
+            self.op_cloud.validate_node,
+            self.fake_baremetal_node['uuid'])
+
+        self.assert_calls()
 
     def test_patch_machine(self):
         test_patch = [{