Propagate qemu-img errors to compute manager
When qemu-img is called with oslo_concurrency.process_utils.execute the ProcessExecutionError was raised when qemu-img either fails to execute or has a non-zero exit code. This error did not propagate up to the compute manager with any meaningful information meaning that if an instance build fails the error message is the generic "There are not enough hosts available". This change captures ProcessExecutionError and re-raises the exception as either InvalidDiskInfo (in qemu_img_info) or ImageUnacceptable (in convert_image and fetch_to_raw) and makes the manager accept this as a cause for a BuildAbortException on the logic that if the image is bad, things are dire, let's bail. Based on the code in qemu_img_info it appears there was a misunderstanding of how process_utils.execute behaves so it seems likely this problem is present elsewhere in the code. This change attempts to only address the issue as it shows up on the new instance path described in the related bug. Change-Id: I4fa1c258db58c70dfbf0178b7bb13978fda3a11f Closes-Bug: #1436166
This commit is contained in:
parent
f1da349a4f
commit
9a4ecfd96d
@ -2068,7 +2068,8 @@ class ComputeManager(manager.Manager):
|
||||
except (exception.FlavorDiskTooSmall,
|
||||
exception.FlavorMemoryTooSmall,
|
||||
exception.ImageNotActive,
|
||||
exception.ImageUnacceptable) as e:
|
||||
exception.ImageUnacceptable,
|
||||
exception.InvalidDiskInfo) as e:
|
||||
self._notify_about_instance_usage(context, instance,
|
||||
'create.error', fault=e)
|
||||
raise exception.BuildAbortException(instance_uuid=instance.uuid,
|
||||
|
@ -3536,6 +3536,10 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
|
||||
exception.ImageUnacceptable(image_id=self.image.get('id'),
|
||||
reason=""))
|
||||
|
||||
def test_build_and_run_invalid_disk_info_exception(self):
|
||||
self._test_build_and_run_spawn_exceptions(
|
||||
exception.InvalidDiskInfo(reason=""))
|
||||
|
||||
def _test_build_and_run_spawn_exceptions(self, exc):
|
||||
with test.nested(
|
||||
mock.patch.object(self.compute.driver, 'spawn',
|
||||
|
@ -31,7 +31,7 @@ class QemuTestCase(test.NoDBTestCase):
|
||||
|
||||
@mock.patch.object(os.path, 'exists', return_value=True)
|
||||
def test_qemu_info_with_errors(self, path_exists):
|
||||
self.assertRaises(processutils.ProcessExecutionError,
|
||||
self.assertRaises(exception.InvalidDiskInfo,
|
||||
images.qemu_img_info,
|
||||
'/fake/path')
|
||||
|
||||
@ -43,3 +43,26 @@ class QemuTestCase(test.NoDBTestCase):
|
||||
image_info = images.qemu_img_info('/fake/path')
|
||||
self.assertTrue(image_info)
|
||||
self.assertTrue(str(image_info))
|
||||
|
||||
@mock.patch.object(utils, 'execute',
|
||||
side_effect=processutils.ProcessExecutionError)
|
||||
def test_convert_image_with_errors(self, mocked_execute):
|
||||
self.assertRaises(exception.ImageUnacceptable,
|
||||
images.convert_image,
|
||||
'/path/that/does/not/exist',
|
||||
'/other/path/that/does/not/exist',
|
||||
'qcow2',
|
||||
'raw')
|
||||
|
||||
@mock.patch.object(images, 'convert_image',
|
||||
side_effect=exception.ImageUnacceptable)
|
||||
@mock.patch.object(images, 'qemu_img_info')
|
||||
@mock.patch.object(images, 'fetch')
|
||||
def test_fetch_to_raw_errors(self, convert_image, qemu_img_info, fetch):
|
||||
qemu_img_info.backing_file = None
|
||||
qemu_img_info.file_format = 'qcow2'
|
||||
qemu_img_info.virtual_size = 20
|
||||
self.assertRaisesRegex(exception.ImageUnacceptable,
|
||||
'Image href123 is unacceptable.*',
|
||||
images.fetch_to_raw,
|
||||
None, 'href123', '/no/path', None, None)
|
||||
|
@ -21,6 +21,7 @@ Handling of VM disk images.
|
||||
|
||||
import os
|
||||
|
||||
from oslo_concurrency import processutils
|
||||
from oslo_config import cfg
|
||||
from oslo_log import log as logging
|
||||
from oslo_utils import fileutils
|
||||
@ -56,8 +57,14 @@ def qemu_img_info(path):
|
||||
msg = (_("Path does not exist %(path)s") % {'path': path})
|
||||
raise exception.InvalidDiskInfo(reason=msg)
|
||||
|
||||
out, err = utils.execute('env', 'LC_ALL=C', 'LANG=C',
|
||||
'qemu-img', 'info', path)
|
||||
try:
|
||||
out, err = utils.execute('env', 'LC_ALL=C', 'LANG=C',
|
||||
'qemu-img', 'info', path)
|
||||
except processutils.ProcessExecutionError as exp:
|
||||
msg = (_("qemu-img failed to execute on %(path)s : %(exp)s") %
|
||||
{'path': path, 'exp': exp})
|
||||
raise exception.InvalidDiskInfo(reason=msg)
|
||||
|
||||
if not out:
|
||||
msg = (_("Failed to run qemu-img info on %(path)s : %(error)s") %
|
||||
{'path': path, 'error': err})
|
||||
@ -91,7 +98,12 @@ def _convert_image(source, dest, in_format, out_format, run_as_root):
|
||||
cmd = ('qemu-img', 'convert', '-O', out_format, source, dest)
|
||||
if in_format is not None:
|
||||
cmd = cmd + ('-f', in_format)
|
||||
utils.execute(*cmd, run_as_root=run_as_root)
|
||||
try:
|
||||
utils.execute(*cmd, run_as_root=run_as_root)
|
||||
except processutils.ProcessExecutionError as exp:
|
||||
msg = (_("Unable to convert image to %(format)s: %(exp)s") %
|
||||
{'format': out_format, 'exp': exp})
|
||||
raise exception.ImageUnacceptable(image_id=source, reason=msg)
|
||||
|
||||
|
||||
def fetch(context, image_href, path, _user_id, _project_id, max_size=0):
|
||||
@ -145,7 +157,14 @@ def fetch_to_raw(context, image_href, path, user_id, project_id, max_size=0):
|
||||
staged = "%s.converted" % path
|
||||
LOG.debug("%s was %s, converting to raw" % (image_href, fmt))
|
||||
with fileutils.remove_path_on_error(staged):
|
||||
convert_image(path_tmp, staged, fmt, 'raw')
|
||||
try:
|
||||
convert_image(path_tmp, staged, fmt, 'raw')
|
||||
except exception.ImageUnacceptable as exp:
|
||||
# re-raise to include image_href
|
||||
raise exception.ImageUnacceptable(image_id=image_href,
|
||||
reason=_("Unable to convert image to raw: %(exp)s")
|
||||
% {'exp': exp})
|
||||
|
||||
os.unlink(path_tmp)
|
||||
|
||||
data = qemu_img_info(staged)
|
||||
|
Loading…
Reference in New Issue
Block a user