Merge "Return retries on HTTP CONFLICT to baremetal.attach_vif_to_node"

This commit is contained in:
Zuul 2019-01-19 13:17:15 +00:00 committed by Gerrit Code Review
commit 6a77be01bf
4 changed files with 32 additions and 7 deletions

View File

@ -632,7 +632,7 @@ class Proxy(proxy.Proxy):
return self._delete(_portgroup.PortGroup, port_group,
ignore_missing=ignore_missing)
def attach_vif_to_node(self, node, vif_id):
def attach_vif_to_node(self, node, vif_id, retry_on_conflict=True):
"""Attach a VIF to the node.
The exact form of the VIF ID depends on the network interface used by
@ -643,12 +643,16 @@ class Proxy(proxy.Proxy):
:param node: The value can be either the name or ID of a node or
a :class:`~openstack.baremetal.v1.node.Node` instance.
:param string vif_id: Backend-specific VIF ID.
:param retry_on_conflict: Whether to retry HTTP CONFLICT errors.
This can happen when either the VIF is already used on a node or
the node is locked. Since the latter happens more often, the
default value is True.
:return: ``None``
:raises: :exc:`~openstack.exceptions.NotSupported` if the server
does not support the VIF API.
"""
res = self._get_resource(_node.Node, node)
res.attach_vif(self, vif_id)
res.attach_vif(self, vif_id, retry_on_conflict=retry_on_conflict)
def detach_vif_from_node(self, node, vif_id, ignore_missing=True):
"""Detach a VIF from the node.

View File

@ -457,7 +457,7 @@ class Node(_common.ListMixin, resource.Resource):
"the last error is %(error)s" %
{'node': self.id, 'error': self.last_error})
def attach_vif(self, session, vif_id):
def attach_vif(self, session, vif_id, retry_on_conflict=True):
"""Attach a VIF to the node.
The exact form of the VIF ID depends on the network interface used by
@ -468,6 +468,10 @@ class Node(_common.ListMixin, resource.Resource):
:param session: The session to use for making this request.
:type session: :class:`~keystoneauth1.adapter.Adapter`
:param string vif_id: Backend-specific VIF ID.
:param retry_on_conflict: Whether to retry HTTP CONFLICT errors.
This can happen when either the VIF is already used on a node or
the node is locked. Since the latter happens more often, the
default value is True.
:return: ``None``
:raises: :exc:`~openstack.exceptions.NotSupported` if the server
does not support the VIF API.
@ -480,12 +484,13 @@ class Node(_common.ListMixin, resource.Resource):
request = self._prepare_request(requires_id=True)
request.url = utils.urljoin(request.url, 'vifs')
body = {'id': vif_id}
retriable_status_codes = _common.RETRIABLE_STATUS_CODES
if not retry_on_conflict:
retriable_status_codes = set(retriable_status_codes) - {409}
response = session.post(
request.url, json=body,
headers=request.headers, microversion=version,
# NOTE(dtantsur): do not retry CONFLICT, it's a valid status code
# in this API when the VIF is already attached to another node.
retriable_status_codes=[503])
retriable_status_codes=retriable_status_codes)
msg = ("Failed to attach VIF {vif} to bare metal node {node}"
.format(node=self.id, vif=vif_id))

View File

@ -342,7 +342,15 @@ class TestNodeVif(base.TestCase):
self.session.post.assert_called_once_with(
'nodes/%s/vifs' % self.node.id, json={'id': self.vif_id},
headers=mock.ANY, microversion='1.28',
retriable_status_codes=[503])
retriable_status_codes=[409, 503])
def test_attach_vif_no_retries(self):
self.assertIsNone(self.node.attach_vif(self.session, self.vif_id,
retry_on_conflict=False))
self.session.post.assert_called_once_with(
'nodes/%s/vifs' % self.node.id, json={'id': self.vif_id},
headers=mock.ANY, microversion='1.28',
retriable_status_codes={503})
def test_detach_vif_existing(self):
self.assertTrue(self.node.detach_vif(self.session, self.vif_id))

View File

@ -0,0 +1,8 @@
---
fixes:
- |
Changes the ``baremetal.attach_vif_to_node`` call to retry HTTP CONFLICT
by default. While it's a valid error code when a VIF is already attached
to a node, the same code is also used when the target node is locked.
The latter happens more often, so the retries are now on by default and
can be disabled by setting ``retry_on_conflict`` to ``False``.