Handle HTTP errors when downloading virtual media
Currently the emulator happily downloads the error page from nginx and tries to connect it to libvirt. This change catches errors and returns an error to the caller with either Bad Request (for 4xx codes upstream) or Bad Gateway (for 5xx codes upstream). Change-Id: I67fadb6b68c0fade9cfd122fe9a36dc6c0d34959
This commit is contained in:
Dmitry Tantsur
5
releasenotes/notes/vmedia-error-73fda32cf6046554.yaml
Normal file
5
releasenotes/notes/vmedia-error-73fda32cf6046554.yaml
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
fixes:
|
||||
- |
|
||||
Fixes handling HTTP errors on downloading virtual media files. Error codes
|
||||
are no longer silently ignored.
|
||||
@@ -127,6 +127,30 @@ class StaticDriver(base.DriverBase):
|
||||
device_info.get('Inserted', False),
|
||||
device_info.get('WriteProtected', False))
|
||||
|
||||
def _write_from_response(self, image_url, rsp, tmp_file):
|
||||
with open(tmp_file.name, 'wb') as fl:
|
||||
for chunk in rsp.iter_content(chunk_size=8192):
|
||||
if chunk:
|
||||
fl.write(chunk)
|
||||
|
||||
local_file = None
|
||||
|
||||
content_dsp = rsp.headers.get('content-disposition')
|
||||
if content_dsp:
|
||||
local_file = re.findall('filename="(.+)"', content_dsp)
|
||||
|
||||
if local_file:
|
||||
local_file = local_file[0]
|
||||
|
||||
if not local_file:
|
||||
parsed_url = urlparse.urlparse(image_url)
|
||||
local_file = os.path.basename(parsed_url.path)
|
||||
|
||||
if not local_file:
|
||||
local_file = 'image.iso'
|
||||
|
||||
return local_file
|
||||
|
||||
def insert_image(self, identity, device, image_url,
|
||||
inserted=True, write_protected=True):
|
||||
"""Upload, remove or insert virtual media
|
||||
@@ -144,42 +168,34 @@ class StaticDriver(base.DriverBase):
|
||||
'SUSHY_EMULATOR_VMEDIA_VERIFY_SSL', True)
|
||||
|
||||
try:
|
||||
with tempfile.NamedTemporaryFile(
|
||||
mode='w+b', delete=False) as tmp_file:
|
||||
with requests.get(image_url,
|
||||
stream=True,
|
||||
verify=verify_media_cert) as rsp:
|
||||
if rsp.status_code >= 400:
|
||||
self._logger.error(
|
||||
'Failed fetching image from URL %s: '
|
||||
'got HTTP error %s:\n%s',
|
||||
image_url, rsp.status_code, rsp.text)
|
||||
target_code = 502 if rsp.status_code >= 500 else 400
|
||||
raise error.FishyError(
|
||||
"Cannot download virtual media: got error %s "
|
||||
"from the server" % rsp.status_code,
|
||||
code=target_code)
|
||||
|
||||
with requests.get(image_url,
|
||||
stream=True,
|
||||
verify=verify_media_cert) as rsp:
|
||||
|
||||
with open(tmp_file.name, 'wb') as fl:
|
||||
|
||||
for chunk in rsp.iter_content(chunk_size=8192):
|
||||
if chunk:
|
||||
fl.write(chunk)
|
||||
|
||||
local_file = None
|
||||
|
||||
content_dsp = rsp.headers.get('content-disposition')
|
||||
if content_dsp:
|
||||
local_file = re.findall('filename="(.+)"', content_dsp)
|
||||
|
||||
if local_file:
|
||||
local_file = local_file[0]
|
||||
|
||||
if not local_file:
|
||||
parsed_url = urlparse.urlparse(image_url)
|
||||
local_file = os.path.basename(parsed_url.path)
|
||||
|
||||
if not local_file:
|
||||
local_file = 'image.iso'
|
||||
with tempfile.NamedTemporaryFile(
|
||||
mode='w+b', delete=False) as tmp_file:
|
||||
|
||||
local_file = self._write_from_response(image_url,
|
||||
rsp, tmp_file)
|
||||
temp_dir = tempfile.mkdtemp(
|
||||
dir=os.path.dirname(tmp_file.name))
|
||||
|
||||
local_file_path = os.path.join(temp_dir, local_file)
|
||||
|
||||
os.rename(tmp_file.name, local_file_path)
|
||||
|
||||
except error.FishyError as ex:
|
||||
msg = 'Failed fetching image from URL %s: %s' % (image_url, ex)
|
||||
self._logger.error(msg)
|
||||
raise # leave the original error intact (code, etc)
|
||||
except Exception as ex:
|
||||
msg = 'Failed fetching image from URL %s: %s' % (image_url, ex)
|
||||
self._logger.exception(msg)
|
||||
|
||||
@@ -17,6 +17,10 @@
|
||||
class FishyError(Exception):
|
||||
"""Create generic sushy-tools exception object"""
|
||||
|
||||
def __init__(self, msg='Unknown error', code=500):
|
||||
super().__init__(msg)
|
||||
self.code = code
|
||||
|
||||
|
||||
class AliasAccessError(FishyError):
|
||||
"""Node access attempted via an alias, not UUID"""
|
||||
|
||||
@@ -19,6 +19,7 @@ from unittest import mock
|
||||
from oslotest import base
|
||||
|
||||
from sushy_tools.emulator.resources import vmedia
|
||||
from sushy_tools import error
|
||||
|
||||
|
||||
class StaticDriverTestCase(base.BaseTestCase):
|
||||
@@ -90,6 +91,7 @@ class StaticDriverTestCase(base.BaseTestCase):
|
||||
mock_rsp.headers = {
|
||||
'content-disposition': 'attachment; filename="fish.iso"'
|
||||
}
|
||||
mock_rsp.status_code = 200
|
||||
|
||||
local_file = self.test_driver.insert_image(
|
||||
self.UUID, 'Cd', 'http://fish.it/red.iso', inserted=True,
|
||||
@@ -125,6 +127,7 @@ class StaticDriverTestCase(base.BaseTestCase):
|
||||
mock_tmp_file.name = 'alphabet.soup'
|
||||
mock_rsp = mock_requests.get.return_value.__enter__.return_value
|
||||
mock_rsp.headers = {}
|
||||
mock_rsp.status_code = 200
|
||||
|
||||
local_file = self.test_driver.insert_image(
|
||||
self.UUID, 'Cd', 'http://fish.it/red.iso', inserted=True,
|
||||
@@ -159,6 +162,7 @@ class StaticDriverTestCase(base.BaseTestCase):
|
||||
mock_tmp_file.name = 'alphabet.soup'
|
||||
mock_rsp = mock_requests.get.return_value.__enter__.return_value
|
||||
mock_rsp.headers = {}
|
||||
mock_rsp.status_code = 200
|
||||
|
||||
full_url = 'http://[::2]:80/redfish/boot-abc?filename=tmp.iso'
|
||||
local_file = self.test_driver.insert_image(
|
||||
@@ -196,6 +200,7 @@ class StaticDriverTestCase(base.BaseTestCase):
|
||||
mock_rsp.headers = {
|
||||
'content-disposition': 'attachment; filename="fish.iso"'
|
||||
}
|
||||
mock_rsp.status_code = 200
|
||||
|
||||
ssl_conf_key = 'SUSHY_EMULATOR_VMEDIA_VERIFY_SSL'
|
||||
default_ssl_verify = self.test_driver._config.get(ssl_conf_key)
|
||||
@@ -220,6 +225,36 @@ class StaticDriverTestCase(base.BaseTestCase):
|
||||
self.assertFalse(device_info['WriteProtected'])
|
||||
self.assertEqual(local_file, device_info['_local_file'])
|
||||
|
||||
@mock.patch.object(vmedia.StaticDriver, '_get_device', autospec=True)
|
||||
@mock.patch.object(builtins, 'open', autospec=True)
|
||||
@mock.patch.object(vmedia.os, 'rename', autospec=True)
|
||||
@mock.patch.object(vmedia, 'tempfile', autospec=True)
|
||||
@mock.patch.object(vmedia, 'requests', autospec=True)
|
||||
def test_insert_image_fail(self, mock_requests, mock_tempfile, mock_rename,
|
||||
mock_open, mock_get_device):
|
||||
device_info = {}
|
||||
mock_get_device.return_value = device_info
|
||||
|
||||
mock_tempfile.mkdtemp.return_value = '/alphabet/soup'
|
||||
mock_tempfile.gettempdir.return_value = '/tmp'
|
||||
mock_tmp_file = (mock_tempfile.NamedTemporaryFile
|
||||
.return_value.__enter__.return_value)
|
||||
mock_tmp_file.name = 'alphabet.soup'
|
||||
mock_rsp = mock_requests.get.return_value.__enter__.return_value
|
||||
mock_rsp.headers = {
|
||||
'content-disposition': 'attachment; filename="fish.iso"'
|
||||
}
|
||||
mock_rsp.status_code = 401
|
||||
|
||||
self.assertRaises(error.FishyError,
|
||||
self.test_driver.insert_image,
|
||||
self.UUID, 'Cd', 'http://fish.it/red.iso',
|
||||
inserted=True, write_protected=False)
|
||||
mock_requests.get.assert_called_once_with(
|
||||
'http://fish.it/red.iso', stream=True, verify=True)
|
||||
mock_open.assert_not_called()
|
||||
self.assertEqual({}, device_info)
|
||||
|
||||
@mock.patch.object(vmedia.StaticDriver, '_get_device', autospec=True)
|
||||
@mock.patch.object(vmedia.os, 'unlink', autospec=True)
|
||||
def test_eject_image(self, mock_unlink, mock_get_device):
|
||||
|
||||
Reference in New Issue
Block a user