From 00647dbb241563b98325b6fbc0c72fb01abecd83 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Thu, 12 Mar 2020 11:16:43 +0100 Subject: [PATCH] baremetal: fail-less mode for wait_for_nodes_provision_state This change adds a new `fail` flag to the call. If set to False, the call will return a namedtuple (success, failure, timeout) with lists of nodes. Timeout and failure exceptions will not be raised. Change-Id: Ie20193ce51fcd5ce3ffd479143225bd1a1e8c94a --- .../user/resources/baremetal/v1/node.rst | 7 ++ openstack/baremetal/v1/_proxy.py | 68 ++++++++++++------- openstack/baremetal/v1/node.py | 19 ++++++ .../tests/unit/baremetal/v1/test_proxy.py | 54 +++++++++++++++ ...vision-state-no-fail-efa74dd39f687df8.yaml | 6 ++ 5 files changed, 131 insertions(+), 23 deletions(-) create mode 100644 releasenotes/notes/wait-provision-state-no-fail-efa74dd39f687df8.yaml diff --git a/doc/source/user/resources/baremetal/v1/node.rst b/doc/source/user/resources/baremetal/v1/node.rst index bf5f8a694..ebc5fb327 100644 --- a/doc/source/user/resources/baremetal/v1/node.rst +++ b/doc/source/user/resources/baremetal/v1/node.rst @@ -18,3 +18,10 @@ The ``ValidationResult`` class represents the result of a validation. .. autoclass:: openstack.baremetal.v1.node.ValidationResult :members: + +The WaitResult Class +^^^^^^^^^^^^^^^^^^^^ + +The ``WaitResult`` class represents the result of waiting for several nodes. + +.. autoclass:: openstack.baremetal.v1.node.WaitResult diff --git a/openstack/baremetal/v1/_proxy.py b/openstack/baremetal/v1/_proxy.py index 0e00b3914..0c4e5b4e9 100644 --- a/openstack/baremetal/v1/_proxy.py +++ b/openstack/baremetal/v1/_proxy.py @@ -17,6 +17,7 @@ from openstack.baremetal.v1 import driver as _driver from openstack.baremetal.v1 import node as _node from openstack.baremetal.v1 import port as _port from openstack.baremetal.v1 import port_group as _portgroup +from openstack import exceptions from openstack import proxy from openstack import utils @@ -371,7 +372,8 @@ class Proxy(proxy.Proxy): def wait_for_nodes_provision_state(self, nodes, expected_state, timeout=None, - abort_on_failed_state=True): + abort_on_failed_state=True, + fail=True): """Wait for the nodes to reach the expected state. :param nodes: List of nodes - name, ID or @@ -384,9 +386,13 @@ class Proxy(proxy.Proxy): if any node reaches a failure state which does not match the expected one. Note that the failure state for ``enroll`` -> ``manageable`` transition is ``enroll`` again. + :param fail: If set to ``False`` this call will not raise on timeouts + and provisioning failures. - :return: The list of :class:`~openstack.baremetal.v1.node.Node` - instances that reached the requested state. + :return: If `fail` is ``True`` (the default), the list of + :class:`~openstack.baremetal.v1.node.Node` instances that reached + the requested state. If `fail` is ``False``, a + :class:`~openstack.baremetal.v1.node.WaitResult` named tuple. :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. @@ -395,29 +401,45 @@ class Proxy(proxy.Proxy): for n in nodes) finished = [] + failed = [] remaining = nodes - for count in utils.iterate_timeout( - timeout, - "Timeout waiting for nodes %(nodes)s to reach " - "target state '%(state)s'" % {'nodes': log_nodes, - 'state': expected_state}): - nodes = [self.get_node(n) for n in remaining] - remaining = [] - for n in nodes: - if n._check_state_reached(self, expected_state, - abort_on_failed_state): - finished.append(n) - else: - remaining.append(n) + try: + for count in utils.iterate_timeout( + timeout, + "Timeout waiting for nodes %(nodes)s to reach " + "target state '%(state)s'" % {'nodes': log_nodes, + 'state': expected_state}): + nodes = [self.get_node(n) for n in remaining] + remaining = [] + for n in nodes: + try: + if n._check_state_reached(self, expected_state, + abort_on_failed_state): + finished.append(n) + else: + remaining.append(n) + except exceptions.ResourceFailure: + if fail: + raise + else: + failed.append(n) - if not remaining: - return finished + if not remaining: + if fail: + return finished + else: + return _node.WaitResult(finished, failed, []) - self.log.debug( - 'Still waiting for nodes %(nodes)s to reach state ' - '"%(target)s"', - {'nodes': ', '.join(n.id for n in remaining), - 'target': expected_state}) + self.log.debug( + 'Still waiting for nodes %(nodes)s to reach state ' + '"%(target)s"', + {'nodes': ', '.join(n.id for n in remaining), + 'target': expected_state}) + except exceptions.ResourceTimeout: + if fail: + raise + else: + return _node.WaitResult(finished, failed, remaining) def set_node_power_state(self, node, target): """Run an action modifying node's power state. diff --git a/openstack/baremetal/v1/node.py b/openstack/baremetal/v1/node.py index 7a515af8f..c2bd09d60 100644 --- a/openstack/baremetal/v1/node.py +++ b/openstack/baremetal/v1/node.py @@ -10,6 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. +import collections + from openstack.baremetal.v1 import _common from openstack import exceptions from openstack import resource @@ -30,6 +32,23 @@ class ValidationResult(object): self.reason = reason +class WaitResult(collections.namedtuple('WaitResult', + ['success', 'failure', 'timeout'])): + """A named tuple representing a result of waiting for several nodes. + + Each component is a list of :class:`~openstack.baremetal.v1.node.Node` + objects: + + :ivar ~.success: a list of :class:`~openstack.baremetal.v1.node.Node` + objects that reached the state. + :ivar ~.timeout: a list of :class:`~openstack.baremetal.v1.node.Node` + objects that reached timeout. + :ivar ~.failure: a list of :class:`~openstack.baremetal.v1.node.Node` + objects that hit a failure. + """ + __slots__ = () + + class Node(_common.ListMixin, resource.Resource): resources_key = 'nodes' diff --git a/openstack/tests/unit/baremetal/v1/test_proxy.py b/openstack/tests/unit/baremetal/v1/test_proxy.py index ee3600d88..b18bb921c 100644 --- a/openstack/tests/unit/baremetal/v1/test_proxy.py +++ b/openstack/tests/unit/baremetal/v1/test_proxy.py @@ -235,6 +235,25 @@ class TestWaitForNodesProvisionState(base.TestCase): n._check_state_reached.assert_called_once_with( self.proxy, 'fake state', True) + def test_success_no_fail(self, mock_get): + # two attempts, one node succeeds after the 1st + nodes = [mock.Mock(spec=node.Node, id=str(i)) + for i in range(3)] + for i, n in enumerate(nodes): + # 1st attempt on 1st node, 2nd attempt on 2nd node + n._check_state_reached.return_value = not (i % 2) + mock_get.side_effect = nodes + + result = self.proxy.wait_for_nodes_provision_state( + ['abcd', node.Node(id='1234')], 'fake state', fail=False) + self.assertEqual([nodes[0], nodes[2]], result.success) + self.assertEqual([], result.failure) + self.assertEqual([], result.timeout) + + for n in nodes: + n._check_state_reached.assert_called_once_with( + self.proxy, 'fake state', True) + def test_timeout(self, mock_get): mock_get.return_value._check_state_reached.return_value = False mock_get.return_value.id = '1234' @@ -245,3 +264,38 @@ class TestWaitForNodesProvisionState(base.TestCase): timeout=0.001) mock_get.return_value._check_state_reached.assert_called_with( self.proxy, 'fake state', True) + + def test_timeout_no_fail(self, mock_get): + mock_get.return_value._check_state_reached.return_value = False + mock_get.return_value.id = '1234' + + result = self.proxy.wait_for_nodes_provision_state( + ['abcd'], 'fake state', timeout=0.001, fail=False) + mock_get.return_value._check_state_reached.assert_called_with( + self.proxy, 'fake state', True) + + self.assertEqual([], result.success) + self.assertEqual([mock_get.return_value], result.timeout) + self.assertEqual([], result.failure) + + def test_timeout_and_failures_not_fail(self, mock_get): + def _fake_get(_self, uuid): + result = mock.Mock() + result.id = uuid + if uuid == '1': + result._check_state_reached.return_value = True + elif uuid == '2': + result._check_state_reached.side_effect = \ + exceptions.ResourceFailure("boom") + else: + result._check_state_reached.return_value = False + return result + + mock_get.side_effect = _fake_get + + result = self.proxy.wait_for_nodes_provision_state( + ['1', '2', '3'], 'fake state', timeout=0.001, fail=False) + + self.assertEqual(['1'], [x.id for x in result.success]) + self.assertEqual(['3'], [x.id for x in result.timeout]) + self.assertEqual(['2'], [x.id for x in result.failure]) diff --git a/releasenotes/notes/wait-provision-state-no-fail-efa74dd39f687df8.yaml b/releasenotes/notes/wait-provision-state-no-fail-efa74dd39f687df8.yaml new file mode 100644 index 000000000..5c4fbca5a --- /dev/null +++ b/releasenotes/notes/wait-provision-state-no-fail-efa74dd39f687df8.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Adds an ability for the bare metal ``wait_for_nodes_provision_state`` call + to return an object with nodes that succeeded, failed or timed out instead + of raising an exception.