Make `redfish_system_id` property optional

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`.

Also bumpted sushy dependency to >= 3.1.0.

Change-Id: I425baa7c7294c6c8a707e89df63a17da8e49b666
Story: 2007258
Task: 38619

5 files changed, 56 insertions, 24 deletions

diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst
index 9c20d6e1b..925d8dd50 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 c9874f12f..6df2f622f 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 fbf0a3b55..758323f71 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 '<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})
diff --git a/ironic/tests/unit/drivers/modules/redfish/test_utils.py b/ironic/tests/unit/drivers/modules/redfish/test_utils.py
index ef56d96c7..ae40d0e40 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 000000000..094ec18b8
--- /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.
+