openstack
/
openstacksdk
Code Issues Proposed changes

Merge "Wire in retries for all baremetal actions"

This commit is contained in:
Zuul 2018-10-16 14:38:14 +00:00 committed by Gerrit Code Review
parent 834e10e167 af3a8f4c12
commit 20f12c0a02
7 changed files with 96 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
openstack/baremetal/v1/_proxy.py
View File

@ -11,6 +11,7 @@
# under the License.
from openstack import _log
from openstack.baremetal.v1 import _common
from openstack.baremetal.v1 import chassis as _chassis
from openstack.baremetal.v1 import driver as _driver
from openstack.baremetal.v1 import node as _node
@ -25,6 +26,8 @@ _logger = _log.setup_logging('openstack')
class Proxy(proxy.Proxy):
retriable_status_codes = _common.RETRIABLE_STATUS_CODES
def chassis(self, details=False, **query):
"""Retrieve a generator of chassis.
@ -238,18 +241,23 @@ class Proxy(proxy.Proxy):
"""
return self._get(_node.Node, node)
def update_node(self, node, **attrs):
def update_node(self, node, retry_on_conflict=True, **attrs):
"""Update a node.
:param chassis: Either the name or the ID of a node or an instance
of :class:`~openstack.baremetal.v1.node.Node`.
:param bool retry_on_conflict: Whether to retry HTTP CONFLICT error.
Most of the time it can be retried, since it is caused by the node
being locked. However, when setting ``instance_id``, this is
a normal code and should not be retried.
:param dict attrs: The attributes to update on the node represented
by the ``node`` parameter.
:returns: The updated node.
:rtype: :class:`~openstack.baremetal.v1.node.Node`
"""
return self._update(_node.Node, node, **attrs)
res = self._get_resource(_node.Node, node, **attrs)
return res.commit(self, retry_on_conflict=retry_on_conflict)
def set_node_provision_state(self, node, target, config_drive=None,
clean_steps=None, rescue_password=None,

14
openstack/proxy.py
View File

@ -42,6 +42,20 @@ def _check_resource(strict=False):
class Proxy(_adapter.OpenStackSDKAdapter):
"""Represents a service."""
retriable_status_codes = None
"""HTTP status codes that should be retried by default.
The number of retries is defined by the configuration in parameters called
``<service-type>_status_code_retries``.
"""
def __init__(self, *args, **kwargs):
# NOTE(dtantsur): keystoneauth defaults retriable_status_codes to None,
# override it with a class-level value.
kwargs.setdefault('retriable_status_codes',
self.retriable_status_codes)
super(Proxy, self).__init__(*args, **kwargs)
def _get_resource(self, resource_type, value, **attrs):
"""Get a resource object to work on

22
openstack/resource.py
View File

@ -1105,7 +1105,8 @@ class Resource(dict):
self._translate_response(response, has_body=False)
return self
def commit(self, session, prepend_key=True, has_body=True):
def commit(self, session, prepend_key=True, has_body=True,
retry_on_conflict=None):
"""Commit the state of the instance to the remote resource.
:param session: The session to use for making this request.
@ -1113,6 +1114,9 @@ class Resource(dict):
:param prepend_key: A boolean indicating whether the resource_key
should be prepended in a resource update request.
Default to True.
:param bool retry_on_conflict: Whether to enable retries on HTTP
CONFLICT (409). Value of ``None`` leaves
the `Adapter` defaults.
:return: This :class:`Resource` instance.
:raises: :exc:`~openstack.exceptions.MethodNotSupported` if
@ -1136,20 +1140,30 @@ class Resource(dict):
request = self._prepare_request(prepend_key=prepend_key, **kwargs)
session = self._get_session(session)
kwargs = {}
retriable_status_codes = set(session.retriable_status_codes or ())
if retry_on_conflict:
kwargs['retriable_status_codes'] = retriable_status_codes | {409}
elif retry_on_conflict is not None and retriable_status_codes:
# The baremetal proxy defaults to retrying on conflict, allow
# overriding it via an explicit retry_on_conflict=False.
kwargs['retriable_status_codes'] = retriable_status_codes - {409}
microversion = self._get_microversion_for(session, 'commit')
if self.commit_method == 'PATCH':
response = session.patch(
request.url, json=request.body, headers=request.headers,
microversion=microversion)
microversion=microversion, **kwargs)
elif self.commit_method == 'POST':
response = session.post(
request.url, json=request.body, headers=request.headers,
microversion=microversion)
microversion=microversion, **kwargs)
elif self.commit_method == 'PUT':
response = session.put(
request.url, json=request.body, headers=request.headers,
microversion=microversion)
microversion=microversion, **kwargs)
else:
raise exceptions.ResourceFailure(
msg="Invalid commit method: %s" % self.commit_method)

16
openstack/tests/unit/baremetal/v1/test_proxy.py
View File

@ -85,8 +85,20 @@ class TestBaremetalProxy(test_proxy_base.TestProxyBase):
def test_get_node(self):
self.verify_get(self.proxy.get_node, node.Node)
def test_update_node(self):
self.verify_update(self.proxy.update_node, node.Node)
@mock.patch.object(node.Node, 'commit', autospec=True)
def test_update_node(self, mock_commit):
self.proxy.update_node('uuid', instance_id='new value')
mock_commit.assert_called_once_with(mock.ANY, self.proxy,
retry_on_conflict=True)
self.assertEqual('new value', mock_commit.call_args[0][0].instance_id)
@mock.patch.object(node.Node, 'commit', autospec=True)
def test_update_node_no_retries(self, mock_commit):
self.proxy.update_node('uuid', instance_id='new value',
retry_on_conflict=False)
mock_commit.assert_called_once_with(mock.ANY, self.proxy,
retry_on_conflict=False)
self.assertEqual('new value', mock_commit.call_args[0][0].instance_id)
def test_delete_node(self):
self.verify_delete(self.proxy.delete_node, node.Node, False)

1
openstack/tests/unit/image/v2/test_image.py
View File

@ -104,6 +104,7 @@ class TestImage(base.TestCase):
self.sess = mock.Mock(spec=adapter.Adapter)
self.sess.post = mock.Mock(return_value=self.resp)
self.sess.default_microversion = None
self.sess.retriable_status_codes = None
def test_basic(self):
sot = image.Image()

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

@ -1016,6 +1016,7 @@ class TestResourceActions(base.TestCase):
self.session.delete = mock.Mock(return_value=self.response)
self.session.head = mock.Mock(return_value=self.response)
self.session.default_microversion = None
self.session.retriable_status_codes = None
self.endpoint_data = mock.Mock(max_microversion='1.99',
min_microversion=None)
@ -1157,7 +1158,8 @@ class TestResourceActions(base.TestCase):
self.assertEqual(result, sot)
def _test_commit(self, commit_method='PUT', prepend_key=True,
has_body=True, microversion=None):
has_body=True, microversion=None,
commit_args=None, expected_args=None):
self.sot.commit_method = commit_method
# Need to make sot look dirty so we can attempt an update
@ -1165,7 +1167,7 @@ class TestResourceActions(base.TestCase):
self.sot._body.dirty = mock.Mock(return_value={"x": "y"})
self.sot.commit(self.session, prepend_key=prepend_key,
has_body=has_body)
has_body=has_body, **(commit_args or {}))
self.sot._prepare_request.assert_called_once_with(
prepend_key=prepend_key)
@ -1174,17 +1176,17 @@ class TestResourceActions(base.TestCase):
self.session.patch.assert_called_once_with(
self.request.url,
json=self.request.body, headers=self.request.headers,
microversion=microversion)
microversion=microversion, **(expected_args or {}))
elif commit_method == 'POST':
self.session.post.assert_called_once_with(
self.request.url,
json=self.request.body, headers=self.request.headers,
microversion=microversion)
microversion=microversion, **(expected_args or {}))
elif commit_method == 'PUT':
self.session.put.assert_called_once_with(
self.request.url,
json=self.request.body, headers=self.request.headers,
microversion=microversion)
microversion=microversion, **(expected_args or {}))
self.assertEqual(self.sot.microversion, microversion)
self.sot._translate_response.assert_called_once_with(
@ -1197,6 +1199,32 @@ class TestResourceActions(base.TestCase):
self._test_commit(
commit_method='PATCH', prepend_key=False, has_body=False)
def test_commit_patch_retry_on_conflict(self):
self._test_commit(
commit_method='PATCH',
commit_args={'retry_on_conflict': True},
expected_args={'retriable_status_codes': {409}})
def test_commit_put_retry_on_conflict(self):
self._test_commit(
commit_method='PUT',
commit_args={'retry_on_conflict': True},
expected_args={'retriable_status_codes': {409}})
def test_commit_patch_no_retry_on_conflict(self):
self.session.retriable_status_codes = {409, 503}
self._test_commit(
commit_method='PATCH',
commit_args={'retry_on_conflict': False},
expected_args={'retriable_status_codes': {503}})
def test_commit_put_no_retry_on_conflict(self):
self.session.retriable_status_codes = {409, 503}
self._test_commit(
commit_method='PATCH',
commit_args={'retry_on_conflict': False},
expected_args={'retriable_status_codes': {503}})
def test_commit_not_dirty(self):
self.sot._body = mock.Mock()
self.sot._body.dirty = dict()

6
releasenotes/notes/baremetal-retries-ff8aa8f73fb97415.yaml Normal file
View File

@ -0,0 +1,6 @@
---
features:
- |
The bare metal operations now retry HTTP 409 and 503 by default. The number
of retries can be changes via the ``baremetal_status_code_retries``
configuration option (defaulting to 5).