From 842e2e810e1377ab532b31d8857b02a636bed3eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Tue, 3 Oct 2017 11:06:03 +0200 Subject: [PATCH] unified-isos: Stop erasing metadata on failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When saving new metadata fails, the images.json file would be left empty. Instead we should not touch it at all. Signed-off-by: Lubomír Sedlář --- pungi_utils/unified_isos.py | 12 +++++++++++- tests/test_unified_isos.py | 22 +++++++++++++++++++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/pungi_utils/unified_isos.py b/pungi_utils/unified_isos.py index 5c935ea2..7702c95a 100644 --- a/pungi_utils/unified_isos.py +++ b/pungi_utils/unified_isos.py @@ -92,7 +92,17 @@ class UnifiedISO(object): shutil.rmtree(self.temp_dir) def dump_manifest(self): - self.compose.images.dump(os.path.join(self.compose_path, 'metadata', 'images.json')) + dest = os.path.join(self.compose_path, 'metadata', 'images.json') + tmp_file = dest + '.tmp' + try: + self.compose.images.dump(tmp_file) + except: + # We failed, clean up the temporary file. + if os.path.exists(tmp_file): + os.remove(tmp_file) + raise + # Success, move the temp file to proper location. + os.rename(tmp_file, dest) def _link_tree(self, dir, variant, arch): blacklist_files = [".treeinfo", ".discinfo", "boot.iso", "media.repo", "extra_files.json"] diff --git a/tests/test_unified_isos.py b/tests/test_unified_isos.py index 5b1a112d..f647bbae 100755 --- a/tests/test_unified_isos.py +++ b/tests/test_unified_isos.py @@ -9,7 +9,7 @@ from ConfigParser import SafeConfigParser sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) -from tests.helpers import PungiTestCase, FIXTURE_DIR, touch +from tests.helpers import PungiTestCase, FIXTURE_DIR, touch, mk_boom from pungi_utils import unified_isos @@ -35,13 +35,29 @@ class TestUnifiedIsos(PungiTestCase): self.assertRegexpMatches(isos.temp_dir, '^%s/' % os.path.join(self.topdir, COMPOSE_ID, 'work')) - def test_dump_manifest(self): + @mock.patch('os.rename') + def test_dump_manifest(self, rename): 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')]) + [mock.call.dump(compose_path + '/metadata/images.json.tmp')]) + self.assertEqual(rename.call_args_list, + [mock.call(compose_path + '/metadata/images.json.tmp', + compose_path + '/metadata/images.json')]) + + @mock.patch('os.rename') + def test_dump_manifest_fails(self, rename): + compose_path = os.path.join(self.topdir, COMPOSE_ID, 'compose') + isos = unified_isos.UnifiedISO(compose_path) + isos.compose._images = mock.Mock() + isos.compose._images.dump.side_effect = mk_boom() + with self.assertRaises(Exception): + isos.dump_manifest() + self.assertEqual(isos.compose._images.mock_calls, + [mock.call.dump(compose_path + '/metadata/images.json.tmp')]) + self.assertEqual(rename.call_args_list, []) class TestCreate(PungiTestCase):