Merge "Make `redfish_system_id` property optional"
This commit is contained in:
Zuul
Gerrit Code Review
commit
d0361c1c2d
|
|
@ -56,11 +56,14 @@ field:
|
|||
missing, https is assumed.
|
||||
For example: https://mgmt.vendor.com. This is required.
|
||||
|
||||
- ``redfish_system_id``: The canonical path to the System resource that
|
||||
the driver will interact with. It should include
|
||||
the root service, version and the unique
|
||||
resource path to the System. For example:
|
||||
/redfish/v1/Systems/1. This is required.
|
||||
- ``redfish_system_id``: The canonical path to the ComputerSystem resource
|
||||
that the driver will interact with. It should include
|
||||
the root service, version and the unique resource
|
||||
path to the ComputerSystem. This property is only
|
||||
required if target BMC manages more than one
|
||||
ComputerSystem. Otherwise ironic will pick the only
|
||||
available ComputerSystem automatically. For
|
||||
example: /redfish/v1/Systems/1.
|
||||
|
||||
- ``redfish_username``: User account with admin/server-profile access
|
||||
privilege. Although not required, it is highly
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ python-dracclient>=3.1.0,<4.0.0
|
|||
python-xclarityclient>=0.1.6
|
||||
|
||||
# The Redfish hardware type uses the Sushy library
|
||||
sushy>=2.0.0
|
||||
sushy>=3.1.0
|
||||
|
||||
# Ansible-deploy interface
|
||||
ansible>=2.5
|
||||
|
|
|
|||
|
|
@ -37,16 +37,19 @@ REQUIRED_PROPERTIES = {
|
|||
'must include the authority portion of the URL. '
|
||||
'If the scheme is missing, https is assumed. '
|
||||
'For example: https://mgmt.vendor.com. Required'),
|
||||
}
|
||||
|
||||
OPTIONAL_PROPERTIES = {
|
||||
'redfish_system_id': _('The canonical path to the ComputerSystem '
|
||||
'resource that the driver will interact with. '
|
||||
'It should include the root service, version and '
|
||||
'the unique resource path to a ComputerSystem '
|
||||
'within the same authority as the redfish_address '
|
||||
'property. For example: /redfish/v1/Systems/1. '
|
||||
'Required')
|
||||
}
|
||||
|
||||
OPTIONAL_PROPERTIES = {
|
||||
'This property is only required if target BMC '
|
||||
'manages more than one ComputerSystem. Otherwise '
|
||||
'ironic will pick the only available '
|
||||
'ComputerSystem automatically.'),
|
||||
'redfish_username': _('User account with admin/server-profile access '
|
||||
'privilege. Although this property is not '
|
||||
'mandatory it\'s highly recommended to set a '
|
||||
|
|
@ -109,16 +112,18 @@ def parse_driver_info(node):
|
|||
'driver_info/redfish_address on node %(node)s') %
|
||||
{'address': address, 'node': node.uuid})
|
||||
|
||||
try:
|
||||
system_id = urlparse.quote(driver_info['redfish_system_id'])
|
||||
except (TypeError, AttributeError):
|
||||
raise exception.InvalidParameterValue(
|
||||
_('Invalid value "%(value)s" set in '
|
||||
'driver_info/redfish_system_id on node %(node)s. '
|
||||
'The value should be a path (string) to the resource '
|
||||
'that the driver will interact with. For example: '
|
||||
'/redfish/v1/Systems/1') %
|
||||
{'value': driver_info['redfish_system_id'], 'node': node.uuid})
|
||||
redfish_system_id = driver_info.get('redfish_system_id')
|
||||
if redfish_system_id is not None:
|
||||
try:
|
||||
redfish_system_id = urlparse.quote(redfish_system_id)
|
||||
except (TypeError, AttributeError):
|
||||
raise exception.InvalidParameterValue(
|
||||
_('Invalid value "%(value)s" set in '
|
||||
'driver_info/redfish_system_id on node %(node)s. '
|
||||
'The value should be a path (string) to the resource '
|
||||
'that the driver will interact with. For example: '
|
||||
'/redfish/v1/Systems/1') %
|
||||
{'value': driver_info['redfish_system_id'], 'node': node.uuid})
|
||||
|
||||
# Check if verify_ca is a Boolean or a file/directory in the file-system
|
||||
verify_ca = driver_info.get('redfish_verify_ca', True)
|
||||
|
|
@ -154,7 +159,7 @@ def parse_driver_info(node):
|
|||
{'value': auth_type, 'node': node.uuid})
|
||||
|
||||
return {'address': address,
|
||||
'system_id': system_id,
|
||||
'system_id': redfish_system_id,
|
||||
'username': driver_info.get('redfish_username'),
|
||||
'password': driver_info.get('redfish_password'),
|
||||
'verify_ca': verify_ca,
|
||||
|
|
@ -234,7 +239,7 @@ def get_system(node):
|
|||
:raises: RedfishError if the System is not registered in Redfish
|
||||
"""
|
||||
driver_info = parse_driver_info(node)
|
||||
system_id = driver_info['system_id']
|
||||
system_id = driver_info.get('system_id')
|
||||
|
||||
@retrying.retry(
|
||||
retry_on_exception=(
|
||||
|
|
@ -249,7 +254,8 @@ def get_system(node):
|
|||
except sushy.exceptions.ResourceNotFoundError as e:
|
||||
LOG.error('The Redfish System "%(system)s" was not found for '
|
||||
'node %(node)s. Error %(error)s',
|
||||
{'system': system_id, 'node': node.uuid, 'error': e})
|
||||
{'system': system_id or '<default>',
|
||||
'node': node.uuid, 'error': e})
|
||||
raise exception.RedfishError(error=e)
|
||||
# TODO(lucasagomes): We should look at other types of
|
||||
# ConnectionError such as AuthenticationError or SSLError and stop
|
||||
|
|
@ -259,7 +265,7 @@ def get_system(node):
|
|||
'Redfish at address "%(address)s" using auth type '
|
||||
'"%(auth_type)s" when fetching System "%(system)s". '
|
||||
'Error: %(error)s',
|
||||
{'system': system_id,
|
||||
{'system': system_id or '<default>',
|
||||
'address': driver_info['address'],
|
||||
'auth_type': driver_info['auth_type'],
|
||||
'node': node.uuid, 'error': e})
|
||||
|
|
|
|||
|
|
@ -122,6 +122,10 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
|
|||
'The value should be a path',
|
||||
redfish_utils.parse_driver_info, self.node)
|
||||
|
||||
def test_parse_driver_info_missing_system_id(self):
|
||||
self.node.driver_info.pop('redfish_system_id')
|
||||
redfish_utils.parse_driver_info(self.node)
|
||||
|
||||
def test_parse_driver_info_valid_string_value_verify_ca(self):
|
||||
for value in ('0', 'f', 'false', 'off', 'n', 'no'):
|
||||
self.node.driver_info['redfish_verify_ca'] = value
|
||||
|
|
@ -182,6 +186,15 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
|
|||
fake_conn.get_system.assert_called_once_with(
|
||||
'/redfish/v1/Systems/FAKESYSTEM')
|
||||
|
||||
@mock.patch.object(sushy, 'Sushy', autospec=True)
|
||||
@mock.patch('ironic.drivers.modules.redfish.utils.'
|
||||
'SessionCache._sessions', {})
|
||||
def test_get_system_multiple_systems(self, mock_sushy):
|
||||
self.node.driver_info.pop('redfish_system_id')
|
||||
fake_conn = mock_sushy.return_value
|
||||
redfish_utils.get_system(self.node)
|
||||
fake_conn.get_system.assert_called_once_with(None)
|
||||
|
||||
@mock.patch('time.sleep', autospec=True)
|
||||
@mock.patch.object(sushy, 'Sushy', autospec=True)
|
||||
@mock.patch('ironic.drivers.modules.redfish.utils.'
|
||||
|
|
|
|||
|
|
@ -0,0 +1,10 @@
|
|||
---
|
||||
features:
|
||||
- |
|
||||
The ``redfish_system_id`` property of redfish hardware type has been made
|
||||
optional. If not specified in ``driver_info``, and the target BMC manages
|
||||
a single ComputerSystem, ironic will assume that system. Otherwise, ironic
|
||||
will fail requiring explicit ``redfish_system_id`` specification in
|
||||
``driver_info``. The minimum version for the sushy library is now
|
||||
3.1.0.
|
||||
|
||||
Loading…
Reference in New Issue