Add Targets to firmware.update on multi system BMCs

Currently, Ironic doesn't add Targets parameter to SimpleUpdate call
when updating firmware. This patch makes Ironic aware of multi-System
BMCs and send Targets parameter if this condition is detected. This is a
prerequisite for using sushy-tools simulated firmware upgrades for
testing.

Change-Id: I5fd0228200afc28b24d90595244d3961b05acc52

ironic/drivers/modules/redfish/firmware.py

@@ -191,6 +191,26 @@ class RedfishFirmware(base.FirmwareInterface):
             to be executed.
         """
         fw_upd = settings[0]
+        # NOTE(janders) try to get the collection of Systems on the BMC
+        # to determine if there may be more than one System
+        try:
+            systems_collection = redfish_utils.get_system_collection(node)
+        except exception.RedfishError as e:
+            LOG.error('Failed getting Redfish Systems Collection'
+                      ' for node %(node)s. Error %(error)s',
+                      {'node': node.uuid, 'error': e})
+            raise exception.RedfishError(error=e)
+        count = len(systems_collection.members_identities)
+        # NOTE(janders) if we see more than one System on the BMC, assume that
+        # we need to explicitly specify Target parameter when calling
+        # SimpleUpdate. This is needed for compatibility with sushy-tools
+        # in automated testing using VMs.
+        if count > 1:
+            target = node.driver_info.get('redfish_system_id')
+            targets = [target]
+        else:
+            targets = None
+
         component_url, cleanup = self._stage_firmware_file(node, fw_upd)
 
         LOG.debug('Applying new firmware %(url)s for %(component)s on node '
@@ -198,7 +218,11 @@ class RedfishFirmware(base.FirmwareInterface):
                   {'url': fw_upd['url'], 'component': fw_upd['component'],
                    'node_uuid': node.uuid})
         try:
-            task_monitor = update_service.simple_update(component_url)
+            if targets is not None:
+                task_monitor = update_service.simple_update(component_url,
+                                                            targets=targets)
+            else:
+                task_monitor = update_service.simple_update(component_url)
         except sushy.exceptions.MissingAttributeError as e:
             LOG.error('The attribute #UpdateService.SimpleUpdate is missing '
                       'on node %(node)s. Error: %(error)s',

ironic/drivers/modules/redfish/utils.py

@@ -345,6 +345,29 @@ def get_system(node):
         raise exception.RedfishError(error=e)
 
 
+def get_system_collection(node):
+    """Get a Redfish System Collection that includes the node
+
+    :param node: an Ironic node object
+    :raises: RedfishConnectionError when it fails to connect to Redfish
+    :raises: RedfishError if the System is not registered in Redfish
+    """
+    driver_info = parse_driver_info(node)
+    system_id = driver_info['system_id']
+
+    try:
+        return _get_connection(
+            node,
+            lambda conn, system_id: conn.get_system_collection(),
+            system_id)
+    except sushy.exceptions.ResourceNotFoundError as e:
+        LOG.error('The Redfish Systems Collection "%(system)s" was not found'
+                  ' for node %(node)s. Error %(error)s',
+                  {'system': system_id or '<default>',
+                   'node': node.uuid, 'error': e})
+        raise exception.RedfishError(error=e)
+
+
 def get_task_monitor(node, uri):
     """Get a TaskMonitor for a node.
 

ironic/tests/json_samples/systems_collection_dual.json

@@ -0,0 +1,19 @@
+{
+    "@odata.type": "#ComputerSystemCollection.ComputerSystemCollection",
+    "Name": "Computer System Collection",
+    "Members@odata.count": 2,
+    "Members": [
+
+            {
+                "@odata.id": "/redfish/v1/Systems/a30c1a96-0c75-498d-b929-f6da8decb736"
+            },
+
+            {
+                "@odata.id": "/redfish/v1/Systems/ea72b191-ef72-4504-9aac-90e936c9dfd9"
+            }
+
+    ],
+    "@odata.context": "/redfish/v1/$metadata#ComputerSystemCollection.ComputerSystemCollection",
+    "@odata.id": "/redfish/v1/Systems",
+    "@Redfish.Copyright": "Copyright 2014-2016 Distributed Management Task Force, Inc. (DMTF). For the full DMTF copyright policy, see http://www.dmtf.org/about/policies/copyright."
+}

ironic/tests/json_samples/systems_collection_single.json

@@ -0,0 +1,15 @@
+{
+    "@odata.type": "#ComputerSystemCollection.ComputerSystemCollection",
+    "Name": "Computer System Collection",
+    "Members@odata.count": 1,
+    "Members": [
+
+            {
+                "@odata.id": "/redfish/v1/Systems/ea72b191-ef72-4504-9aac-90e936c9dfd9"
+            }
+
+    ],
+    "@odata.context": "/redfish/v1/$metadata#ComputerSystemCollection.ComputerSystemCollection",
+    "@odata.id": "/redfish/v1/Systems",
+    "@Redfish.Copyright": "Copyright 2014-2016 Distributed Management Task Force, Inc. (DMTF). For the full DMTF copyright policy, see http://www.dmtf.org/about/policies/copyright."
+}

ironic/tests/unit/drivers/modules/redfish/test_firmware.py

@@ -12,6 +12,7 @@
 #    under the License.
 
 import datetime
+import json
 from unittest import mock
 
 from oslo_utils import timeutils
@@ -228,7 +229,9 @@ class RedfishFirmwareTestCase(db_base.DbTestCase):
 
     @mock.patch.object(redfish_fw, 'LOG', autospec=True)
     @mock.patch.object(redfish_utils, 'get_update_service', autospec=True)
-    def test_missing_simple_update_action(self, update_service_mock, log_mock):
+    @mock.patch.object(redfish_utils, 'get_system_collection', autospec=True)
+    def test_missing_simple_update_action(self, get_systems_collection_mock,
+                                          update_service_mock, log_mock):
         settings = [{'component': 'bmc', 'url': 'http://upfwbmc/v2.0.0'}]
         update_service = update_service_mock.return_value
         update_service.simple_update.side_effect = \
@@ -663,7 +666,9 @@ class RedfishFirmwareTestCase(db_base.DbTestCase):
 
     @mock.patch.object(redfish_fw, 'LOG', autospec=True)
     @mock.patch.object(manager_utils, 'node_power_action', autospec=True)
-    def test_continue_updates_more_updates(self, node_power_action_mock,
+    @mock.patch.object(redfish_utils, 'get_system_collection', autospec=True)
+    def test_continue_updates_more_updates(self, get_system_collection_mock,
+                                           node_power_action_mock,
                                            log_mock):
         self._generate_new_driver_internal_info(['bmc', 'bios'])
 
@@ -696,3 +701,55 @@ class RedfishFirmwareTestCase(db_base.DbTestCase):
                 'https://bios/v1.0.1')
             task.node.save.assert_called_once_with()
             node_power_action_mock.assert_called_once_with(task, states.REBOOT)
+
+    @mock.patch.object(redfish_utils, 'get_system', autospec=True)
+    @mock.patch.object(redfish_utils, 'get_system_collection', autospec=True)
+    def test__execute_firmware_update_no_targets(self,
+                                                 get_system_collection_mock,
+                                                 system_mock):
+        self._generate_new_driver_internal_info(['bios'])
+        with open('ironic/tests/json_samples/'
+                  'systems_collection_single.json') as f:
+            response_obj = json.load(f)
+        system_collection_mock = mock.MagicMock()
+        system_collection_mock.get_members.return_value = response_obj[
+            'Members']
+        get_system_collection_mock.return_value = system_collection_mock
+
+        task_monitor_mock = mock.Mock()
+        task_monitor_mock.task_monitor_uri = '/task/2'
+        update_service_mock = mock.Mock()
+        update_service_mock.simple_update.return_value = task_monitor_mock
+        firmware = redfish_fw.RedfishFirmware()
+
+        settings = [{'component': 'bios', 'url': 'https://bios/v1.0.1'}]
+        firmware._execute_firmware_update(self.node, update_service_mock,
+                                          settings)
+        update_service_mock.simple_update.assert_called_once_with(
+            'https://bios/v1.0.1')
+
+    @mock.patch.object(redfish_utils, 'get_system', autospec=True)
+    @mock.patch.object(redfish_utils, 'get_system_collection', autospec=True)
+    def test__execute_firmware_update_targets(self,
+                                              get_system_collection_mock,
+                                              system_mock):
+        self._generate_new_driver_internal_info(['bios'])
+        with open('ironic/tests/json_samples/'
+                  'systems_collection_dual.json') as f:
+            response_obj = json.load(f)
+        system_collection_mock = mock.MagicMock()
+        system_collection_mock.members_identities = response_obj[
+            'Members']
+        get_system_collection_mock.return_value = system_collection_mock
+
+        task_monitor_mock = mock.Mock()
+        task_monitor_mock.task_monitor_uri = '/task/2'
+        update_service_mock = mock.Mock()
+        update_service_mock.simple_update.return_value = task_monitor_mock
+        firmware = redfish_fw.RedfishFirmware()
+
+        settings = [{'component': 'bios', 'url': 'https://bios/v1.0.1'}]
+        firmware._execute_firmware_update(self.node, update_service_mock,
+                                          settings)
+        update_service_mock.simple_update.assert_called_once_with(
+            'https://bios/v1.0.1', targets=[mock.ANY])

ironic/tests/unit/drivers/modules/redfish/test_utils.py

@@ -227,6 +227,25 @@ class RedfishUtilsTestCase(db_base.DbTestCase):
         self.assertRaises(exception.RedfishError,
                           redfish_utils.get_event_service, self.node)
 
+    def test_get_system_collection(self):
+        redfish_utils._get_connection = mock.Mock()
+        mock_system_collection = mock.Mock()
+        redfish_utils._get_connection.return_value = mock_system_collection
+
+        result = redfish_utils.get_system_collection(self.node)
+
+        self.assertEqual(mock_system_collection, result)
+
+    def test_get_system_collection_error(self):
+        redfish_utils._get_connection = mock.Mock()
+        redfish_utils._get_connection.side_effect =\
+            sushy.exceptions.ResourceNotFoundError('GET',
+                                                   '/',
+                                                   requests.Response())
+
+        self.assertRaises(exception.RedfishError,
+                          redfish_utils.get_system_collection, self.node)
+
 
 class RedfishUtilsAuthTestCase(db_base.DbTestCase):
 

releasenotes/notes/add-support-for-simpleupdate-targets-1ac970f4ff458981.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    Adds support for updating BIOS in configurations where a single BMC is
+    managing multiple systems (e.g. sushy-tools emulator with multiple VMs).
+    In such cases, Targets parameter is added to SimpleUpdate API call.