Merge "resource: Make _assert_microversion_for a classmethod"
This commit is contained in:
commit
e87f4eea5a
@ -334,7 +334,7 @@ class Node(_common.Resource):
|
||||
)
|
||||
else:
|
||||
error_message = (
|
||||
"Cannot create a node with initial provision "
|
||||
f"Cannot create a node with initial provision "
|
||||
f"state {expected_provision_state}"
|
||||
)
|
||||
# Nodes cannot be created as available using new API versions
|
||||
@ -343,7 +343,6 @@ class Node(_common.Resource):
|
||||
)
|
||||
microversion = self._assert_microversion_for(
|
||||
session,
|
||||
'create',
|
||||
microversion,
|
||||
maximum=maximum,
|
||||
error_message=error_message,
|
||||
@ -452,26 +451,26 @@ class Node(_common.Resource):
|
||||
"""
|
||||
session = self._get_session(session)
|
||||
|
||||
version = None
|
||||
microversion = None
|
||||
if target in _common.PROVISIONING_VERSIONS:
|
||||
version = f'1.{_common.PROVISIONING_VERSIONS[target]}'
|
||||
microversion = f'1.{_common.PROVISIONING_VERSIONS[target]}'
|
||||
|
||||
if config_drive:
|
||||
# Some config drive actions require a higher version.
|
||||
if isinstance(config_drive, dict):
|
||||
version = _common.CONFIG_DRIVE_DICT_VERSION
|
||||
microversion = _common.CONFIG_DRIVE_DICT_VERSION
|
||||
elif target == 'rebuild':
|
||||
version = _common.CONFIG_DRIVE_REBUILD_VERSION
|
||||
microversion = _common.CONFIG_DRIVE_REBUILD_VERSION
|
||||
|
||||
if deploy_steps:
|
||||
version = _common.DEPLOY_STEPS_VERSION
|
||||
microversion = _common.DEPLOY_STEPS_VERSION
|
||||
|
||||
version = self._assert_microversion_for(session, 'commit', version)
|
||||
microversion = self._assert_microversion_for(session, microversion)
|
||||
|
||||
body = {'target': target}
|
||||
if runbook:
|
||||
version = self._assert_microversion_for(
|
||||
session, 'commit', _common.RUNBOOKS_VERSION
|
||||
microversion = self._assert_microversion_for(
|
||||
session, _common.RUNBOOKS_VERSION
|
||||
)
|
||||
|
||||
if clean_steps is not None:
|
||||
@ -546,7 +545,7 @@ class Node(_common.Resource):
|
||||
except KeyError:
|
||||
raise ValueError(
|
||||
f'For target {target} the expected state is not '
|
||||
'known, cannot wait for it'
|
||||
f'known, cannot wait for it'
|
||||
)
|
||||
|
||||
request = self._prepare_request(requires_id=True)
|
||||
@ -555,7 +554,7 @@ class Node(_common.Resource):
|
||||
request.url,
|
||||
json=body,
|
||||
headers=request.headers,
|
||||
microversion=version,
|
||||
microversion=microversion,
|
||||
retriable_status_codes=_common.RETRIABLE_STATUS_CODES,
|
||||
)
|
||||
|
||||
@ -740,9 +739,7 @@ class Node(_common.Resource):
|
||||
"""
|
||||
session = self._get_session(session)
|
||||
version = self._assert_microversion_for(
|
||||
session,
|
||||
'commit',
|
||||
_common.INJECT_NMI_VERSION,
|
||||
session, _common.INJECT_NMI_VERSION
|
||||
)
|
||||
request = self._prepare_request(requires_id=True)
|
||||
request.url = utils.urljoin(request.url, 'management', 'inject_nmi')
|
||||
@ -781,17 +778,17 @@ class Node(_common.Resource):
|
||||
except KeyError:
|
||||
raise ValueError(
|
||||
f"Cannot use target power state {target} with wait, "
|
||||
"the expected state is not known"
|
||||
f"the expected state is not known"
|
||||
)
|
||||
|
||||
session = self._get_session(session)
|
||||
|
||||
if target.startswith("soft "):
|
||||
version = '1.27'
|
||||
microversion = '1.27'
|
||||
else:
|
||||
version = None
|
||||
microversion = None
|
||||
|
||||
version = self._assert_microversion_for(session, 'commit', version)
|
||||
microversion = self._assert_microversion_for(session, microversion)
|
||||
|
||||
# TODO(dtantsur): server timeout support
|
||||
body = {'target': target}
|
||||
@ -802,7 +799,7 @@ class Node(_common.Resource):
|
||||
request.url,
|
||||
json=body,
|
||||
headers=request.headers,
|
||||
microversion=version,
|
||||
microversion=microversion,
|
||||
retriable_status_codes=_common.RETRIABLE_STATUS_CODES,
|
||||
)
|
||||
|
||||
@ -856,13 +853,12 @@ class Node(_common.Resource):
|
||||
|
||||
session = self._get_session(session)
|
||||
if port_id or port_group_id:
|
||||
required_version = _common.VIF_OPTIONAL_PARAMS_VERSION
|
||||
microversion = _common.VIF_OPTIONAL_PARAMS_VERSION
|
||||
else:
|
||||
required_version = _common.VIF_VERSION
|
||||
version = self._assert_microversion_for(
|
||||
microversion = _common.VIF_VERSION
|
||||
microversion = self._assert_microversion_for(
|
||||
session,
|
||||
'commit',
|
||||
required_version,
|
||||
microversion,
|
||||
error_message=("Cannot use VIF attachment API"),
|
||||
)
|
||||
|
||||
@ -880,7 +876,7 @@ class Node(_common.Resource):
|
||||
request.url,
|
||||
json=body,
|
||||
headers=request.headers,
|
||||
microversion=version,
|
||||
microversion=microversion,
|
||||
retriable_status_codes=retriable_status_codes,
|
||||
)
|
||||
|
||||
@ -908,7 +904,6 @@ class Node(_common.Resource):
|
||||
session = self._get_session(session)
|
||||
version = self._assert_microversion_for(
|
||||
session,
|
||||
'commit',
|
||||
_common.VIF_VERSION,
|
||||
error_message=("Cannot use VIF attachment API"),
|
||||
)
|
||||
@ -949,7 +944,6 @@ class Node(_common.Resource):
|
||||
session = self._get_session(session)
|
||||
version = self._assert_microversion_for(
|
||||
session,
|
||||
'fetch',
|
||||
_common.VIF_VERSION,
|
||||
error_message=("Cannot use VIF attachment API"),
|
||||
)
|
||||
@ -1144,7 +1138,7 @@ class Node(_common.Resource):
|
||||
if target not in ('uefi', 'bios'):
|
||||
raise ValueError(
|
||||
f"Unrecognized boot mode {target}."
|
||||
"Boot mode should be one of 'uefi' or 'bios'."
|
||||
f"Boot mode should be one of 'uefi' or 'bios'."
|
||||
)
|
||||
body = {'target': target}
|
||||
|
||||
@ -1180,7 +1174,7 @@ class Node(_common.Resource):
|
||||
if not isinstance(target, bool):
|
||||
raise ValueError(
|
||||
f"Invalid target {target}. It should be True or False "
|
||||
"corresponding to secure boot state 'on' or 'off'"
|
||||
f"corresponding to secure boot state 'on' or 'off'"
|
||||
)
|
||||
body = {'target': target}
|
||||
|
||||
@ -1384,7 +1378,7 @@ class Node(_common.Resource):
|
||||
if not isinstance(enabled, bool):
|
||||
raise ValueError(
|
||||
f"Invalid enabled {enabled}. It should be True or False "
|
||||
"corresponding to console enabled or disabled"
|
||||
f"corresponding to console enabled or disabled"
|
||||
)
|
||||
body = {'enabled': enabled}
|
||||
|
||||
@ -1437,7 +1431,6 @@ class Node(_common.Resource):
|
||||
session = self._get_session(session)
|
||||
version = self._assert_microversion_for(
|
||||
session,
|
||||
'fetch',
|
||||
_common.FIRMWARE_VERSION,
|
||||
error_message=("Cannot use node list firmware components API"),
|
||||
)
|
||||
@ -1482,7 +1475,7 @@ class Node(_common.Resource):
|
||||
|
||||
session = self._get_session(session)
|
||||
microversion = self._assert_microversion_for(
|
||||
session, 'commit', _common.RESET_INTERFACES_VERSION
|
||||
session, _common.RESET_INTERFACES_VERSION
|
||||
)
|
||||
params = [('reset_interfaces', reset_interfaces)]
|
||||
|
||||
|
@ -1238,8 +1238,9 @@ class Resource(dict):
|
||||
"a raw keystoneauth1.adapter.Adapter."
|
||||
)
|
||||
|
||||
# TODO(stephenfin): Drop action argument. It has never been used.
|
||||
@classmethod
|
||||
def _get_microversion(cls, session, *, action):
|
||||
def _get_microversion(cls, session, *, action=None):
|
||||
"""Get microversion to use for the given action.
|
||||
|
||||
The base version uses the following logic:
|
||||
@ -1265,6 +1266,7 @@ class Resource(dict):
|
||||
'create',
|
||||
'delete',
|
||||
'patch',
|
||||
None,
|
||||
}:
|
||||
raise ValueError(f'Invalid action: {action}')
|
||||
|
||||
@ -1275,18 +1277,18 @@ class Resource(dict):
|
||||
session, cls._max_microversion
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def _assert_microversion_for(
|
||||
self,
|
||||
session,
|
||||
action,
|
||||
expected,
|
||||
error_message=None,
|
||||
maximum=None,
|
||||
):
|
||||
cls,
|
||||
session: adapter.Adapter,
|
||||
expected: ty.Optional[str],
|
||||
*,
|
||||
error_message: ty.Optional[str] = None,
|
||||
maximum: ty.Optional[str] = None,
|
||||
) -> str:
|
||||
"""Enforce that the microversion for action satisfies the requirement.
|
||||
|
||||
:param session: :class`keystoneauth1.adapter.Adapter`
|
||||
:param action: One of "fetch", "commit", "create", "delete".
|
||||
:param expected: Expected microversion.
|
||||
:param error_message: Optional error message with details. Will be
|
||||
prepended to the message generated here.
|
||||
@ -1296,21 +1298,22 @@ class Resource(dict):
|
||||
used for the action is lower than the expected one.
|
||||
"""
|
||||
|
||||
def _raise(message):
|
||||
def _raise(message: str) -> ty.NoReturn:
|
||||
if error_message:
|
||||
error_message.rstrip('.')
|
||||
message = f'{error_message}. {message}'
|
||||
|
||||
raise exceptions.NotSupported(message)
|
||||
|
||||
actual = self._get_microversion(session, action=action)
|
||||
actual = cls._get_microversion(session)
|
||||
|
||||
if actual is None:
|
||||
message = (
|
||||
f"API version {expected} is required, but the default "
|
||||
"version will be used."
|
||||
f"version will be used."
|
||||
)
|
||||
_raise(message)
|
||||
|
||||
actual_n = discover.normalize_version_number(actual)
|
||||
|
||||
if expected is not None:
|
||||
@ -1318,14 +1321,15 @@ class Resource(dict):
|
||||
if actual_n < expected_n:
|
||||
message = (
|
||||
f"API version {expected} is required, but {actual} "
|
||||
"will be used."
|
||||
f"will be used."
|
||||
)
|
||||
_raise(message)
|
||||
|
||||
if maximum is not None:
|
||||
maximum_n = discover.normalize_version_number(maximum)
|
||||
# Assume that if a service supports higher versions, it also
|
||||
# supports lower ones. Breaks for services that remove old API
|
||||
# versions (which is not something they should do).
|
||||
# versions (which is not something that has been done yet).
|
||||
if actual_n > maximum_n:
|
||||
return maximum
|
||||
|
||||
|
@ -102,7 +102,7 @@ FAKE = {
|
||||
}
|
||||
|
||||
|
||||
def _fake_assert(self, session, action, expected, error_message=None):
|
||||
def _fake_assert(self, session, expected, error_message=None):
|
||||
return expected
|
||||
|
||||
|
||||
|
@ -3631,9 +3631,9 @@ class TestAssertMicroversionFor(base.TestCase):
|
||||
|
||||
self.assertEqual(
|
||||
'1.42',
|
||||
self.res._assert_microversion_for(self.session, 'fetch', '1.6'),
|
||||
self.res._assert_microversion_for(self.session, '1.6'),
|
||||
)
|
||||
mock_get_ver.assert_called_once_with(self.session, action='fetch')
|
||||
mock_get_ver.assert_called_once_with(self.session)
|
||||
|
||||
def test_incompatible(self, mock_get_ver):
|
||||
mock_get_ver.return_value = '1.1'
|
||||
@ -3643,10 +3643,9 @@ class TestAssertMicroversionFor(base.TestCase):
|
||||
'1.6 is required, but 1.1 will be used',
|
||||
self.res._assert_microversion_for,
|
||||
self.session,
|
||||
'fetch',
|
||||
'1.6',
|
||||
)
|
||||
mock_get_ver.assert_called_once_with(self.session, action='fetch')
|
||||
mock_get_ver.assert_called_once_with(self.session)
|
||||
|
||||
def test_custom_message(self, mock_get_ver):
|
||||
mock_get_ver.return_value = '1.1'
|
||||
@ -3656,11 +3655,10 @@ class TestAssertMicroversionFor(base.TestCase):
|
||||
'boom.*1.6 is required, but 1.1 will be used',
|
||||
self.res._assert_microversion_for,
|
||||
self.session,
|
||||
'fetch',
|
||||
'1.6',
|
||||
error_message='boom',
|
||||
)
|
||||
mock_get_ver.assert_called_once_with(self.session, action='fetch')
|
||||
mock_get_ver.assert_called_once_with(self.session)
|
||||
|
||||
def test_none(self, mock_get_ver):
|
||||
mock_get_ver.return_value = None
|
||||
@ -3670,7 +3668,6 @@ class TestAssertMicroversionFor(base.TestCase):
|
||||
'1.6 is required, but the default version',
|
||||
self.res._assert_microversion_for,
|
||||
self.session,
|
||||
'fetch',
|
||||
'1.6',
|
||||
)
|
||||
mock_get_ver.assert_called_once_with(self.session, action='fetch')
|
||||
mock_get_ver.assert_called_once_with(self.session)
|
||||
|
Loading…
x
Reference in New Issue
Block a user