checksum: Refactor creating checksum files

Instead of iterating over the images metadata and appending the checksum
to relevant files immediately, we should store them and write only once.

This avoid an issue when the same image is mentioned in the metadata
multiple times. This happens for source images that are listed under
each binary arch.

The unified isos script is updated to use the exact same logic and code.
This also uncovered a problem with the metadata for debuginfo unified
isos: their paths in metadata were incorrect, which lead to missing
checksums.

Fixes: https://pagure.io/pungi/issue/667
Fixes: https://pagure.io/pungi/issue/668
Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>
This commit is contained in:
Lubomír Sedlář 2017-07-12 14:52:40 +02:00
parent a831d65c40
commit 81cb0952ca
5 changed files with 116 additions and 293 deletions

View file

@ -94,7 +94,7 @@ class DummyCompose(object):
self.log_debug = mock.Mock()
self.log_warning = mock.Mock()
self.get_image_name = mock.Mock(return_value='image-name')
self.image = mock.Mock(path='Client/i386/iso/image.iso', can_fail=False)
self.image = mock.Mock(path='Client/i386/iso/image.iso', can_fail=False, size=123)
self.im = mock.Mock(images={'Client': {'amd64': [self.image]}})
self.old_composes = []
self.config_dir = '/home/releng/config'

View file

@ -14,7 +14,7 @@ import shutil
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
from pungi.phases.image_checksum import ImageChecksumPhase, dump_checksums, dump_filesizes
from pungi.phases.image_checksum import ImageChecksumPhase, dump_checksums
from tests.helpers import DummyCompose, PungiTestCase
@ -38,9 +38,8 @@ class TestImageChecksumPhase(PungiTestCase):
@mock.patch('os.path.exists')
@mock.patch('kobo.shortcuts.compute_file_checksums')
@mock.patch('pungi.phases.image_checksum.dump_filesizes')
@mock.patch('pungi.phases.image_checksum.dump_checksums')
def test_checksum_one_file(self, dump_checksums, dump_filesizes, cc, exists):
def test_checksum_one_file(self, dump_checksums, cc, exists):
compose = DummyCompose(self.topdir, {
'media_checksums': ['sha256'],
'media_checksum_one_file': True,
@ -53,15 +52,15 @@ class TestImageChecksumPhase(PungiTestCase):
phase.run()
dump_checksums.assert_called_once_with(self.topdir + '/compose/Client/i386/iso', 'sha256', {'image.iso': 'cafebabe'}, 'CHECKSUM')
dump_checksums.assert_called_once_with(self.topdir + '/compose/Client/i386/iso/CHECKSUM',
set([('image.iso', 123, 'sha256', 'cafebabe')]))
cc.assert_called_once_with(self.topdir + '/compose/Client/i386/iso/image.iso', ['sha256'])
compose.image.add_checksum.assert_called_once_with(None, 'sha256', 'cafebabe')
@mock.patch('os.path.exists')
@mock.patch('kobo.shortcuts.compute_file_checksums')
@mock.patch('pungi.phases.image_checksum.dump_filesizes')
@mock.patch('pungi.phases.image_checksum.dump_checksums')
def test_checksum_save_individuals(self, dump_checksums, dump_filesizes, cc, exists):
def test_checksum_save_individuals(self, dump_checksums, cc, exists):
compose = DummyCompose(self.topdir, {
'media_checksums': ['md5', 'sha256'],
})
@ -74,14 +73,14 @@ class TestImageChecksumPhase(PungiTestCase):
phase.run()
dump_checksums.assert_has_calls(
[mock.call(self.topdir + '/compose/Client/i386/iso', 'md5',
{'image.iso': 'cafebabe'}, 'image.iso.MD5SUM'),
mock.call(self.topdir + '/compose/Client/i386/iso', 'sha256',
{'image.iso': 'deadbeef'}, 'image.iso.SHA256SUM'),
mock.call(self.topdir + '/compose/Client/i386/iso', 'md5',
{'image.iso': 'cafebabe'}, 'MD5SUM'),
mock.call(self.topdir + '/compose/Client/i386/iso', 'sha256',
{'image.iso': 'deadbeef'}, 'SHA256SUM')],
[mock.call(self.topdir + '/compose/Client/i386/iso/image.iso.MD5SUM',
set([('image.iso', 123, 'md5', 'cafebabe')])),
mock.call(self.topdir + '/compose/Client/i386/iso/image.iso.SHA256SUM',
set([('image.iso', 123, 'sha256', 'deadbeef')])),
mock.call(self.topdir + '/compose/Client/i386/iso/MD5SUM',
set([('image.iso', 123, 'md5', 'cafebabe')])),
mock.call(self.topdir + '/compose/Client/i386/iso/SHA256SUM',
set([('image.iso', 123, 'sha256', 'deadbeef')]))],
any_order=True
)
cc.assert_called_once_with(self.topdir + '/compose/Client/i386/iso/image.iso', ['md5', 'sha256'])
@ -91,9 +90,8 @@ class TestImageChecksumPhase(PungiTestCase):
@mock.patch('os.path.exists')
@mock.patch('kobo.shortcuts.compute_file_checksums')
@mock.patch('pungi.phases.image_checksum.dump_filesizes')
@mock.patch('pungi.phases.image_checksum.dump_checksums')
def test_checksum_one_file_custom_name(self, dump_checksums, dump_filesizes, cc, exists):
def test_checksum_one_file_custom_name(self, dump_checksums, cc, exists):
compose = DummyCompose(self.topdir, {
'media_checksums': ['sha256'],
'media_checksum_one_file': True,
@ -108,17 +106,16 @@ class TestImageChecksumPhase(PungiTestCase):
phase.run()
dump_checksums.assert_called_once_with(self.topdir + '/compose/Client/i386/iso', 'sha256',
{'image.iso': 'cafebabe'},
'test-Client-1.0-20151203.t.0_Alpha-1.0-CHECKSUM')
dump_checksums.assert_called_once_with(
self.topdir + '/compose/Client/i386/iso/test-Client-1.0-20151203.t.0_Alpha-1.0-CHECKSUM',
set([('image.iso', 123, 'sha256', 'cafebabe')]))
cc.assert_called_once_with(self.topdir + '/compose/Client/i386/iso/image.iso', ['sha256'])
compose.image.add_checksum.assert_called_once_with(None, 'sha256', 'cafebabe')
@mock.patch('os.path.exists')
@mock.patch('kobo.shortcuts.compute_file_checksums')
@mock.patch('pungi.phases.image_checksum.dump_filesizes')
@mock.patch('pungi.phases.image_checksum.dump_checksums')
def test_checksum_save_individuals_custom_name(self, dump_checksums, dump_filesizes, cc, exists):
def test_checksum_save_individuals_custom_name(self, dump_checksums, cc, exists):
compose = DummyCompose(self.topdir, {
'media_checksums': ['md5', 'sha256'],
'media_checksum_base_filename': '%(release_short)s-%(variant)s-%(version)s-%(date)s%(type_suffix)s.%(respin)s'
@ -132,14 +129,14 @@ class TestImageChecksumPhase(PungiTestCase):
phase.run()
dump_checksums.assert_has_calls(
[mock.call(self.topdir + '/compose/Client/i386/iso', 'md5',
{'image.iso': 'cafebabe'}, 'image.iso.MD5SUM'),
mock.call(self.topdir + '/compose/Client/i386/iso', 'sha256',
{'image.iso': 'deadbeef'}, 'image.iso.SHA256SUM'),
mock.call(self.topdir + '/compose/Client/i386/iso', 'md5', {'image.iso': 'cafebabe'},
'test-Client-1.0-20151203.t.0-MD5SUM'),
mock.call(self.topdir + '/compose/Client/i386/iso', 'sha256', {'image.iso': 'deadbeef'},
'test-Client-1.0-20151203.t.0-SHA256SUM')],
[mock.call(self.topdir + '/compose/Client/i386/iso/image.iso.MD5SUM',
set([('image.iso', 123, 'md5', 'cafebabe')])),
mock.call(self.topdir + '/compose/Client/i386/iso/image.iso.SHA256SUM',
set([('image.iso', 123, 'sha256', 'deadbeef')])),
mock.call(self.topdir + '/compose/Client/i386/iso/test-Client-1.0-20151203.t.0-MD5SUM',
set([('image.iso', 123, 'md5', 'cafebabe')])),
mock.call(self.topdir + '/compose/Client/i386/iso/test-Client-1.0-20151203.t.0-SHA256SUM',
set([('image.iso', 123, 'sha256', 'deadbeef')]))],
any_order=True
)
cc.assert_called_once_with(self.topdir + '/compose/Client/i386/iso/image.iso', ['md5', 'sha256'])
@ -149,9 +146,8 @@ class TestImageChecksumPhase(PungiTestCase):
@mock.patch('os.path.exists')
@mock.patch('kobo.shortcuts.compute_file_checksums')
@mock.patch('pungi.phases.image_checksum.dump_filesizes')
@mock.patch('pungi.phases.image_checksum.dump_checksums')
def test_checksum_save_individuals_custom_name_str_format(self, dump_checksums, dump_filesizes, cc, exists):
def test_checksum_save_individuals_custom_name_str_format(self, dump_checksums, cc, exists):
compose = DummyCompose(self.topdir, {
'media_checksums': ['md5', 'sha256'],
'media_checksum_base_filename': '{release_short}-{variant}-{version}-{date}{type_suffix}.{respin}'
@ -165,14 +161,14 @@ class TestImageChecksumPhase(PungiTestCase):
phase.run()
dump_checksums.assert_has_calls(
[mock.call(self.topdir + '/compose/Client/i386/iso', 'md5',
{'image.iso': 'cafebabe'}, 'image.iso.MD5SUM'),
mock.call(self.topdir + '/compose/Client/i386/iso', 'sha256',
{'image.iso': 'deadbeef'}, 'image.iso.SHA256SUM'),
mock.call(self.topdir + '/compose/Client/i386/iso', 'md5', {'image.iso': 'cafebabe'},
'test-Client-1.0-20151203.t.0-MD5SUM'),
mock.call(self.topdir + '/compose/Client/i386/iso', 'sha256', {'image.iso': 'deadbeef'},
'test-Client-1.0-20151203.t.0-SHA256SUM')],
[mock.call(self.topdir + '/compose/Client/i386/iso/image.iso.MD5SUM',
set([('image.iso', 123, 'md5', 'cafebabe')])),
mock.call(self.topdir + '/compose/Client/i386/iso/image.iso.SHA256SUM',
set([('image.iso', 123, 'sha256', 'deadbeef')])),
mock.call(self.topdir + '/compose/Client/i386/iso/test-Client-1.0-20151203.t.0-MD5SUM',
set([('image.iso', 123, 'md5', 'cafebabe')])),
mock.call(self.topdir + '/compose/Client/i386/iso/test-Client-1.0-20151203.t.0-SHA256SUM',
set([('image.iso', 123, 'sha256', 'deadbeef')]))],
any_order=True
)
cc.assert_called_once_with(self.topdir + '/compose/Client/i386/iso/image.iso', ['md5', 'sha256'])
@ -180,67 +176,8 @@ class TestImageChecksumPhase(PungiTestCase):
mock.call(None, 'md5', 'cafebabe')],
any_order=True)
@mock.patch('os.path.exists')
@mock.patch('kobo.shortcuts.compute_file_checksums')
@mock.patch('pungi.phases.image_checksum.get_file_size')
@mock.patch('pungi.phases.image_checksum.dump_filesizes')
@mock.patch('pungi.phases.image_checksum.dump_checksums')
def test_dump_filesizes_one_file(self, dump_checksums, dump_filesizes, get_file_size, cc, exists):
compose = DummyCompose(self.topdir, {
'media_checksums': ['md5', 'sha256'],
'media_checksum_base_filename': '{release_short}-{variant}-{version}-{date}{type_suffix}.{respin}',
'media_checksum_one_file': True
})
compose.image.size = None
get_file_size.return_value = '12345'
phase = ImageChecksumPhase(compose)
exists.return_value = True
cc.return_value = {'md5': 'cafebabe', 'sha256': 'deadbeef'}
phase.run()
dump_filesizes.assert_called_once_with(
self.topdir + '/compose/Client/i386/iso', {'image.iso': '12345'}, 'test-Client-1.0-20151203.t.0-CHECKSUM')
get_file_size.assert_called_once_with(self.topdir + '/compose/Client/i386/iso/image.iso')
@mock.patch('os.path.exists')
@mock.patch('kobo.shortcuts.compute_file_checksums')
@mock.patch('pungi.phases.image_checksum.get_file_size')
@mock.patch('pungi.phases.image_checksum.dump_filesizes')
@mock.patch('pungi.phases.image_checksum.dump_checksums')
def test_dump_filesizes_save_individuals(self, dump_checksums, dump_filesizes, get_file_size, cc, exists):
compose = DummyCompose(self.topdir, {
'media_checksums': ['md5', 'sha256'],
'media_checksum_base_filename': '{release_short}-{variant}-{version}-{date}{type_suffix}.{respin}'
})
compose.image.size = None
get_file_size.return_value = '12345'
phase = ImageChecksumPhase(compose)
exists.return_value = True
cc.return_value = {'md5': 'cafebabe', 'sha256': 'deadbeef'}
phase.run()
dump_filesizes.assert_has_calls(
[mock.call(self.topdir + '/compose/Client/i386/iso',
{'image.iso': '12345'}, 'image.iso.SHA256SUM'),
mock.call(self.topdir + '/compose/Client/i386/iso',
{'image.iso': '12345'}, 'image.iso.MD5SUM'),
mock.call(self.topdir + '/compose/Client/i386/iso',
{'image.iso': '12345'}, 'test-Client-1.0-20151203.t.0-SHA256SUM'),
mock.call(self.topdir + '/compose/Client/i386/iso',
{'image.iso': '12345'}, 'test-Client-1.0-20151203.t.0-MD5SUM')],
any_order=True
)
get_file_size.assert_called_once_with(self.topdir + '/compose/Client/i386/iso/image.iso')
class TestChecksums(unittest.TestCase):
class TestDumpChecksums(unittest.TestCase):
def setUp(self):
self.tmp_dir = tempfile.mkdtemp()
@ -248,39 +185,20 @@ class TestChecksums(unittest.TestCase):
shutil.rmtree(self.tmp_dir)
def test_dump_checksums(self):
dump_checksums(self.tmp_dir,
'md5',
{'file1.iso': 'abcdef', 'file2.iso': 'cafebabe'},
'CHECKSUM')
with open(os.path.join(self.tmp_dir, 'CHECKSUM'), 'r') as f:
data = f.read().rstrip().split('\n')
expected = [
'MD5 (file1.iso) = abcdef',
'MD5 (file2.iso) = cafebabe',
]
self.assertItemsEqual(expected, data)
class TestDumpFilesizes(unittest.TestCase):
def setUp(self):
self.tmp_dir = tempfile.mkdtemp()
def tearDown(self):
shutil.rmtree(self.tmp_dir)
def test_dump_files(self):
filesizes = {'file1.iso': 123,
'file2.iso': 456}
dump_filesizes(self.tmp_dir, filesizes, 'CHECKSUM')
dump_checksums(os.path.join(self.tmp_dir, 'CHECKSUM'),
[('file2.iso', 456, 'md5', 'cafebabe'),
('file1.iso', 123, 'md5', 'abcdef')])
with open(os.path.join(self.tmp_dir, 'CHECKSUM'), 'r') as f:
data = f.read().rstrip().split('\n')
expected = [
'# file1.iso: 123 bytes',
'MD5 (file1.iso) = abcdef',
'# file2.iso: 456 bytes',
'MD5 (file2.iso) = cafebabe',
]
self.assertItemsEqual(expected, data)
self.assertEqual(expected, data)
if __name__ == "__main__":
unittest.main()

View file

@ -1,7 +1,6 @@
#!/usr/bin/env python
# -*- coding: utf-8 -*-
import json
import mock
import os
import shutil
@ -36,6 +35,14 @@ class TestUnifiedIsos(PungiTestCase):
self.assertRegexpMatches(isos.temp_dir,
'^%s/' % os.path.join(self.topdir, COMPOSE_ID, 'work'))
def test_dump_manifest(self):
compose_path = os.path.join(self.topdir, COMPOSE_ID, 'compose')
isos = unified_isos.UnifiedISO(compose_path)
isos.compose._images = mock.Mock()
isos.dump_manifest()
self.assertEqual(isos.compose._images.mock_calls,
[mock.call.dump(compose_path + '/metadata/images.json')])
class TestCreate(PungiTestCase):
def setUp(self):
@ -47,7 +54,7 @@ class TestCreate(PungiTestCase):
def test_create_method(self):
methods = ('link_to_temp', 'createrepo', 'discinfo', 'createiso',
'link_to_compose', 'update_checksums')
'link_to_compose', 'update_checksums', 'dump_manifest')
for attr in methods:
setattr(self.isos, attr, mock.Mock())
@ -391,25 +398,22 @@ class TestCreateiso(PungiTestCase):
self.mkisofs_cmd = self.mkisofs_cmd or mock.Mock(name='mkisofs cmd')
return self.mkisofs_cmd
def _img(self, arch, exts):
exts = ['manifest'] + exts
def _img(self, arch):
base_path = os.path.join(self.isos.temp_dir, 'iso', arch,
u'DP-1.0-20161013.t.4-%s-dvd.iso' % arch)
yield base_path
for ext in exts:
yield base_path + '.' + ext
yield base_path + '.manifest'
def _imgs(self, arches, exts):
def _imgs(self, arches):
images = {}
exts = [e + 'SUM' for e in exts]
for arch in arches:
file_arch = arch
if arch.startswith('debug-'):
file_arch = arch.split('-', 1)[-1] + '-debuginfo'
images[arch] = set(self._img(file_arch if arch != 'src' else 'source', exts))
images[arch] = set(self._img(file_arch if arch != 'src' else 'source'))
return images
def assertResults(self, iso, run, arches, checksums):
def assertResults(self, iso, run, arches):
self.assertEqual(
run.mock_calls,
[mock.call(self.mkisofs_cmd),
@ -417,33 +421,19 @@ class TestCreateiso(PungiTestCase):
mock.call(iso.get_manifest_cmd.return_value)] * len(arches)
)
self.assertEqual(
self.isos.images,
self._imgs(arches, checksums),
)
self.assertEqual(self.isos.images, self._imgs(arches))
with open(os.path.join(self.compose_path, 'metadata', 'images.json')) as f:
manifest = json.load(f)
images = self.isos.compose.images
for v in ('Client', 'Server'):
for a in arches:
for image in manifest['payload']['images'][v]['x86_64']:
arch = iso_arch = 'source' if image['arch'] == 'src' else image['arch']
for image in images[v]['x86_64']:
arch = iso_arch = 'source' if image.arch == 'src' else image.arch
if a.startswith('debug-'):
iso_arch += '-debuginfo'
a = a.split('-', 1)[1]
path = '{0}/{1}/iso/DP-1.0-20161013.t.4-{1}-dvd.iso'.format(v, arch, iso_arch)
if image.get('unified', False) and image['arch'] == a and image['path'] == path:
checksum_file_base = os.path.join(self.isos.temp_dir, 'iso',
arch, os.path.basename(image['path']))
for ch in checksums:
fp = '%s.%sSUM' % (checksum_file_base, ch)
with open(fp) as f:
self.assertEqual(
f.read(),
'%s (%s) = %s\n' % (ch, os.path.basename(image['path']),
CHECKSUMS[ch])
)
if image.unified and image.arch == a and image.path == path:
break
else:
self.fail('Image for %s.%s missing' % (v, a))
@ -460,7 +450,7 @@ class TestCreateiso(PungiTestCase):
self.isos.createiso()
self.assertResults(iso, run, ['src', 'x86_64'], ['MD5', 'SHA1', 'SHA256'])
self.assertResults(iso, run, ['src', 'x86_64'])
@mock.patch('pungi_utils.unified_isos.iso')
@mock.patch('pungi_utils.unified_isos.run')
@ -475,39 +465,7 @@ class TestCreateiso(PungiTestCase):
self.isos.createiso()
self.assertResults(iso, run, ['src', 'x86_64', 'debug-x86_64'], ['MD5', 'SHA1', 'SHA256'])
@mock.patch('pungi_utils.unified_isos.iso')
@mock.patch('pungi_utils.unified_isos.run')
def test_createiso_checksum_one_file(self, run, iso):
iso.get_mkisofs_cmd.side_effect = self.mock_gmc
iso.get_implanted_md5.return_value = 'beefcafebabedeadbeefcafebabedead'
iso.get_volume_id.return_value = 'VOLID'
self.isos.conf['media_checksum_one_file'] = True
self.isos.treeinfo = {'x86_64': self.isos.treeinfo['x86_64'],
'src': self.isos.treeinfo['src']}
self.isos.createiso()
self.assertResults(iso, run, ['src', 'x86_64'], [])
@mock.patch('pungi_utils.unified_isos.iso')
@mock.patch('pungi_utils.unified_isos.run')
def test_createiso_single_checksum(self, run, iso):
iso.get_mkisofs_cmd.side_effect = self.mock_gmc
iso.get_implanted_md5.return_value = 'beefcafebabedeadbeefcafebabedead'
iso.get_volume_id.return_value = 'VOLID'
self.isos.conf['media_checksums'] = ['sha256']
self.isos.treeinfo = {'x86_64': self.isos.treeinfo['x86_64'],
'src': self.isos.treeinfo['src']}
self.isos.createiso()
self.assertResults(iso, run, ['src', 'x86_64'], ['SHA256'])
self.assertResults(iso, run, ['src', 'x86_64', 'debug-x86_64'])
class TestLinkToCompose(PungiTestCase):
@ -575,15 +533,9 @@ class TestUpdateChecksums(PungiTestCase):
self.isos.update_checksums()
self.assertItemsEqual(
mmc.call_args_list,
[self._call('Client', 'i386'),
self._call('Client', 'x86_64'),
self._call('Server', 's390x'),
self._call('Server', 'x86_64'),
self._call('Client', 'i386', source=True),
self._call('Client', 'x86_64', source=True),
self._call('Server', 's390x', source=True),
self._call('Server', 'x86_64', source=True)]
)
[mock.call(self.compose_path, self.isos.compose.images,
unified_isos.DEFAULT_CHECKSUMS, False,
self.isos._get_base_filename)])
@mock.patch('pungi_utils.unified_isos.make_checksums')
def test_update_checksums_one_file(self, mmc):
@ -591,28 +543,11 @@ class TestUpdateChecksums(PungiTestCase):
self.isos.update_checksums()
self.assertItemsEqual(
mmc.call_args_list,
[self._call('Client', 'i386', one_file=True),
self._call('Client', 'x86_64', one_file=True),
self._call('Server', 's390x', one_file=True),
self._call('Server', 'x86_64', one_file=True),
self._call('Client', 'i386', source=True, one_file=True),
self._call('Client', 'x86_64', source=True, one_file=True),
self._call('Server', 's390x', source=True, one_file=True),
self._call('Server', 'x86_64', source=True, one_file=True)]
)
[mock.call(self.compose_path, self.isos.compose.images,
unified_isos.DEFAULT_CHECKSUMS, True,
self.isos._get_base_filename)])
@mock.patch('pungi_utils.unified_isos.make_checksums')
def test_update_checksums_basename(self, mmc):
def test_get_base_filename(self):
self.isos.conf['media_checksum_base_filename'] = '{variant}-{arch}'
self.isos.update_checksums()
self.assertItemsEqual(
mmc.call_args_list,
[self._call('Client', 'i386', basename='Client-i386-'),
self._call('Client', 'x86_64', basename='Client-x86_64-'),
self._call('Server', 's390x', basename='Server-s390x-'),
self._call('Server', 'x86_64', basename='Server-x86_64-'),
self._call('Client', 'i386', source=True, basename='Client-i386-'),
self._call('Client', 'x86_64', source=True, basename='Client-x86_64-'),
self._call('Server', 's390x', source=True, basename='Server-s390x-'),
self._call('Server', 'x86_64', source=True, basename='Server-x86_64-')]
)
self.assertEqual(self.isos._get_base_filename('Client', 'x86_64'),
'Client-x86_64-')