diff --git a/releasenotes/notes/fix-virtual-media-fallback-15a559414a65c014.yaml b/releasenotes/notes/fix-virtual-media-fallback-15a559414a65c014.yaml new file mode 100644 index 00000000..caf129c5 --- /dev/null +++ b/releasenotes/notes/fix-virtual-media-fallback-15a559414a65c014.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Adds a fallback for inserting and ejecting virtual media + using the PATCH HTTP request instead of the explicit action URIs. + The fallback is required for Lenovo ThinkSystem machines (i.e. SD530, ..) + that only implement the PATCH method. \ No newline at end of file diff --git a/sushy/resources/base.py b/sushy/resources/base.py index 6ecf4e8f..b2b40c1e 100644 --- a/sushy/resources/base.py +++ b/sushy/resources/base.py @@ -19,6 +19,7 @@ import copy import io import json import logging +import re import zipfile import pkg_resources @@ -570,6 +571,35 @@ class ResourceBase(object, metaclass=abc.ABCMeta): return settings + def _get_etag(self): + """Returns the ETag of the HTTP request if any was specified. + + :returns ETag or None + """ + pattern = re.compile(r'^(W\/)?("\w*")$') + match = pattern.match(self._get_headers().get('ETag', '')) + if match: + return match.group(2) + return None + + def _get_headers(self): + """Returns the HTTP headers of the request for the resource. + + :returns: dict of HTTP headers + """ + return self._reader.get_data()._headers + + def _allow_patch(self): + """Returns if the resource supports the PATCH HTTP method. + + If the resource supports the PATCH HTTP method for updates, + it will return it in the Allow HTTP header. + :returns: Boolean flag if PATCH is supported or not + """ + allow_header = self._get_headers().get('Allow', '') + methods = set([h.strip().upper() for h in allow_header.split(',')]) + return "PATCH" in methods + def refresh(self, force=True, json_doc=None): """Refresh the resource diff --git a/sushy/resources/manager/virtual_media.py b/sushy/resources/manager/virtual_media.py index 150e7d06..a1000223 100644 --- a/sushy/resources/manager/virtual_media.py +++ b/sushy/resources/manager/virtual_media.py @@ -67,19 +67,31 @@ class VirtualMedia(base.ResourceBase): _actions = ActionsField('Actions') """Insert/eject action for virtual media""" - def _get_insert_media_element(self): - insert_media = self._actions.insert_media + def _get_insert_media_uri(self): + insert_media = self._actions.insert_media if self._actions else None + use_patch = False if not insert_media: - raise exceptions.MissingActionError( - action='#VirtualMedia.InsertMedia', resource=self._path) - return insert_media + insert_uri = self.path + use_patch = self._allow_patch() + if not use_patch: + raise exceptions.MissingActionError( + action='#VirtualMedia.InsertMedia', resource=self._path) + else: + insert_uri = insert_media.target_uri + return insert_uri, use_patch - def _get_eject_media_element(self): - eject_media = self._actions.eject_media + def _get_eject_media_uri(self): + eject_media = self._actions.eject_media if self._actions else None + use_patch = False if not eject_media: - raise exceptions.MissingActionError( - action='#VirtualMedia.EjectMedia', resource=self._path) - return eject_media + eject_uri = self.path + use_patch = self._allow_patch() + if not use_patch: + raise exceptions.MissingActionError( + action='#VirtualMedia.EjectMedia', resource=self._path) + else: + eject_uri = eject_media.target_uri + return eject_uri, use_patch def insert_media(self, image, inserted=True, write_protected=False): """Attach remote media to virtual media @@ -89,9 +101,17 @@ class VirtualMedia(base.ResourceBase): completion of the action. :param write_protected: indicates the media is write protected """ - target_uri = self._get_insert_media_element().target_uri - self._conn.post(target_uri, data={"Image": image, "Inserted": inserted, - "WriteProtected": write_protected}) + target_uri, use_patch = self._get_insert_media_uri() + payload = {"Image": image, "Inserted": inserted, + "WriteProtected": write_protected} + if use_patch: + 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: + self._conn.post(target_uri, data=payload) self.invalidate() def eject_media(self): @@ -101,8 +121,19 @@ class VirtualMedia(base.ResourceBase): empty. """ try: - target_uri = self._get_eject_media_element().target_uri - self._conn.post(target_uri) + target_uri, use_patch = self._get_eject_media_uri() + if use_patch: + payload = { + "Image": None, + "Inserted": False + } + 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: + self._conn.post(target_uri) except exceptions.HTTPError as response: # Some vendors like HPE iLO has this kind of implementation. # It needs to pass an empty dict. diff --git a/sushy/tests/unit/resources/manager/test_virtual_media.py b/sushy/tests/unit/resources/manager/test_virtual_media.py index 6bbb8d27..fd52af8c 100644 --- a/sushy/tests/unit/resources/manager/test_virtual_media.py +++ b/sushy/tests/unit/resources/manager/test_virtual_media.py @@ -28,6 +28,7 @@ class VirtualMediaTestCase(base.TestCase): def setUp(self): super(VirtualMediaTestCase, self).setUp() self.conn = mock.Mock() + self.conn.get.return_value.headers = {'Allow': 'GET,HEAD'} with open('sushy/tests/unit/json_samples/' 'virtual_media.json') as f: self.json_doc = json.load(f) @@ -74,6 +75,12 @@ class VirtualMediaTestCase(base.TestCase): self.sys_virtual_media.insert_media, "https://www.dmtf.org/freeImages/Sardine.img", True, False) + self.sys_virtual_media._actions = None + self.assertRaisesRegex( + exceptions.MissingActionError, 'action #VirtualMedia.InsertMedia', + self.sys_virtual_media.insert_media, + "https://www.dmtf.org/freeImages/Sardine.img", True, False) + def test_insert_media(self): self.assertFalse(self.sys_virtual_media._is_stale) self.sys_virtual_media.insert_media( @@ -86,12 +93,55 @@ class VirtualMediaTestCase(base.TestCase): ) self.assertTrue(self.sys_virtual_media._is_stale) + def test_insert_media_fallback(self): + self.conn.get.return_value.headers = {'Allow': 'GET,HEAD,PATCH'} + self.sys_virtual_media._actions.insert_media = None + self.sys_virtual_media.insert_media( + "https://www.dmtf.org/freeImages/Sardine.img", True, False) + self.sys_virtual_media._conn.patch.assert_called_once_with( + ("/redfish/v1/Managers/BMC/VirtualMedia/Floppy1"), + data={"Image": "https://www.dmtf.org/freeImages/Sardine.img", + "Inserted": True, "WriteProtected": False}, + headers=None) + self.assertTrue(self.sys_virtual_media._is_stale) + + def test_insert_media_fallback_with_etag(self): + self.conn.get.return_value.headers = {'Allow': 'GET,HEAD,PATCH', + 'ETag': '"3d7b8a7360bf2941d"'} + self.sys_virtual_media._actions.insert_media = None + self.sys_virtual_media.insert_media( + "https://www.dmtf.org/freeImages/Sardine.img", True, False) + self.sys_virtual_media._conn.patch.assert_called_once_with( + ("/redfish/v1/Managers/BMC/VirtualMedia/Floppy1"), + data={"Image": "https://www.dmtf.org/freeImages/Sardine.img", + "Inserted": True, "WriteProtected": False}, + headers={"If-Match": '"3d7b8a7360bf2941d"'}) + self.assertTrue(self.sys_virtual_media._is_stale) + + def test_insert_media_fallback_with_weak_etag(self): + self.conn.get.return_value.headers = {'Allow': 'GET,HEAD,PATCH', + 'ETag': 'W/"3d7b8a7360bf2941d"'} + self.sys_virtual_media._actions.insert_media = None + self.sys_virtual_media.insert_media( + "https://www.dmtf.org/freeImages/Sardine.img", True, False) + self.sys_virtual_media._conn.patch.assert_called_once_with( + ("/redfish/v1/Managers/BMC/VirtualMedia/Floppy1"), + data={"Image": "https://www.dmtf.org/freeImages/Sardine.img", + "Inserted": True, "WriteProtected": False}, + headers={"If-Match": '"3d7b8a7360bf2941d"'}) + self.assertTrue(self.sys_virtual_media._is_stale) + def test_eject_media_none(self): self.sys_virtual_media._actions.eject_media = None self.assertRaisesRegex( exceptions.MissingActionError, 'action #VirtualMedia.EjectMedia', self.sys_virtual_media.eject_media) + self.sys_virtual_media._actions = None + self.assertRaisesRegex( + exceptions.MissingActionError, 'action #VirtualMedia.EjectMedia', + self.sys_virtual_media.eject_media) + def test_eject_media(self): self.assertFalse(self.sys_virtual_media._is_stale) self.sys_virtual_media.eject_media() @@ -100,6 +150,37 @@ class VirtualMediaTestCase(base.TestCase): "/VirtualMedia.EjectMedia")) self.assertTrue(self.sys_virtual_media._is_stale) + def test_eject_media_fallback(self): + self.conn.get.return_value.headers = {'Allow': 'GET,HEAD,PATCH'} + self.sys_virtual_media._actions.eject_media = None + self.sys_virtual_media.eject_media() + self.sys_virtual_media._conn.patch.assert_called_once_with( + ("/redfish/v1/Managers/BMC/VirtualMedia/Floppy1"), + data={"Image": None, "Inserted": False}, headers=None) + self.assertTrue(self.sys_virtual_media._is_stale) + + def test_eject_media_fallback_with_etag(self): + self.conn.get.return_value.headers = {'Allow': 'GET,HEAD,PATCH', + 'ETag': '"3d7b8a7360bf2941d"'} + self.sys_virtual_media._actions.eject_media = None + self.sys_virtual_media.eject_media() + self.sys_virtual_media._conn.patch.assert_called_once_with( + ("/redfish/v1/Managers/BMC/VirtualMedia/Floppy1"), + data={"Image": None, "Inserted": False}, + headers={"If-Match": '"3d7b8a7360bf2941d"'}) + self.assertTrue(self.sys_virtual_media._is_stale) + + def test_eject_media_fallback_with_weak_etag(self): + self.conn.get.return_value.headers = {'Allow': 'GET,HEAD,PATCH', + 'ETag': 'W/"3d7b8a7360bf2941d"'} + self.sys_virtual_media._actions.eject_media = None + self.sys_virtual_media.eject_media() + self.sys_virtual_media._conn.patch.assert_called_once_with( + ("/redfish/v1/Managers/BMC/VirtualMedia/Floppy1"), + data={"Image": None, "Inserted": False}, + headers={"If-Match": '"3d7b8a7360bf2941d"'}) + self.assertTrue(self.sys_virtual_media._is_stale) + def test_eject_media_pass_empty_dict_415(self): target_uri = ("/redfish/v1/Managers/BMC/VirtualMedia/Floppy1/Actions" "/VirtualMedia.EjectMedia")