From ecf1bcc80bd14a1836d015c3dbdb4fd88f2bbd75 Mon Sep 17 00:00:00 2001 From: Jacob Anders Date: Tue, 22 Feb 2022 18:24:39 +1000 Subject: [PATCH] Re-trying InsertMedia call with TransferProtocolType if required Adding extra logic to handle InsertMedia API call failure due to TransferProtocolType parameter missing. This is a non-standard parameter (rarely) required on some servers. If such failure mode is detected, TransferProtocolType value is automatically added to the request payload prior to re-trying the InsertMedia call. Story: 2009871 Task: 44574 Change-Id: I608a776f1849621f00e17d1edca16a54906d05fd --- ...ferprototype-missing-9cae57f3ecf470a9.yaml | 7 +++++ sushy/resources/manager/virtual_media.py | 26 ++++++++++++++++++- .../transfer_proto_required_error.json | 22 ++++++++++++++++ .../resources/manager/test_virtual_media.py | 16 ++++++++++++ 4 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/retry-if-transferprototype-missing-9cae57f3ecf470a9.yaml create mode 100644 sushy/tests/unit/json_samples/transfer_proto_required_error.json diff --git a/releasenotes/notes/retry-if-transferprototype-missing-9cae57f3ecf470a9.yaml b/releasenotes/notes/retry-if-transferprototype-missing-9cae57f3ecf470a9.yaml new file mode 100644 index 00000000..5f80cde9 --- /dev/null +++ b/releasenotes/notes/retry-if-transferprototype-missing-9cae57f3ecf470a9.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Resolved virtualmedia attach failures caused by the lack of + TransferProtocolType parameter in the request payload which is required + by certain BMCs (e.g. on Nokia servers). This is done by adding capability + to retry virtualmedia InsertMedia with the updated payload in such cases. diff --git a/sushy/resources/manager/virtual_media.py b/sushy/resources/manager/virtual_media.py index 5ba61903..7b7b857e 100644 --- a/sushy/resources/manager/virtual_media.py +++ b/sushy/resources/manager/virtual_media.py @@ -106,6 +106,18 @@ class VirtualMedia(base.ResourceBase): eject_uri = eject_media.target_uri return eject_uri, use_patch + def is_transfer_protocol_required(self, response=None): + """Check the response code and body and in case of failure + + Try to determine if it happened due to missing TransferProtocolType. + """ + if (response.code == "Base.1.5.ActionParameterMissing" + and response.body is not None): + if ("#/TransferProtocolType" in response.body + ["@Message.ExtendedInfo"][0]['RelatedProperties']): + return True + return False + def insert_media(self, image, inserted=True, write_protected=True, username=None, password=None, transfer_method=None): """Attach remote media to virtual media @@ -161,7 +173,19 @@ class VirtualMedia(base.ResourceBase): payload['Inserted'] = False if not write_protected: payload['WriteProtected'] = False - self._conn.post(target_uri, data=payload) + # NOTE(janders) attempting to detect whether attachment failure is + # due to absence of TransferProtocolType param and if so adding it + try: + self._conn.post(target_uri, data=payload) + except exceptions.HTTPError as response: + if self.is_transfer_protocol_required(response): + if payload['Image'].startswith('https://'): + payload['TransferProtocolType'] = "HTTPS" + elif payload['Image'].startswith('http://'): + payload['TransferProtocolType'] = "HTTP" + self._conn.post(target_uri, data=payload) + else: + raise self.invalidate() def eject_media(self): diff --git a/sushy/tests/unit/json_samples/transfer_proto_required_error.json b/sushy/tests/unit/json_samples/transfer_proto_required_error.json new file mode 100644 index 00000000..64839104 --- /dev/null +++ b/sushy/tests/unit/json_samples/transfer_proto_required_error.json @@ -0,0 +1,22 @@ +{ + "error": { + "@Message.ExtendedInfo": [ + { + "@odata.type": "#Message.v1_0_8.Message", + "Message": "The action /redfish/v1/Managers/Self/VirtualMedia/CD1/Actions/VirtualMedia.InsertMedia requires the parameter TransferProtocolType to be present in the request body.", + "MessageArgs": [ + "/redfish/v1/Managers/Self/VirtualMedia/CD1/Actions/VirtualMedia.InsertMedia", + "TransferProtocolType" + ], + "MessageId": "Base.1.5.ActionParameterMissing", + "RelatedProperties": [ + "#/TransferProtocolType" + ], + "Resolution": "Supply the action with the required parameter in the request body when the request is resubmitted.", + "Severity": "Critical" + } + ], + "code": "Base.1.5.ActionParameterMissing", + "message": "The action /redfish/v1/Managers/Self/VirtualMedia/CD1/Actions/VirtualMedia.InsertMedia requires the parameter TransferProtocolType to be present in the request body." + } +} diff --git a/sushy/tests/unit/resources/manager/test_virtual_media.py b/sushy/tests/unit/resources/manager/test_virtual_media.py index a79fca98..fc876428 100644 --- a/sushy/tests/unit/resources/manager/test_virtual_media.py +++ b/sushy/tests/unit/resources/manager/test_virtual_media.py @@ -16,6 +16,8 @@ from http import client as http_client import json from unittest import mock +import requests + import sushy from sushy import exceptions from sushy.resources.certificateservice import certificate @@ -176,6 +178,20 @@ class VirtualMediaTestCase(base.TestCase): headers={"If-Match": 'W/"3d7b8a7360bf2941d","3d7b8a7360bf2941d"'}) self.assertTrue(self.sys_virtual_media._is_stale) + @mock.patch.object(requests, 'post', autospec=True) + def test_is_transfer_protocol_required(self, mock_post): + with open('sushy/tests/unit/json_samples/' + 'transfer_proto_required_error.json') as f: + response_obj = json.load(f) + response = mock.MagicMock() + response.status_code = 400 + response.code = "Base.1.5.ActionParameterMissing" + response.body = response_obj['error'] + response.raise_for_status.side_effect = requests.exceptions.HTTPError + mock_post.return_value = response + retval = self.sys_virtual_media.is_transfer_protocol_required(response) + self.assertTrue(retval) + def test_eject_media_none(self): self.sys_virtual_media._actions.eject_media = None self.assertRaisesRegex(