Fix missing ETag when patching Redfish resource

Some Redfish implementations require current ETag of Redfish
resource against which resource update is requested to be
included in HTTP PATCH operation header.
This commit adds missing ETag in PATCH operation header.

Change-Id: I3f88d8a7032fd9071aa033c706768127da1d12af

releasenotes/notes/fix-missing-etags-ded8c0bb31fafef7.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes missing ETag in PATCH operation against Redfish resources with
+    backward compatibility for Redfish implementation which doesn't work
+    with ETag in header.

sushy/resources/chassis/chassis.py

@@ -214,9 +214,10 @@ class Chassis(base.ResourceBase):
                 parameter='state', value=state,
                 valid_values=' ,'.join(i.value for i in res_cons.IndicatorLED))
 
+        etag = self._get_etag()
         data = {'IndicatorLED': state}
 
-        self._conn.patch(self.path, data=data)
+        self._conn.patch(self.path, data=data, etag=etag)
         self.invalidate()
 
     @property

sushy/resources/manager/virtual_media.py

@@ -248,8 +248,10 @@ class VirtualMedia(base.ResourceBase):
                 parameter='verify_certificate', value=verify_certificate,
                 valid_values='boolean (True, False)')
 
+        etag = self._get_etag()
         self._conn.patch(self.path,
-                         data={'VerifyCertificate': verify_certificate})
+                         data={'VerifyCertificate': verify_certificate},
+                         etag=etag)
         self.invalidate()
 
     @property

sushy/resources/settings.py

@@ -170,7 +170,7 @@ class SettingsField(base.CompositeField):
         :returns: Response object
         """
 
-        return connector.patch(self.resource_uri, data=value)
+        return connector.patch(self.resource_uri, data=value, etag=self._etag)
 
     @property
     def resource_uri(self):

sushy/resources/system/bios.py

@@ -149,6 +149,10 @@ class Bios(base.ResourceBase):
         payload = utils.process_apply_time_input(
             payload, apply_time, maint_window_start_time,
             maint_window_duration)
+        # NOTE(vanou): To retrieve current ETag value of @Redfish.Settings
+        # but not update cached _pending_settings_resource, because cached
+        # property is only this one and re-cache is not required
+        self.refresh(force=False)
         self._settings.commit(self._conn,
                               payload)
         utils.cache_clear(self, force_refresh=False,

sushy/resources/system/secure_boot.py

@@ -144,4 +144,6 @@ class SecureBoot(base.ResourceBase):
             raise exceptions.InvalidParameterValueError(
                 "Expected a boolean for 'enabled', got %r" % enabled)
 
-        self._conn.patch(self.path, data={'SecureBootEnable': enabled})
+        etag = self._get_etag()
+        self._conn.patch(self.path, data={'SecureBootEnable': enabled},
+                         etag=etag)

sushy/resources/system/storage/controller.py

@@ -97,6 +97,10 @@ class StorageController(base.ResourceBase):
         payload = utils.process_apply_time_input(
             payload, apply_time, maint_window_start_time,
             maint_window_duration)
+        # NOTE(vanou): To retrieve current ETag value of @Redfish.Settings
+        # but not update cached pending_settings, because cached property is
+        # only this one and re-cache this is not required
+        self.refresh(force=False)
         r = self._settings.commit(self._conn, payload)
         utils.cache_clear(self, force_refresh=False,
                           only_these=['pending_settings'])

sushy/resources/system/storage/drive.py

@@ -102,7 +102,8 @@ class Drive(base.ResourceBase):
                 parameter='state', value=state,
                 valid_values=' ,'.join(i.value for i in res_cons.IndicatorLED))
 
+        etag = self._get_etag()
         data = {'IndicatorLED': state}
 
-        self._conn.patch(self.path, data=data)
+        self._conn.patch(self.path, data=data, etag=etag)
         self.invalidate()

sushy/resources/system/system.py

@@ -342,9 +342,10 @@ class System(base.ResourceBase):
                 parameter='state', value=state,
                 valid_values=' ,'.join(i.value for i in res_cons.IndicatorLED))
 
+        etag = self._get_etag()
         data = {'IndicatorLED': state}
 
-        self._conn.patch(self.path, data=data)
+        self._conn.patch(self.path, data=data, etag=etag)
         self.invalidate()
 
     def _get_processor_collection_path(self):

sushy/tests/unit/resources/chassis/test_chassis.py

@@ -34,6 +34,7 @@ class ChassisTestCase(base.TestCase):
             self.json_doc = json.load(f)
 
         self.conn.get.return_value.json.return_value = self.json_doc
+        self.conn.get.return_value.headers = {'ETag': 'd37f7bcd528e4d59'}
 
         self.chassis = chassis.Chassis(self.conn, '/redfish/v1/Chassis/Blade1',
                                        redfish_version='1.8.0')
@@ -144,7 +145,8 @@ class ChassisTestCase(base.TestCase):
             self.chassis.set_indicator_led(sushy.IndicatorLED.BLINKING)
             self.chassis._conn.patch.assert_called_once_with(
                 '/redfish/v1/Chassis/Blade1',
-                data={'IndicatorLED': 'Blinking'})
+                data={'IndicatorLED': 'Blinking'},
+                etag='d37f7bcd528e4d59')
 
             invalidate_mock.assert_called_once_with()
 

sushy/tests/unit/resources/manager/test_virtual_media.py

@@ -294,13 +294,16 @@ class VirtualMediaTestCase(base.TestCase):
         self.assertTrue(self.sys_virtual_media._is_stale)
 
     def test_set_verify_certificate(self):
+        self.conn.get.return_value.headers = {'Allow': 'GET,HEAD',
+                                              'ETag': '3d7b8a7360bf2941d'}
         with mock.patch.object(
                 self.sys_virtual_media, 'invalidate',
                 autospec=True) as invalidate_mock:
             self.sys_virtual_media.set_verify_certificate(True)
             self.sys_virtual_media._conn.patch.assert_called_once_with(
                 "/redfish/v1/Managers/BMC/VirtualMedia/Floppy1",
-                data={'VerifyCertificate': True})
+                data={'VerifyCertificate': True},
+                etag='3d7b8a7360bf2941d')
 
             invalidate_mock.assert_called_once_with()
 

sushy/tests/unit/resources/system/storage/test_controller.py

@@ -65,6 +65,8 @@ class ControllerTestCase(base.TestCase):
                          self.controller.supported_apply_times)
 
     def test_update(self):
+        self.conn.get.return_value.json.side_effect = [
+            self.json_doc, self.json_doc]
         mock_response = mock.Mock()
         mock_response.status_code = http_client.ACCEPTED
         mock_response.headers = {'Content-Length': 42,
@@ -81,7 +83,8 @@ class ControllerTestCase(base.TestCase):
             data={'ControllerRates': {'ConsistencyCheckRatePercent': 30},
                   '@Redfish.SettingsApplyTime': {
                       '@odata.type': '#Settings.v1_0_0.PreferredApplyTime',
-                      'ApplyTime': 'OnReset'}})
+                      'ApplyTime': 'OnReset'}},
+            etag=None)
         self.assertIsInstance(tm, taskmonitor.TaskMonitor)
         self.assertEqual('/Task/545', tm.task_monitor_uri)
 

sushy/tests/unit/resources/system/storage/test_drive.py

@@ -83,13 +83,15 @@ class DriveTestCase(base.TestCase):
         self.assertEqual('3', volumes[1].identity)
 
     def test_set_indicator_led(self):
+        self.conn.get.return_value.headers = {'ETag': 'a3b01b63f80a4913'}
         with mock.patch.object(
                 self.stor_drive, 'invalidate',
                 autospec=True) as invalidate_mock:
             self.stor_drive.set_indicator_led(sushy.IndicatorLED.BLINKING)
             self.stor_drive._conn.patch.assert_called_once_with(
                 '/redfish/v1/Systems/437XR1138/Storage/1/Drives/'
-                '32ADF365C6C1B7BD', data={'IndicatorLED': 'Blinking'})
+                '32ADF365C6C1B7BD', data={'IndicatorLED': 'Blinking'},
+                etag='a3b01b63f80a4913')
 
             invalidate_mock.assert_called_once_with()
 

sushy/tests/unit/resources/system/test_bios.py

@@ -119,6 +119,10 @@ class BiosTestCase(base.TestCase):
                          attributes.get('maintenance_window'))
 
     def test_set_attribute_apply_time(self):
+        self.conn.get.return_value.json.side_effect = [
+            self.bios_json,
+            self.bios_json]
+
         self.sys_bios.set_attribute(
             'ProcTurboMode', 'Disabled',
             res_cons.ApplyTime.IN_MAINTENANCE_WINDOW_ON_RESET,
@@ -131,9 +135,15 @@ class BiosTestCase(base.TestCase):
                       '@odata.type': '#Settings.v1_0_0.PreferredApplyTime',
                       'ApplyTime': 'InMaintenanceWindowOnReset',
                       'MaintenanceWindowStartTime': '2020-09-01T04:30:00',
-                      'MaintenanceWindowDurationInSeconds': 600}})
+                      'MaintenanceWindowDurationInSeconds': 600}},
+            etag='9234ac83b9700123cc32')
 
     def test_set_attribute_on_refresh(self):
+        self.conn.get.return_value.json.side_effect = [
+            self.bios_settings_json,
+            self.bios_json,
+            self.bios_settings_json]
+
         self.conn.get.reset_mock()
         # make it to instantiate pending attributes
         self.sys_bios.pending_attributes
@@ -150,6 +160,9 @@ class BiosTestCase(base.TestCase):
         self.assertTrue(self.conn.get.called)
 
     def test_set_attributes(self):
+        self.conn.get.return_value.json.side_effect = [
+            self.bios_json]
+
         self.sys_bios.set_attributes(
             {'ProcTurboMode': 'Disabled', 'UsbControl': 'UsbDisabled'},
             res_cons.ApplyTime.AT_MAINTENANCE_WINDOW_START,
@@ -163,9 +176,15 @@ class BiosTestCase(base.TestCase):
                       '@odata.type': '#Settings.v1_0_0.PreferredApplyTime',
                       'ApplyTime': 'AtMaintenanceWindowStart',
                       'MaintenanceWindowStartTime': '2020-09-01T04:30:00',
-                      'MaintenanceWindowDurationInSeconds': 600}})
+                      'MaintenanceWindowDurationInSeconds': 600}},
+            etag='9234ac83b9700123cc32')
 
     def test_set_attributes_on_refresh(self):
+        self.conn.get.return_value.json.side_effect = [
+            self.bios_settings_json,
+            self.bios_json,
+            self.bios_settings_json]
+
         self.conn.get.reset_mock()
         # make it to instantiate pending attributes
         self.sys_bios.pending_attributes

sushy/tests/unit/resources/system/test_secure_boot.py

@@ -29,6 +29,7 @@ class SecureBootTestCase(base.TestCase):
             self.secure_boot_json = json.load(f)
 
         self.conn.get.return_value.json.return_value = self.secure_boot_json
+        self.conn.get.return_value.headers = {'ETag': 'b26ae716a2c1f39f'}
         self.secure_boot = secure_boot.SecureBoot(
             self.conn, '/redfish/v1/Systems/437XR1138R2/SecureBoot',
             registries={}, redfish_version='1.1.0')
@@ -79,7 +80,8 @@ class SecureBootTestCase(base.TestCase):
         self.secure_boot.set_enabled(True)
         self.conn.patch.assert_called_once_with(
             '/redfish/v1/Systems/437XR1138R2/SecureBoot',
-            data={'SecureBootEnable': True})
+            data={'SecureBootEnable': True},
+            etag='b26ae716a2c1f39f')
 
     def test_set_enabled_wrong_type(self):
         self.assertRaises(exceptions.InvalidParameterValueError,

sushy/tests/unit/resources/system/test_system.py

@@ -46,6 +46,8 @@ class SystemTestCase(base.TestCase):
         self.sys_inst = system.System(
             self.conn, '/redfish/v1/Systems/437XR1138R2',
             redfish_version='1.0.2')
+        self.sys_inst._get_etag = mock.Mock()
+        self.sys_inst._get_etag.return_value = '81802dbf61beb0bd'
 
     def test__parse_attributes(self):
         self.sys_inst._parse_attributes(self.json_doc)
@@ -283,7 +285,7 @@ class SystemTestCase(base.TestCase):
             data={'Boot': {'BootSourceOverrideEnabled': 'Continuous',
                            'BootSourceOverrideTarget': 'Pxe',
                            'BootSourceOverrideMode': 'UEFI'}},
-            etag=None)
+            etag='81802dbf61beb0bd')
 
     def test_set_system_boot_options_no_mode_specified(self):
         self.sys_inst.set_system_boot_options(
@@ -293,7 +295,7 @@ class SystemTestCase(base.TestCase):
             '/redfish/v1/Systems/437XR1138R2',
             data={'Boot': {'BootSourceOverrideEnabled': 'Once',
                            'BootSourceOverrideTarget': 'Hdd'}},
-            etag=None)
+            etag='81802dbf61beb0bd')
 
     def test_set_system_boot_options_no_target_specified(self):
         self.sys_inst.set_system_boot_options(
@@ -303,7 +305,7 @@ class SystemTestCase(base.TestCase):
             '/redfish/v1/Systems/437XR1138R2',
             data={'Boot': {'BootSourceOverrideEnabled': 'Continuous',
                            'BootSourceOverrideMode': 'UEFI'}},
-            etag=None)
+            etag='81802dbf61beb0bd')
 
     def test_set_system_boot_options_no_freq_specified(self):
         self.sys_inst.set_system_boot_options(
@@ -313,7 +315,7 @@ class SystemTestCase(base.TestCase):
             '/redfish/v1/Systems/437XR1138R2',
             data={'Boot': {'BootSourceOverrideTarget': 'Pxe',
                            'BootSourceOverrideMode': 'UEFI'}},
-            etag=None)
+            etag='81802dbf61beb0bd')
 
     def test_set_system_boot_options_nothing_specified(self):
         self.sys_inst.set_system_boot_options()
@@ -348,7 +350,7 @@ class SystemTestCase(base.TestCase):
             '/redfish/v1/Systems/437XR1138R2',
             data={'Boot': {'BootSourceOverrideEnabled': 'Once',
                            'BootSourceOverrideTarget': 'UsbCd'}},
-            etag=None)
+            etag='81802dbf61beb0bd')
 
     def test_set_system_boot_options_supermicro_no_usb_cd_boot(self):
 
@@ -361,7 +363,7 @@ class SystemTestCase(base.TestCase):
             '/redfish/v1/Systems/437XR1138R2',
             data={'Boot': {'BootSourceOverrideEnabled': 'Once',
                            'BootSourceOverrideTarget': 'Cd'}},
-            etag=None)
+            etag='81802dbf61beb0bd')
 
     def test_set_system_boot_options_settings_resource_nokia(self):
         with open('sushy/tests/unit/json_samples/settings-nokia.json') as f:
@@ -487,10 +489,10 @@ class SystemTestCase(base.TestCase):
             data={'Boot': {'BootSourceOverrideEnabled': 'Continuous',
                            'BootSourceOverrideTarget': 'Pxe',
                            'BootSourceOverrideMode': 'UEFI'}},
-            etag=None)
+            etag='81802dbf61beb0bd')
 
     def test_set_system_boot_source_with_etag(self):
-        self.conn.get.return_value.headers = {'ETag': '"3d7b838291941d"'}
+        self.conn.get.return_value.headers = {'ETag': '"81802dbf61beb0bd"'}
         self.sys_inst.set_system_boot_source(
             sushy.BOOT_SOURCE_TARGET_PXE,
             enabled=sushy.BOOT_SOURCE_ENABLED_CONTINUOUS,
@@ -500,7 +502,7 @@ class SystemTestCase(base.TestCase):
             data={'Boot': {'BootSourceOverrideEnabled': 'Continuous',
                            'BootSourceOverrideTarget': 'Pxe',
                            'BootSourceOverrideMode': 'UEFI'}},
-            etag='"3d7b838291941d"')
+            etag="81802dbf61beb0bd")
 
     def test_set_system_boot_source_no_mode_specified(self):
         self.sys_inst.set_system_boot_source(
@@ -510,7 +512,7 @@ class SystemTestCase(base.TestCase):
             '/redfish/v1/Systems/437XR1138R2',
             data={'Boot': {'BootSourceOverrideEnabled': 'Once',
                            'BootSourceOverrideTarget': 'Hdd'}},
-            etag=None)
+            etag='81802dbf61beb0bd')
 
     def test_set_system_boot_source_invalid_target(self):
         self.assertRaises(exceptions.InvalidParameterValueError,
@@ -533,7 +535,8 @@ class SystemTestCase(base.TestCase):
             self.sys_inst.set_indicator_led(sushy.IndicatorLED.BLINKING)
             self.sys_inst._conn.patch.assert_called_once_with(
                 '/redfish/v1/Systems/437XR1138R2',
-                data={'IndicatorLED': 'Blinking'})
+                data={'IndicatorLED': 'Blinking'},
+                etag='81802dbf61beb0bd')
 
             invalidate_mock.assert_called_once_with()
 

sushy/tests/unit/resources/test_settings.py

@@ -78,7 +78,8 @@ class SettingsFieldTestCase(base.TestCase):
         instance.commit(conn, {'Attributes': {'key': 'value'}})
         conn.patch.assert_called_once_with(
             '/redfish/v1/Systems/437XR1138R2/BIOS/Settings',
-            data={'Attributes': {'key': 'value'}})
+            data={'Attributes': {'key': 'value'}},
+            etag='9234ac83b9700123cc32')
 
     def test_get_status_failure(self):
         instance = self.settings._load(self.json, mock.Mock())