Merge "Use retries provided by ironicclient instead of ad-hoc ones"
This commit is contained in:
commit
b383c87dfc
@ -260,6 +260,14 @@
|
||||
# Ironic endpoint type. (string value)
|
||||
#os_endpoint_type = internalURL
|
||||
|
||||
# Interval between retries in case of conflict error (HTTP 409).
|
||||
# (integer value)
|
||||
#retry_interval = 2
|
||||
|
||||
# Maximum number of retries in case of conflict error (HTTP 409).
|
||||
# (integer value)
|
||||
#max_retries = 30
|
||||
|
||||
|
||||
[keystone_authtoken]
|
||||
|
||||
|
@ -65,6 +65,14 @@ IRONIC_OPTS = [
|
||||
cfg.StrOpt('os_endpoint_type',
|
||||
default='internalURL',
|
||||
help='Ironic endpoint type.'),
|
||||
cfg.IntOpt('retry_interval',
|
||||
default=2,
|
||||
help='Interval between retries in case of conflict error '
|
||||
'(HTTP 409).'),
|
||||
cfg.IntOpt('max_retries',
|
||||
default=30,
|
||||
help='Maximum number of retries in case of conflict error '
|
||||
'(HTTP 409).'),
|
||||
]
|
||||
|
||||
|
||||
|
@ -95,7 +95,7 @@ def introspect(uuid, new_ipmi_credentials=None, token=None):
|
||||
new_ipmi_credentials = (
|
||||
_validate_ipmi_credentials(node, new_ipmi_credentials))
|
||||
else:
|
||||
validation = utils.retry_on_conflict(ironic.node.validate, node.uuid)
|
||||
validation = ironic.node.validate(node.uuid)
|
||||
if not validation.power['result']:
|
||||
msg = _('Failed validation of power interface for node %(node)s, '
|
||||
'reason: %(reason)s')
|
||||
@ -133,8 +133,8 @@ def _background_introspect(ironic, node_info):
|
||||
|
||||
if not node_info.options.get('new_ipmi_credentials'):
|
||||
try:
|
||||
utils.retry_on_conflict(ironic.node.set_boot_device,
|
||||
node_info.uuid, 'pxe', persistent=False)
|
||||
ironic.node.set_boot_device(node_info.uuid, 'pxe',
|
||||
persistent=False)
|
||||
except Exception as exc:
|
||||
LOG.warning(_LW('Failed to set boot device to PXE for'
|
||||
' node %(node)s: %(exc)s') %
|
||||
@ -152,8 +152,7 @@ def _background_introspect(ironic, node_info):
|
||||
_LAST_INTROSPECTION_TIME = time.time()
|
||||
|
||||
try:
|
||||
utils.retry_on_conflict(ironic.node.set_power_state,
|
||||
node_info.uuid, 'reboot')
|
||||
ironic.node.set_power_state(node_info.uuid, 'reboot')
|
||||
except Exception as exc:
|
||||
raise utils.Error(_('Failed to power on node %(node)s,'
|
||||
' check it\'s power '
|
||||
|
@ -143,10 +143,10 @@ def _process_node(ironic, node, introspection_data, node_info):
|
||||
node_patches, port_patches = _run_post_hooks(node_info,
|
||||
introspection_data)
|
||||
|
||||
node = utils.retry_on_conflict(ironic.node.update, node.uuid, node_patches)
|
||||
node = ironic.node.update(node.uuid, node_patches)
|
||||
for mac, patches in port_patches.items():
|
||||
port = node_info.ports(ironic)[mac]
|
||||
utils.retry_on_conflict(ironic.port.update, port.uuid, patches)
|
||||
ironic.port.update(port.uuid, patches)
|
||||
|
||||
LOG.debug('Node %s was updated with data from introspection process, '
|
||||
'patches %s, port patches %s',
|
||||
@ -181,7 +181,7 @@ def _finish_set_ipmi_credentials(ironic, node, node_info, introspection_data,
|
||||
introspection_data.get('ipmi_address')):
|
||||
patch.append({'op': 'add', 'path': '/driver_info/ipmi_address',
|
||||
'value': introspection_data['ipmi_address']})
|
||||
utils.retry_on_conflict(ironic.node.update, node_info.uuid, patch)
|
||||
ironic.node.update(node_info.uuid, patch)
|
||||
|
||||
for attempt in range(_CREDENTIALS_WAIT_RETRIES):
|
||||
try:
|
||||
@ -207,8 +207,7 @@ def _finish_set_ipmi_credentials(ironic, node, node_info, introspection_data,
|
||||
def _finish(ironic, node_info):
|
||||
LOG.debug('Forcing power off of node %s', node_info.uuid)
|
||||
try:
|
||||
utils.retry_on_conflict(ironic.node.set_power_state,
|
||||
node_info.uuid, 'off')
|
||||
ironic.node.set_power_state(node_info.uuid, 'off')
|
||||
except Exception as exc:
|
||||
msg = (_('Failed to power off node %(node)s, check it\'s power '
|
||||
'management configuration: %(exc)s') %
|
||||
|
@ -95,31 +95,6 @@ class TestIntrospect(BaseTest):
|
||||
add_mock.assert_called_with(self.uuid,
|
||||
bmc_address=self.bmc_address)
|
||||
|
||||
def test_retries(self, client_mock, add_mock, filters_mock):
|
||||
cli = self._prepare(client_mock)
|
||||
cli.node.validate.side_effect = [exceptions.Conflict,
|
||||
mock.Mock(power={'result': True})]
|
||||
cli.node.set_boot_device.side_effect = [exceptions.Conflict,
|
||||
None]
|
||||
cli.node.set_power_state.side_effect = [exceptions.Conflict,
|
||||
None]
|
||||
add_mock.return_value = self.node_info
|
||||
|
||||
introspect.introspect(self.node.uuid)
|
||||
|
||||
cli.node.get.assert_called_once_with(self.uuid)
|
||||
cli.node.validate.assert_called_with(self.uuid)
|
||||
self.node_info.ports.assert_called_once_with(cli)
|
||||
|
||||
add_mock.assert_called_once_with(self.uuid,
|
||||
bmc_address=self.bmc_address)
|
||||
filters_mock.assert_called_with(cli)
|
||||
cli.node.set_boot_device.assert_called_with(self.uuid,
|
||||
'pxe',
|
||||
persistent=False)
|
||||
cli.node.set_power_state.assert_called_with(self.uuid,
|
||||
'reboot')
|
||||
|
||||
def test_power_failure(self, client_mock, add_mock, filters_mock):
|
||||
cli = self._prepare(client_mock)
|
||||
cli.node.set_boot_device.side_effect = exceptions.BadRequest()
|
||||
|
@ -443,34 +443,6 @@ class TestProcessNode(BaseTest):
|
||||
|
||||
self.cli.node.update.assert_called_once_with(self.uuid, patch)
|
||||
|
||||
def test_update_retry_on_conflict(self, filters_mock, post_hook_mock):
|
||||
self.cli.node.update.side_effect = [exceptions.Conflict, self.node,
|
||||
exceptions.Conflict, self.node]
|
||||
|
||||
self.call()
|
||||
|
||||
self.cli.port.create.assert_any_call(node_uuid=self.uuid,
|
||||
address=self.macs[0])
|
||||
self.cli.port.create.assert_any_call(node_uuid=self.uuid,
|
||||
address=self.macs[1])
|
||||
self.cli.node.update.assert_called_with(self.uuid, self.patch_props)
|
||||
self.assertEqual(2, self.cli.node.update.call_count)
|
||||
self.cli.node.set_power_state.assert_called_once_with(self.uuid, 'off')
|
||||
|
||||
def test_power_off_retry_on_conflict(self, filters_mock, post_hook_mock):
|
||||
self.cli.node.set_power_state.side_effect = [exceptions.Conflict, None]
|
||||
|
||||
self.call()
|
||||
|
||||
self.cli.port.create.assert_any_call(node_uuid=self.uuid,
|
||||
address=self.macs[0])
|
||||
self.cli.port.create.assert_any_call(node_uuid=self.uuid,
|
||||
address=self.macs[1])
|
||||
self.cli.node.update.assert_called_once_with(self.uuid,
|
||||
self.patch_props)
|
||||
self.cli.node.set_power_state.assert_called_with(self.uuid, 'off')
|
||||
self.assertEqual(2, self.cli.node.set_power_state.call_count)
|
||||
|
||||
def test_port_failed(self, filters_mock, post_hook_mock):
|
||||
self.cli.port.create.side_effect = (
|
||||
[exceptions.Conflict()] + self.ports[1:])
|
||||
|
@ -13,9 +13,7 @@
|
||||
|
||||
import unittest
|
||||
|
||||
import eventlet
|
||||
from ironicclient import client
|
||||
from ironicclient import exceptions
|
||||
import keystoneclient.v2_0.client as keystone_client
|
||||
from keystonemiddleware import auth_token
|
||||
from oslo_config import cfg
|
||||
@ -47,7 +45,9 @@ class TestCheckAuth(base.BaseTest):
|
||||
utils.get_client(fake_token)
|
||||
args = {'os_auth_token': fake_token,
|
||||
'ironic_url': fake_ironic_url,
|
||||
'os_ironic_api_version': '1.6'}
|
||||
'os_ironic_api_version': '1.6',
|
||||
'max_retries': CONF.ironic.max_retries,
|
||||
'retry_interval': CONF.ironic.retry_interval}
|
||||
mock_client.assert_called_once_with(1, **args)
|
||||
|
||||
@mock.patch.object(client, 'get_client')
|
||||
@ -59,7 +59,9 @@ class TestCheckAuth(base.BaseTest):
|
||||
'os_tenant_name': CONF.ironic.os_tenant_name,
|
||||
'os_endpoint_type': CONF.ironic.os_endpoint_type,
|
||||
'os_service_type': CONF.ironic.os_service_type,
|
||||
'os_ironic_api_version': '1.6'}
|
||||
'os_ironic_api_version': '1.6',
|
||||
'max_retries': CONF.ironic.max_retries,
|
||||
'retry_interval': CONF.ironic.retry_interval}
|
||||
mock_client.assert_called_once_with(1, **args)
|
||||
|
||||
@mock.patch.object(auth_token, 'AuthProtocol')
|
||||
@ -159,27 +161,6 @@ class TestGetIpmiAddress(base.BaseTest):
|
||||
self.assertRaises(utils.Error, utils.get_ipmi_address, node)
|
||||
|
||||
|
||||
@mock.patch.object(eventlet.greenthread, 'sleep', lambda _: None)
|
||||
class TestRetryOnConflict(unittest.TestCase):
|
||||
def test_retry_on_conflict(self):
|
||||
call = mock.Mock()
|
||||
call.side_effect = ([exceptions.Conflict()] * (utils.RETRY_COUNT - 1)
|
||||
+ [mock.sentinel.result])
|
||||
res = utils.retry_on_conflict(call, 1, 2, x=3)
|
||||
self.assertEqual(mock.sentinel.result, res)
|
||||
call.assert_called_with(1, 2, x=3)
|
||||
self.assertEqual(utils.RETRY_COUNT, call.call_count)
|
||||
|
||||
def test_retry_on_conflict_fail(self):
|
||||
call = mock.Mock()
|
||||
call.side_effect = ([exceptions.Conflict()] * (utils.RETRY_COUNT + 1)
|
||||
+ [mock.sentinel.result])
|
||||
self.assertRaises(exceptions.Conflict, utils.retry_on_conflict,
|
||||
call, 1, 2, x=3)
|
||||
call.assert_called_with(1, 2, x=3)
|
||||
self.assertEqual(utils.RETRY_COUNT, call.call_count)
|
||||
|
||||
|
||||
class TestCapabilities(unittest.TestCase):
|
||||
|
||||
def test_capabilities_to_dict(self):
|
||||
|
@ -17,13 +17,12 @@ import socket
|
||||
|
||||
import eventlet
|
||||
from ironicclient import client
|
||||
from ironicclient import exceptions
|
||||
import keystoneclient.v2_0.client as keystone_client
|
||||
from keystonemiddleware import auth_token
|
||||
from oslo_config import cfg
|
||||
import six
|
||||
|
||||
from ironic_inspector.common.i18n import _, _LE, _LI, _LW
|
||||
from ironic_inspector.common.i18n import _, _LE, _LI
|
||||
|
||||
CONF = cfg.CONF
|
||||
|
||||
@ -32,8 +31,6 @@ VALID_STATES = {'enroll', 'manageable', 'inspecting', 'inspectfail'}
|
||||
|
||||
|
||||
LOG = logging.getLogger('ironic_inspector.utils')
|
||||
RETRY_COUNT = 12
|
||||
RETRY_DELAY = 5
|
||||
|
||||
GREEN_POOL = None
|
||||
|
||||
@ -90,6 +87,8 @@ def get_client(token=None,
|
||||
args = {'os_auth_token': token,
|
||||
'ironic_url': ironic_url}
|
||||
args['os_ironic_api_version'] = api_version
|
||||
args['max_retries'] = CONF.ironic.max_retries
|
||||
args['retry_interval'] = CONF.ironic.retry_interval
|
||||
return client.get_client(1, **args)
|
||||
|
||||
|
||||
@ -168,24 +167,6 @@ def get_ipmi_address(node):
|
||||
raise Error(msg % (value, node.uuid))
|
||||
|
||||
|
||||
def retry_on_conflict(call, *args, **kwargs):
|
||||
"""Wrapper to retry 409 CONFLICT exceptions."""
|
||||
for i in range(RETRY_COUNT):
|
||||
try:
|
||||
return call(*args, **kwargs)
|
||||
except exceptions.Conflict as exc:
|
||||
LOG.warning(_LW('Conflict on calling %(call)s: %(exc)s,'
|
||||
' retry attempt %(count)d') %
|
||||
{'call': getattr(call, '__name__', repr(call)),
|
||||
'exc': exc,
|
||||
'count': i + 1})
|
||||
if i == RETRY_COUNT - 1:
|
||||
raise
|
||||
eventlet.greenthread.sleep(RETRY_DELAY)
|
||||
|
||||
raise RuntimeError('unreachable code') # pragma: no cover
|
||||
|
||||
|
||||
def check_provision_state(node):
|
||||
if not node.maintenance:
|
||||
provision_state = node.provision_state
|
||||
|
Loading…
x
Reference in New Issue
Block a user