openstack
/
openstacksdk
Code Issues Proposed changes

Several improvements to resource.wait_for_status 

* Clarify that interval and wait can be None (already supported
  by the underlying interval_timeout function).
* Fix incorrect :returns: docstring.
* Allow the caller to override which attribute to use.
  (ironic nodes have provision_state and power_state, not status).
* Add some logging when looping (the underlying interval_timeout
  call does it, but just "Waiting %d seconds" is not enough in
  the context of a big caller application).
* Use lower() on the initial check for status to be consistent
  with how it is treated inside the loop (correctly handling None,
  which is a possible state according to the CI).

Change-Id: Icd58fdab1438b738d3ce6cd3162bb7445e9a2bf9
This commit is contained in:
Dmitry Tantsur 2018-05-25 10:28:22 +02:00
parent 2d4be744a9
commit 4e8a1f6ae9
2 changed files with 91 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
openstack/resource.py
View File

@ -37,6 +37,7 @@ import itertools
from keystoneauth1 import adapter
from requests import structures
from openstack import _log
from openstack import exceptions
from openstack import format
from openstack import utils
@ -1047,21 +1048,31 @@ class Resource(object):
"No %s found for %s" % (cls.__name__, name_or_id))
def wait_for_status(session, resource, status, failures, interval, wait):
def _normalize_status(status):
if status is not None:
status = status.lower()
return status
def wait_for_status(session, resource, status, failures, interval=None,
wait=None, attribute='status'):
"""Wait for the resource to be in a particular status.
:param session: The session to use for making this request.
:type session: :class:`~keystoneauth1.adapter.Adapter`
:param resource: The resource to wait on to reach the status. The resource
must have a status attribute.
must have a status attribute specified via ``attribute``.
:type resource: :class:`~openstack.resource.Resource`
:param status: Desired status of the resource.
:param list failures: Statuses that would indicate the transition
failed such as 'ERROR'. Defaults to ['ERROR'].
:param interval: Number of seconds to wait between checks.
Set to ``None`` to use the default interval.
:param wait: Maximum number of seconds to wait for transition.
Set to ``None`` to wait forever.
:param attribute: Name of the resource attribute that contains the status.
:return: Method returns self on success.
:return: The updated resource.
:raises: :class:`~openstack.exceptions.ResourceTimeout` transition
to status failed to occur in wait seconds.
:raises: :class:`~openstack.exceptions.ResourceFailure` resource
@ -1069,7 +1080,10 @@ def wait_for_status(session, resource, status, failures, interval, wait):
:raises: :class:`~AttributeError` if the resource does not have a status
attribute
"""
if resource.status == status:
log = _log.setup_logging(__name__)
current_status = getattr(resource, attribute)
if _normalize_status(current_status) == status.lower():
return resource
if failures is None:
@ -1090,13 +1104,18 @@ def wait_for_status(session, resource, status, failures, interval, wait):
raise exceptions.ResourceFailure(
"{name} went away while waiting for {status}".format(
name=name, status=status))
new_status = resource.status
if new_status.lower() == status.lower():
new_status = getattr(resource, attribute)
normalized_status = _normalize_status(new_status)
if normalized_status == status.lower():
return resource
if resource.status.lower() in failures:
elif normalized_status in failures:
raise exceptions.ResourceFailure(
"{name} transitioned to failure state {status}".format(
name=name, status=resource.status))
name=name, status=new_status))
log.debug('Still waiting for resource %s to reach state %s, '
'current state is %s', name, status, new_status)
def wait_for_delete(session, resource, interval, wait):

68
openstack/tests/unit/test_resource.py
View File

@ -1771,7 +1771,7 @@ class TestWaitForStatus(base.TestCase):
def test_immediate_status(self):
status = "loling"
res = mock.Mock()
res = mock.Mock(spec=['id', 'status'])
res.status = status
result = resource.wait_for_status(
@ -1779,11 +1779,34 @@ class TestWaitForStatus(base.TestCase):
self.assertTrue(result, res)
def _resources_from_statuses(self, *statuses):
def test_immediate_status_case(self):
status = "LOLing"
res = mock.Mock(spec=['id', 'status'])
res.status = status
result = resource.wait_for_status(
"session", res, 'lOling', "failures", "interval", "wait")
self.assertTrue(result, res)
def test_immediate_status_different_attribute(self):
status = "loling"
res = mock.Mock(spec=['id', 'mood'])
res.mood = status
result = resource.wait_for_status(
"session", res, status, "failures", "interval", "wait",
attribute='mood')
self.assertTrue(result, res)
def _resources_from_statuses(self, *statuses, **kwargs):
attribute = kwargs.pop('attribute', 'status')
assert not kwargs, 'Unexpected keyword arguments: %s' % kwargs
resources = []
for status in statuses:
res = mock.Mock()
res.status = status
res = mock.Mock(spec=['id', 'get', attribute])
setattr(res, attribute, status)
resources.append(res)
for index, res in enumerate(resources[:-1]):
res.get.return_value = resources[index + 1]
@ -1802,6 +1825,31 @@ class TestWaitForStatus(base.TestCase):
self.assertEqual(result, resources[-1])
def test_status_match_with_none(self):
status = "loling"
# apparently, None is a correct state in some cases
resources = self._resources_from_statuses(
None, "other", None, "another", status)
result = resource.wait_for_status(
mock.Mock(), resources[0], status, None, 1, 5)
self.assertEqual(result, resources[-1])
def test_status_match_different_attribute(self):
status = "loling"
resources = self._resources_from_statuses(
"first", "other", "another", "another", status,
attribute='mood')
result = resource.wait_for_status(
mock.Mock(), resources[0], status, None, 1, 5,
attribute='mood')
self.assertEqual(result, resources[-1])
def test_status_fails(self):
failure = "crying"
@ -1812,6 +1860,18 @@ class TestWaitForStatus(base.TestCase):
resource.wait_for_status,
mock.Mock(), resources[0], "loling", [failure], 1, 5)
def test_status_fails_different_attribute(self):
failure = "crying"
resources = self._resources_from_statuses("success", "other", failure,
attribute='mood')
self.assertRaises(
exceptions.ResourceFailure,
resource.wait_for_status,
mock.Mock(), resources[0], "loling", [failure.upper()], 1, 5,
attribute='mood')
def test_timeout(self):
status = "loling"
res = mock.Mock()