Skip to content

Commit c623eab

Browse files
Zuulopenstack-gerrit
authored andcommitted
Merge "Change force_format strategy to catch mismatches" into unmaintained/zed
2 parents 8011595 + a541252 commit c623eab

File tree

3 files changed

+108
-73
lines changed

3 files changed

+108
-73
lines changed

nova/tests/unit/virt/libvirt/test_utils.py

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -405,12 +405,12 @@ def test_fetch_initrd_image(self, mock_images):
405405
_context, image_id, target, trusted_certs)
406406

407407
@mock.patch.object(images, 'IMAGE_API')
408-
@mock.patch.object(format_inspector, 'get_inspector')
408+
@mock.patch.object(format_inspector, 'detect_file_format')
409409
@mock.patch.object(compute_utils, 'disk_ops_semaphore')
410410
@mock.patch('nova.privsep.utils.supports_direct_io', return_value=True)
411411
@mock.patch('nova.privsep.qemu.unprivileged_convert_image')
412412
def test_fetch_raw_image(self, mock_convert_image, mock_direct_io,
413-
mock_disk_op_sema, mock_gi, mock_glance):
413+
mock_disk_op_sema, mock_detect, mock_glance):
414414

415415
def fake_rename(old, new):
416416
self.executes.append(('mv', old, new))
@@ -450,7 +450,7 @@ class FakeImgInfo(object):
450450
self.stub_out('oslo_utils.fileutils.delete_if_exists',
451451
fake_rm_on_error)
452452

453-
mock_inspector = mock_gi.return_value.from_file.return_value
453+
mock_inspector = mock_detect.return_value
454454

455455
# Since the remove param of fileutils.remove_path_on_error()
456456
# is initialized at load time, we must provide a wrapper
@@ -464,6 +464,7 @@ class FakeImgInfo(object):
464464

465465
# Make sure qcow2 gets converted to raw
466466
mock_inspector.safety_check.return_value = True
467+
mock_inspector.__str__.return_value = 'qcow2'
467468
mock_glance.get.return_value = {'disk_format': 'qcow2'}
468469
target = 't.qcow2'
469470
self.executes = []
@@ -477,12 +478,13 @@ class FakeImgInfo(object):
477478
CONF.instances_path, False)
478479
mock_convert_image.reset_mock()
479480
mock_inspector.safety_check.assert_called_once_with()
480-
mock_gi.assert_called_once_with('qcow2')
481+
mock_detect.assert_called_once_with('t.qcow2.part')
481482

482483
# Make sure raw does not get converted
483-
mock_gi.reset_mock()
484+
mock_detect.reset_mock()
484485
mock_inspector.safety_check.reset_mock()
485486
mock_inspector.safety_check.return_value = True
487+
mock_inspector.__str__.return_value = 'raw'
486488
mock_glance.get.return_value = {'disk_format': 'raw'}
487489
target = 't.raw'
488490
self.executes = []
@@ -491,12 +493,13 @@ class FakeImgInfo(object):
491493
self.assertEqual(self.executes, expected_commands)
492494
mock_convert_image.assert_not_called()
493495
mock_inspector.safety_check.assert_called_once_with()
494-
mock_gi.assert_called_once_with('raw')
496+
mock_detect.assert_called_once_with('t.raw.part')
495497

496498
# Make sure safety check failure prevents us from proceeding
497-
mock_gi.reset_mock()
499+
mock_detect.reset_mock()
498500
mock_inspector.safety_check.reset_mock()
499501
mock_inspector.safety_check.return_value = False
502+
mock_inspector.__str__.return_value = 'qcow2'
500503
mock_glance.get.return_value = {'disk_format': 'qcow2'}
501504
target = 'backing.qcow2'
502505
self.executes = []
@@ -506,10 +509,10 @@ class FakeImgInfo(object):
506509
self.assertEqual(self.executes, expected_commands)
507510
mock_convert_image.assert_not_called()
508511
mock_inspector.safety_check.assert_called_once_with()
509-
mock_gi.assert_called_once_with('qcow2')
512+
mock_detect.assert_called_once_with('backing.qcow2.part')
510513

511514
# Make sure a format mismatch prevents us from proceeding
512-
mock_gi.reset_mock()
515+
mock_detect.reset_mock()
513516
mock_inspector.safety_check.reset_mock()
514517
mock_inspector.safety_check.side_effect = (
515518
format_inspector.ImageFormatError)
@@ -522,7 +525,7 @@ class FakeImgInfo(object):
522525
self.assertEqual(self.executes, expected_commands)
523526
mock_convert_image.assert_not_called()
524527
mock_inspector.safety_check.assert_called_once_with()
525-
mock_gi.assert_called_once_with('qcow2')
528+
mock_detect.assert_called_once_with('backing.qcow2.part')
526529

527530
del self.executes
528531

nova/tests/unit/virt/test_images.py

Lines changed: 55 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
from nova.compute import utils as compute_utils
2323
from nova import exception
24-
from nova.image import format_inspector
2524
from nova import test
2625
from nova.virt import images
2726

@@ -101,15 +100,16 @@ def test_qemu_img_info_with_disk_not_found(self, exists, mocked_execute):
101100
mocked_execute.assert_called_once()
102101

103102
@mock.patch.object(images, 'IMAGE_API')
104-
@mock.patch('nova.image.format_inspector.get_inspector')
103+
@mock.patch('nova.image.format_inspector.detect_file_format')
105104
@mock.patch.object(images, 'convert_image',
106105
side_effect=exception.ImageUnacceptable)
107106
@mock.patch.object(images, 'qemu_img_info')
108107
@mock.patch.object(images, 'fetch')
109108
def test_fetch_to_raw_errors(self, convert_image, qemu_img_info, fetch,
110-
get_inspector, glance):
111-
inspector = get_inspector.return_value.from_file.return_value
109+
mock_detect, glance):
110+
inspector = mock_detect.return_value
112111
inspector.safety_check.return_value = True
112+
inspector.__str__.return_value = 'qcow2'
113113
glance.get.return_value = {'disk_format': 'qcow2'}
114114
qemu_img_info.backing_file = None
115115
qemu_img_info.file_format = 'qcow2'
@@ -120,16 +120,17 @@ def test_fetch_to_raw_errors(self, convert_image, qemu_img_info, fetch,
120120
None, 'href123', '/no/path')
121121

122122
@mock.patch.object(images, 'IMAGE_API')
123-
@mock.patch('nova.image.format_inspector.get_inspector')
123+
@mock.patch('nova.image.format_inspector.detect_file_format')
124124
@mock.patch.object(images, 'convert_image',
125125
side_effect=exception.ImageUnacceptable)
126126
@mock.patch.object(images, 'qemu_img_info')
127127
@mock.patch.object(images, 'fetch')
128128
def test_fetch_to_raw_data_file(self, convert_image, qemu_img_info_fn,
129-
fetch, mock_gi, mock_glance):
129+
fetch, mock_detect, mock_glance):
130130
mock_glance.get.return_value = {'disk_format': 'qcow2'}
131-
inspector = mock_gi.return_value.from_file.return_value
131+
inspector = mock_detect.return_value
132132
inspector.safety_check.return_value = True
133+
inspector.__str__.return_value = 'qcow2'
133134
# NOTE(danms): the above test needs the following line as well, as it
134135
# is broken without it.
135136
qemu_img_info = qemu_img_info_fn.return_value
@@ -142,16 +143,17 @@ def test_fetch_to_raw_data_file(self, convert_image, qemu_img_info_fn,
142143
images.fetch_to_raw,
143144
None, 'href123', '/no/path')
144145

145-
@mock.patch('nova.image.format_inspector.get_inspector')
146+
@mock.patch('nova.image.format_inspector.detect_file_format')
146147
@mock.patch.object(images, 'IMAGE_API')
147148
@mock.patch('os.rename')
148149
@mock.patch.object(images, 'qemu_img_info')
149150
@mock.patch.object(images, 'fetch')
150151
def test_fetch_to_raw_from_raw(self, fetch, qemu_img_info_fn, mock_rename,
151-
mock_glance, mock_gi):
152+
mock_glance, mock_detect):
152153
# Make sure we support a case where we fetch an already-raw image and
153154
# qemu-img returns None for "format_specific".
154155
mock_glance.get.return_value = {'disk_format': 'raw'}
156+
mock_detect.return_value.__str__.return_value = 'raw'
155157
qemu_img_info = qemu_img_info_fn.return_value
156158
qemu_img_info.file_format = 'raw'
157159
qemu_img_info.backing_file = None
@@ -215,14 +217,15 @@ def test_convert_image_vmdk_allowed_list_checking(self):
215217
format='json'))
216218

217219
@mock.patch.object(images, 'IMAGE_API')
218-
@mock.patch('nova.image.format_inspector.get_inspector')
220+
@mock.patch('nova.image.format_inspector.detect_file_format')
219221
@mock.patch.object(images, 'fetch')
220222
@mock.patch('nova.privsep.qemu.unprivileged_qemu_img_info')
221-
def test_fetch_checks_vmdk_rules(self, mock_info, mock_fetch, mock_gi,
223+
def test_fetch_checks_vmdk_rules(self, mock_info, mock_fetch, mock_detect,
222224
mock_glance):
223225
mock_glance.get.return_value = {'disk_format': 'vmdk'}
224-
inspector = mock_gi.return_value.from_file.return_value
226+
inspector = mock_detect.return_value
225227
inspector.safety_check.return_value = True
228+
inspector.__str__.return_value = 'vmdk'
226229
info = {'format': 'vmdk',
227230
'format-specific': {
228231
'type': 'vmdk',
@@ -238,13 +241,17 @@ def test_fetch_checks_vmdk_rules(self, mock_info, mock_fetch, mock_gi,
238241
@mock.patch('os.rename')
239242
@mock.patch.object(images, 'IMAGE_API')
240243
@mock.patch('nova.image.format_inspector.get_inspector')
244+
@mock.patch('nova.image.format_inspector.detect_file_format')
241245
@mock.patch.object(images, 'fetch')
242246
@mock.patch('nova.privsep.qemu.unprivileged_qemu_img_info')
243-
def test_fetch_iso_is_raw(self, mock_info, mock_fetch, mock_gi,
244-
mock_glance, mock_rename):
247+
def test_fetch_iso_is_raw(
248+
self, mock_info, mock_fetch, mock_detect_file_format, mock_gi,
249+
mock_glance, mock_rename):
245250
mock_glance.get.return_value = {'disk_format': 'iso'}
246251
inspector = mock_gi.return_value.from_file.return_value
247252
inspector.safety_check.return_value = True
253+
inspector.__str__.return_value = 'iso'
254+
mock_detect_file_format.return_value = inspector
248255
# qemu-img does not have a parser for iso so it is treated as raw
249256
info = {
250257
"virtual-size": 356352,
@@ -258,27 +265,27 @@ def test_fetch_iso_is_raw(self, mock_info, mock_fetch, mock_gi,
258265
images.fetch_to_raw(None, 'foo', 'anypath')
259266
# Make sure we called info with -f raw for an iso, since qemu-img does
260267
# not support iso
261-
mock_info.assert_called_once_with('anypath.part', format='raw')
268+
mock_info.assert_called_once_with('anypath.part', format=None)
262269
# Make sure that since we considered this to be a raw file, we did the
263270
# just-rename-don't-convert path
264271
mock_rename.assert_called_once_with('anypath.part', 'anypath')
265272

266273
@mock.patch.object(images, 'IMAGE_API')
267-
@mock.patch('nova.image.format_inspector.get_inspector')
274+
@mock.patch('nova.image.format_inspector.detect_file_format')
268275
@mock.patch.object(images, 'qemu_img_info')
269276
@mock.patch.object(images, 'fetch')
270-
def test_fetch_to_raw_inspector(self, fetch, qemu_img_info, mock_gi,
277+
def test_fetch_to_raw_inspector(self, fetch, qemu_img_info, mock_detect,
271278
mock_glance):
272279
# Image claims to be qcow2, is qcow2, but fails safety check, so we
273280
# abort before qemu-img-info
274281
mock_glance.get.return_value = {'disk_format': 'qcow2'}
275-
inspector = mock_gi.return_value.from_file.return_value
282+
inspector = mock_detect.return_value
276283
inspector.safety_check.return_value = False
284+
inspector.__str__.return_value = 'qcow2'
277285
self.assertRaises(exception.ImageUnacceptable,
278286
images.fetch_to_raw, None, 'href123', '/no.path')
279287
qemu_img_info.assert_not_called()
280-
mock_gi.assert_called_once_with('qcow2')
281-
mock_gi.return_value.from_file.assert_called_once_with('/no.path.part')
288+
mock_detect.assert_called_once_with('/no.path.part')
282289
inspector.safety_check.assert_called_once_with()
283290
mock_glance.get.assert_called_once_with(None, 'href123')
284291

@@ -292,18 +299,17 @@ def test_fetch_to_raw_inspector(self, fetch, qemu_img_info, mock_gi,
292299
# Image claims to be qcow2 in glance, but the image is something else,
293300
# so we abort before qemu-img-info
294301
qemu_img_info.reset_mock()
295-
mock_gi.reset_mock()
302+
mock_detect.reset_mock()
296303
inspector.safety_check.reset_mock()
297-
mock_gi.return_value.from_file.side_effect = (
298-
format_inspector.ImageFormatError)
304+
mock_detect.return_value.__str__.return_value = 'vmdk'
299305
self.assertRaises(exception.ImageUnacceptable,
300306
images.fetch_to_raw, None, 'href123', '/no.path')
301-
mock_gi.assert_called_once_with('qcow2')
302-
inspector.safety_check.assert_not_called()
307+
mock_detect.assert_called_once_with('/no.path.part')
308+
inspector.safety_check.assert_called_once_with()
303309
qemu_img_info.assert_not_called()
304310

305311
@mock.patch.object(images, 'IMAGE_API')
306-
@mock.patch('nova.image.format_inspector.get_inspector')
312+
@mock.patch('nova.image.format_inspector.detect_file_format')
307313
@mock.patch.object(images, 'qemu_img_info')
308314
@mock.patch.object(images, 'fetch')
309315
def test_fetch_to_raw_inspector_disabled(self, fetch, qemu_img_info,
@@ -316,36 +322,41 @@ def test_fetch_to_raw_inspector_disabled(self, fetch, qemu_img_info,
316322
# If deep inspection is disabled, we should never call the inspector
317323
mock_gi.assert_not_called()
318324
# ... and we let qemu-img detect the format itself.
319-
qemu_img_info.assert_called_once_with('/no.path.part',
320-
format=None)
325+
qemu_img_info.assert_called_once_with('/no.path.part')
321326
mock_glance.get.assert_not_called()
322327

323328
@mock.patch.object(images, 'IMAGE_API')
324329
@mock.patch.object(images, 'qemu_img_info')
325-
def test_fetch_inspect_ami(self, imginfo, glance):
330+
@mock.patch('nova.image.format_inspector.detect_file_format')
331+
def test_fetch_inspect_ami(self, detect, imginfo, glance):
326332
glance.get.return_value = {'disk_format': 'ami'}
333+
detect.return_value.__str__.return_value = 'raw'
327334
self.assertRaises(exception.ImageUnacceptable,
328335
images.fetch_to_raw, None, 'href123', '/no.path')
329336
# Make sure 'ami was translated into 'raw' before we call qemu-img
330-
imginfo.assert_called_once_with('/no.path.part', format='raw')
337+
imginfo.assert_called_once_with('/no.path.part')
331338

332339
@mock.patch.object(images, 'IMAGE_API')
333340
@mock.patch.object(images, 'qemu_img_info')
334-
def test_fetch_inspect_aki(self, imginfo, glance):
341+
@mock.patch('nova.image.format_inspector.detect_file_format')
342+
def test_fetch_inspect_aki(self, detect, imginfo, glance):
335343
glance.get.return_value = {'disk_format': 'aki'}
344+
detect.return_value.__str__.return_value = 'raw'
336345
self.assertRaises(exception.ImageUnacceptable,
337346
images.fetch_to_raw, None, 'href123', '/no.path')
338347
# Make sure 'aki was translated into 'raw' before we call qemu-img
339-
imginfo.assert_called_once_with('/no.path.part', format='raw')
348+
imginfo.assert_called_once_with('/no.path.part')
340349

341350
@mock.patch.object(images, 'IMAGE_API')
342351
@mock.patch.object(images, 'qemu_img_info')
343-
def test_fetch_inspect_ari(self, imginfo, glance):
352+
@mock.patch('nova.image.format_inspector.detect_file_format')
353+
def test_fetch_inspect_ari(self, detect, imginfo, glance):
344354
glance.get.return_value = {'disk_format': 'ari'}
355+
detect.return_value.__str__.return_value = 'raw'
345356
self.assertRaises(exception.ImageUnacceptable,
346357
images.fetch_to_raw, None, 'href123', '/no.path')
347358
# Make sure 'aki was translated into 'raw' before we call qemu-img
348-
imginfo.assert_called_once_with('/no.path.part', format='raw')
359+
imginfo.assert_called_once_with('/no.path.part')
349360

350361
@mock.patch.object(images, 'IMAGE_API')
351362
@mock.patch.object(images, 'qemu_img_info')
@@ -358,13 +369,16 @@ def test_fetch_inspect_unknown_format(self, imginfo, glance):
358369

359370
@mock.patch.object(images, 'IMAGE_API')
360371
@mock.patch.object(images, 'qemu_img_info')
361-
@mock.patch('nova.image.format_inspector.get_inspector')
362-
def test_fetch_inspect_disagrees_qemu(self, mock_gi, imginfo, glance):
372+
@mock.patch('nova.image.format_inspector.detect_file_format')
373+
def test_fetch_inspect_disagrees_qemu(self, mock_detect, imginfo, glance):
363374
glance.get.return_value = {'disk_format': 'qcow2'}
375+
mock_detect.return_value.__str__.return_value = 'qcow2'
364376
# Glance and inspector think it is a qcow2 file, but qemu-img does not
365-
# agree. It was forced to interpret as a qcow2, but returned no
366-
# format information as a result.
377+
# agree.
367378
imginfo.return_value.data_file = None
368-
self.assertRaises(exception.ImageUnacceptable,
369-
images.fetch_to_raw, None, 'href123', '/no.path')
370-
imginfo.assert_called_once_with('/no.path.part', format='qcow2')
379+
imginfo.return_value.file_format = 'vmdk'
380+
ex = self.assertRaises(exception.ImageUnacceptable,
381+
images.fetch_to_raw,
382+
None, 'href123', '/no.path')
383+
self.assertIn('content does not match disk_format', str(ex))
384+
imginfo.assert_called_once_with('/no.path.part')

0 commit comments

Comments
 (0)