diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst index 9c20d6e1be..925d8dd501 100644 --- a/doc/source/admin/drivers/redfish.rst +++ b/doc/source/admin/drivers/redfish.rst @@ -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 diff --git a/driver-requirements.txt b/driver-requirements.txt index c9874f12f9..6df2f622f2 100644 --- a/driver-requirements.txt +++ b/driver-requirements.txt @@ -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 diff --git a/ironic/drivers/modules/redfish/utils.py b/ironic/drivers/modules/redfish/utils.py index fbf0a3b559..758323f71e 100644 --- a/ironic/drivers/modules/redfish/utils.py +++ b/ironic/drivers/modules/redfish/utils.py @@ -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 '', + '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 '', 'address': driver_info['address'], 'auth_type': driver_info['auth_type'], 'node': node.uuid, 'error': e}) diff --git a/ironic/tests/unit/drivers/modules/redfish/test_utils.py b/ironic/tests/unit/drivers/modules/redfish/test_utils.py index ef56d96c77..ae40d0e40a 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_utils.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_utils.py @@ -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.' diff --git a/releasenotes/notes/optional-redfish-system-id-3f6e8b0ac989cb9b.yaml b/releasenotes/notes/optional-redfish-system-id-3f6e8b0ac989cb9b.yaml new file mode 100644 index 0000000000..094ec18b89 --- /dev/null +++ b/releasenotes/notes/optional-redfish-system-id-3f6e8b0ac989cb9b.yaml @@ -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. +