Removing optional fields from insert_media payload

Removing "Inserted" and "WriteProtected" from the
VirtualMedia::insert_media() API call payload when PATCH method of
configuring virtual media is not used (e.g. InserMedia action) and
values are set to defaults as per Redfish specification
(https://redfish.dmtf.org/schemas/v1/VirtualMedia.v1_3_0.json):
{"Inserted": true, "WriteProtected": True}.

"Inserted" and "WriteProtected" attributes to VirtualMedia::insert_media()
are optional and should have reasonable default values on the BMC side.
Specifying them in every insert_media call can cause issues on those BMCs that
treat them as read-only and not specifying these attributes at all should
not cause issues. SuperMicro X11/X12 platforms can be such example -
vMedia will not work on these without this fix.
Keeping old behaviour in BMCs using PATCH request for configuring
virtual media (e.g. Lenovo SD530) as these may need "Inserted" attribute
to attach virtual media correctly.

Story: 2009086
Task: 42907
rhbz#1986238

Change-Id: I975b3a4aa53704906cb13c859b61a254de79933f

releasenotes/notes/fix-insert-media-payload-b5d4c707f81d9603.yaml

@@ -0,0 +1,20 @@
+---
+upgrade:
+  - |
+    "Inserted" and "WriteProtected" optional attributes are no longer present
+    in the InsertMedia API call payload when default values {"Inserted": True,
+    "WriteProtected": True} are specified by the consumer (e.g. Ironic) and
+    PATCH method of configuring virtual media is not used.
+    Behaviour is unchanged if PATCH method is used.
+fixes:
+  - |
+    Removing "Inserted" and "WriteProtected" parameters from the Redfish
+    VirtualMedia::insert_media() API call payload when default values
+    {"Inserted": True, "WriteProtected": True} are set and PATCH method is not
+    used. Those parameters are optional as per Redfish schema 2021.1. Some
+    BMCs (e.g. SuperMicro X11/X12 platforms) treat these fields as read-only
+    and setting them causes vMedia insert failures. These attributes should
+    default to True on the BMC side.
+    Some BMCs using PATCH method of configuring virtual media (e.g. Lenovo
+    SD530) still require "Inserted" attribute, so only changing this for
+    non-PATCH methods of configuring virtual media such as InsertMedia action.

sushy/resources/manager/virtual_media.py

@@ -102,15 +102,33 @@ class VirtualMedia(base.ResourceBase):
         :param write_protected: indicates the media is write protected
         """
         target_uri, use_patch = self._get_insert_media_uri()
-        payload = {"Image": image, "Inserted": inserted,
-                   "WriteProtected": write_protected}
+        # NOTE(janders) Inserted and WriteProtected attributes are optional
+        # as per Redfish schema 2021.1. However - some BMCs (e.g. Lenovo SD530
+        # which is using PATCH method as opposed to InsertMedia action) will
+        # not attach vMedia if Inserted is not specified.
+        # On the other hand, machines such as SuperMicro X11 will return
+        # an error if Inserted or WriteProtected are specified. In order to
+        # make both work, we remove Inserted and WriteProtected from payload
+        # for BMCs which don't use PATCH if their values are set to defaults
+        # as per the spec (True, True). We continue to set Inserted and
+        # WriteProtected in payload if PATCH method is used.
+        payload = {'Image': image}
         if use_patch:
+            payload['Inserted'] = inserted
+            payload['WriteProtected'] = write_protected
             headers = None
             etag = self._get_etag()
             if etag is not None:
                 headers = {"If-Match": etag}
             self._conn.patch(target_uri, data=payload, headers=headers)
         else:
+            # NOTE(janders) only include Inserted and WriteProtected
+            # in request payload if values other than defaults (True,True)
+            # are set (fix for SuperMicro X11/X12).
+            if not inserted:
+                payload['Inserted'] = False
+            if not write_protected:
+                payload['WriteProtected'] = False
             self._conn.post(target_uri, data=payload)
         self.invalidate()
 

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

@@ -89,7 +89,18 @@ class VirtualMediaTestCase(base.TestCase):
             ("/redfish/v1/Managers/BMC/VirtualMedia/Floppy1/Actions"
              "/VirtualMedia.InsertMedia"),
             data={"Image": "https://www.dmtf.org/freeImages/Sardine.img",
-                  "Inserted": True, "WriteProtected": False}
+                  "WriteProtected": False}
+        )
+        self.assertTrue(self.sys_virtual_media._is_stale)
+
+    def test_insert_media_rf_default(self):
+        self.assertFalse(self.sys_virtual_media._is_stale)
+        self.sys_virtual_media.insert_media(
+            "https://www.dmtf.org/freeImages/Sardine.img", True, True)
+        self.sys_virtual_media._conn.post.assert_called_once_with(
+            ("/redfish/v1/Managers/BMC/VirtualMedia/Floppy1/Actions"
+             "/VirtualMedia.InsertMedia"),
+            data={"Image": "https://www.dmtf.org/freeImages/Sardine.img"}
         )
         self.assertTrue(self.sys_virtual_media._is_stale)