From bc4987800906d8413f964c9a3f5c7eb580de55e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Cmit=20Seren?= Date: Sat, 24 Apr 2021 21:57:26 +0200 Subject: [PATCH] Implement fallback method for virtual media Some vendors implement the original method of inserting and ejecting virtual media using a PATCH request to the target URI instead of using a POST request to the insert_media and eject_media action URI. Additionally the actions attribute might set to None This implements the fallback method, if there are no actions and the PATCH method is sent back in the Allow header Change-Id: I2046c4e7c8739edd4aaac3e7facc378b82276ca4 (cherry picked from commit 9e5a103565e204b9358e10580cd0346391be9bec) --- ...rtual-media-fallback-15a559414a65c014.yaml | 7 ++ sushy/resources/base.py | 30 +++++++ sushy/resources/manager/virtual_media.py | 61 ++++++++++---- .../resources/manager/test_virtual_media.py | 81 +++++++++++++++++++ 4 files changed, 164 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/fix-virtual-media-fallback-15a559414a65c014.yaml 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")