baremetal: raise more specific ResourceFailure in wait_for_* methods

Raising a generic SDKException makes it harder to distinguish them
from other failures. Also add missing docstrings.

Change-Id: I576469b0bdb664512ce9d34d5217329f3fa62b64

openstack/baremetal/v1/_proxy.py

@@ -344,6 +344,9 @@ class Proxy(proxy.Proxy):
 
         :return: The list of :class:`~openstack.baremetal.v1.node.Node`
             instances that reached the requested state.
+        :raises: :class:`~openstack.exceptions.ResourceFailure` if a node
+            reaches an error state and ``abort_on_failed_state`` is ``True``.
+        :raises: :class:`~openstack.exceptions.ResourceTimeout` on timeout.
         """
         log_nodes = ', '.join(n.id if isinstance(n, _node.Node) else n
                               for n in nodes)
@@ -895,6 +898,9 @@ class Proxy(proxy.Proxy):
 
         :returns: The instance of the allocation.
         :rtype: :class:`~openstack.baremetal.v1.allocation.Allocation`.
+        :raises: :class:`~openstack.exceptions.ResourceFailure` if allocation
+            fails and ``ignore_error`` is ``False``.
+        :raises: :class:`~openstack.exceptions.ResourceTimeout` on timeout.
         """
         res = self._get_resource(_allocation.Allocation, allocation)
         return res.wait(self, timeout=timeout, ignore_error=ignore_error)

openstack/baremetal/v1/allocation.py

@@ -76,6 +76,9 @@ class Allocation(_common.ListMixin, resource.Resource):
             state is considered successful and the call returns.
 
         :return: This :class:`Allocation` instance.
+        :raises: :class:`~openstack.exceptions.ResourceFailure` if allocation
+            fails and ``ignore_error`` is ``False``.
+        :raises: :class:`~openstack.exceptions.ResourceTimeout` on timeout.
         """
         if self.state == 'active':
             return self
@@ -86,7 +89,7 @@ class Allocation(_common.ListMixin, resource.Resource):
             self.fetch(session)
 
             if self.state == 'error' and not ignore_error:
-                raise exceptions.SDKException(
+                raise exceptions.ResourceFailure(
                     "Allocation %(allocation)s failed: %(error)s" %
                     {'allocation': self.id, 'error': self.last_error})
             elif self.state != 'allocating':

openstack/baremetal/v1/node.py

@@ -323,6 +323,10 @@ class Node(_common.ListMixin, resource.Resource):
         :return: This :class:`Node` instance.
         :raises: ValueError if ``config_drive``, ``clean_steps`` or
             ``rescue_password`` are provided with an invalid ``target``.
+        :raises: :class:`~openstack.exceptions.ResourceFailure` if the node
+            reaches an error state while waiting for the state.
+        :raises: :class:`~openstack.exceptions.ResourceTimeout` if timeout
+            is reached while waiting for the state.
         """
         session = self._get_session(session)
 
@@ -400,6 +404,9 @@ class Node(_common.ListMixin, resource.Resource):
             ``manageable`` transition is ``enroll`` again.
 
         :return: This :class:`Node` instance.
+        :raises: :class:`~openstack.exceptions.ResourceFailure` if the node
+            reaches an error state and ``abort_on_failed_state`` is ``True``.
+        :raises: :class:`~openstack.exceptions.ResourceTimeout` on timeout.
         """
         for count in utils.iterate_timeout(
                 timeout,
@@ -469,8 +476,8 @@ class Node(_common.ListMixin, resource.Resource):
             ``manageable`` transition is ``enroll`` again.
 
         :return: ``True`` if the target state is reached
-        :raises: SDKException if ``abort_on_failed_state`` is ``True`` and
-            a failure state is reached.
+        :raises: :class:`~openstack.exceptions.ResourceFailure` if the node
+            reaches an error state and ``abort_on_failed_state`` is ``True``.
         """
         # NOTE(dtantsur): microversion 1.2 changed None to available
         if (self.provision_state == expected_state
@@ -481,7 +488,7 @@ class Node(_common.ListMixin, resource.Resource):
             return False
 
         if self.provision_state.endswith(' failed'):
-            raise exceptions.SDKException(
+            raise exceptions.ResourceFailure(
                 "Node %(node)s reached failure state \"%(state)s\"; "
                 "the last error is %(error)s" %
                 {'node': self.id, 'state': self.provision_state,
@@ -490,7 +497,7 @@ class Node(_common.ListMixin, resource.Resource):
         # "enroll"
         elif (expected_state == 'manageable'
               and self.provision_state == 'enroll' and self.last_error):
-            raise exceptions.SDKException(
+            raise exceptions.ResourceFailure(
                 "Node %(node)s could not reach state manageable: "
                 "failed to verify management credentials; "
                 "the last error is %(error)s" %

openstack/baremetal_introspection/v1/_proxy.py

@@ -125,6 +125,9 @@ class Proxy(proxy.Proxy):
             if the introspection reaches the ``error`` state. Otherwise the
             error state is considered successful and the call returns.
         :returns: :class:`~.introspection.Introspection` instance.
+        :raises: :class:`~openstack.exceptions.ResourceFailure` if
+            introspection fails and ``ignore_error`` is ``False``.
+        :raises: :class:`~openstack.exceptions.ResourceTimeout` on timeout.
         """
         res = self._get_resource(_introspect.Introspection, introspection)
         return res.wait(self, timeout=timeout, ignore_error=ignore_error)

openstack/baremetal_introspection/v1/introspection.py

@@ -107,6 +107,9 @@ class Introspection(resource.Resource):
             if the introspection reaches the ``error`` state. Otherwise the
             error state is considered successful and the call returns.
         :return: This :class:`Introspection` instance.
+        :raises: :class:`~openstack.exceptions.ResourceFailure` if
+            introspection fails and ``ignore_error`` is ``False``.
+        :raises: :class:`~openstack.exceptions.ResourceTimeout` on timeout.
         """
         if self._check_state(ignore_error):
             return self
@@ -124,7 +127,7 @@ class Introspection(resource.Resource):
 
     def _check_state(self, ignore_error):
         if self.state == 'error' and not ignore_error:
-            raise exceptions.SDKException(
+            raise exceptions.ResourceFailure(
                 "Introspection of node %(node)s failed: %(error)s" %
                 {'node': self.id, 'error': self.error})
         else:

openstack/tests/unit/baremetal/v1/test_allocation.py

@@ -104,6 +104,36 @@ class TestWaitForAllocation(base.TestCase):
         self.assertIs(allocation, self.allocation)
         self.assertEqual(2, mock_fetch.call_count)
 
+    def test_failure(self, mock_fetch):
+        marker = [False]  # mutable object to modify in the closure
+
+        def _side_effect(allocation, session):
+            if marker[0]:
+                self.allocation.state = 'error'
+                self.allocation.last_error = 'boom!'
+            else:
+                marker[0] = True
+
+        mock_fetch.side_effect = _side_effect
+        self.assertRaises(exceptions.ResourceFailure,
+                          self.allocation.wait, self.session)
+        self.assertEqual(2, mock_fetch.call_count)
+
+    def test_failure_ignored(self, mock_fetch):
+        marker = [False]  # mutable object to modify in the closure
+
+        def _side_effect(allocation, session):
+            if marker[0]:
+                self.allocation.state = 'error'
+                self.allocation.last_error = 'boom!'
+            else:
+                marker[0] = True
+
+        mock_fetch.side_effect = _side_effect
+        allocation = self.allocation.wait(self.session, ignore_error=True)
+        self.assertIs(allocation, self.allocation)
+        self.assertEqual(2, mock_fetch.call_count)
+
     def test_timeout(self, mock_fetch):
         self.assertRaises(exceptions.ResourceTimeout,
                           self.allocation.wait, self.session, timeout=0.001)

openstack/tests/unit/baremetal/v1/test_node.py

@@ -178,7 +178,7 @@ class TestNodeWaitForProvisionState(base.TestCase):
 
         mock_fetch.side_effect = _get_side_effect
 
-        self.assertRaisesRegex(exceptions.SDKException,
+        self.assertRaisesRegex(exceptions.ResourceFailure,
                                'failure state "deploy failed"',
                                self.node.wait_for_provision_state,
                                self.session, 'manageable')
@@ -191,7 +191,7 @@ class TestNodeWaitForProvisionState(base.TestCase):
 
         mock_fetch.side_effect = _get_side_effect
 
-        self.assertRaisesRegex(exceptions.SDKException,
+        self.assertRaisesRegex(exceptions.ResourceFailure,
                                'failed to verify management credentials',
                                self.node.wait_for_provision_state,
                                self.session, 'manageable')

openstack/tests/unit/baremetal_introspection/v1/test_proxy.py

@@ -86,7 +86,7 @@ class TestWaitForIntrospection(base.TestCase):
             self.introspection.error = 'boom'
 
         mock_fetch.side_effect = _side_effect
-        self.assertRaisesRegex(exceptions.SDKException, 'boom',
+        self.assertRaisesRegex(exceptions.ResourceFailure, 'boom',
                                self.proxy.wait_for_introspection,
                                self.introspection)
         mock_fetch.assert_called_once_with(self.introspection, self.proxy)

releasenotes/notes/baremetal-wait-e4571cdb150b188a.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    The baremetal calls ``wait_for_nodes_provision_state``,
+    ``wait_for_allocation`` and the baremetal introspection call
+    ``wait_for_introspection`` now raise ``ResourceFailure`` on reaching
+    an error state instead of a generic ``SDKException``.