From 1c9b27690bcb2edf9cf516ba3d87ab3436df3e54 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Apr 15 2025 14:53:50 +0000 Subject: [PATCH 1/2] handle arch-duplicate logs in importImageInternal --- diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index c326854..8fa560c 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -10545,25 +10545,30 @@ def importImageInternal(task_id, build_info, imgdata): raise koji.BuildError('Unsupported file type: %s' % imgfile) archives.append(import_archive(fullpath, build_info, 'image', imgdata)) - # upload logs + # get uploaded logs logs = [f for f in os.listdir(workpath) if f.endswith('.log')] + logdir = joinpath(koji.pathinfo.build(build_info), 'data/logs/image') for logfile in logs: logsrc = joinpath(workpath, logfile) - if tinfo['method'] in ('appliance', 'image', 'indirectionimage', 'livecd'): - logdir = joinpath(koji.pathinfo.build(build_info), - 'data/logs/image') - else: - # multiarch livemedia (and plugins') spins can have log name conflicts, so we - # add the arch to the path - logdir = joinpath(koji.pathinfo.build(build_info), - 'data/logs/image', imgdata['arch']) - koji.ensuredir(logdir) + + # figure out destination dir final_path = joinpath(logdir, os.path.basename(logfile)) + add_arch = False + if tinfo['method'] not in ('appliance', 'image', 'indirectionimage', 'livecd'): + add_arch = True + if add_arch or os.path.exists(final_path): + # add arch for uniqueness + final_path = joinpath(logdir, imgdata['arch'], os.path.basename(logfile)) + + # sanity checks if os.path.exists(final_path): raise koji.GenericError("Error importing build log. %s already exists." % final_path) if os.path.islink(logsrc) or not os.path.isfile(logsrc): raise koji.GenericError("Error importing build log. %s is not a regular file." % logsrc) + + # move the logs + koji.ensuredir(logdir) move_and_symlink(logsrc, final_path, create_dir=True) # record all of the RPMs installed in the image(s) From 62021b8119654de87b0cfc3cd7c603cc76af3696 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Apr 15 2025 14:53:50 +0000 Subject: [PATCH 2/2] unit tests --- diff --git a/tests/test_hub/test_complete_image_build.py b/tests/test_hub/test_complete_image_build.py index f9d5d56..467b251 100644 --- a/tests/test_hub/test_complete_image_build.py +++ b/tests/test_hub/test_complete_image_build.py @@ -67,6 +67,7 @@ class TestCompleteImageBuild(unittest.TestCase): self._dml = mock.patch('kojihub.kojihub._dml').start() mock.patch('kojihub.kojihub.build_notification').start() mock.patch('kojihub.kojihub.assert_policy').start() + mock.patch('kojihub.kojihub.policy_data_from_task').start() mock.patch('kojihub.kojihub.check_volume_policy', return_value={'id': 0, 'name': 'DEFAULT'}).start() self.set_up_callbacks() @@ -116,7 +117,8 @@ class TestCompleteImageBuild(unittest.TestCase): for filename in data[arch]['files']: paths.append(os.path.join(imgdir, filename)) for filename in data[arch]['logs']: - paths.append(os.path.join(logdir, 'image', arch, filename)) + # paths.append(os.path.join(logdir, 'image', arch, filename)) + paths.append(os.path.join(logdir, 'image', filename)) # bdir = koji.pathinfo.build(buildinfo) # paths.append(os.path.join(bdir, 'metadata.json')) return paths @@ -204,6 +206,8 @@ class TestCompleteImageBuild(unittest.TestCase): image_info = {'build_id': buildinfo['id']} self.get_build.return_value = buildinfo self.get_image_build.return_value = image_info + taskinfo = {'id': 'TASKID', 'method': 'image'} + self.Task.return_value.getInfo.return_value = taskinfo # run the import call self.hostcalls.completeImageBuild('TASK_ID', 'BUILD_ID', self.image_data) diff --git a/tests/test_hub/test_import_image_internal.py b/tests/test_hub/test_import_image_internal.py index 73f77ed..ec1c6c3 100644 --- a/tests/test_hub/test_import_image_internal.py +++ b/tests/test_hub/test_import_image_internal.py @@ -13,6 +13,7 @@ class TestImportImageInternal(unittest.TestCase): self.context = mock.patch('kojihub.kojihub.context').start() self.context_db = mock.patch('kojihub.db.context').start() self.Task = mock.patch('kojihub.kojihub.Task').start() + self.Task.return_value.assertHost = mock.MagicMock() self.get_build = mock.patch('kojihub.kojihub.get_build').start() self.get_archive_type = mock.patch('kojihub.kojihub.get_archive_type').start() self.path_work = mock.patch('koji.pathinfo.work').start() @@ -25,9 +26,6 @@ class TestImportImageInternal(unittest.TestCase): mock.patch.stopall() def test_basic(self): - task = mock.MagicMock() - task.assertHost = mock.MagicMock() - self.Task.return_value = task imgdata = { 'arch': 'x86_64', 'task_id': 1, @@ -53,9 +51,8 @@ class TestImportImageInternal(unittest.TestCase): task_id=1, build_info=self.get_build.return_value, imgdata=imgdata) def test_with_rpm(self): - task = mock.MagicMock() - task.assertHost = mock.MagicMock() - self.Task.return_value = task + taskinfo = {'id': 101010, 'method': 'image'} + self.Task.return_value.getInfo.return_value = taskinfo rpm = { # 'location': 'foo', 'id': 6, @@ -105,7 +102,7 @@ class TestImportImageInternal(unittest.TestCase): # Check that the log symlink made it to where it was supposed to. dest = os.readlink(workdir + '/foo.log') dest = os.path.abspath(os.path.join(workdir, dest)) - self.assertEqual(dest, self.tempdir + '/data/logs/image/x86_64/foo.log') + self.assertEqual(dest, self.tempdir + '/data/logs/image/foo.log') # And.. check all the sql statements self.assertEqual(len(cursor.execute.mock_calls), 1) @@ -115,3 +112,94 @@ class TestImportImageInternal(unittest.TestCase): 'VALUES (%(archive_id0)s, %(rpm_id0)s)' self.assertEqual(expression, expected) self.assertEqual(kwargs, {'archive_id0': 9, 'rpm_id0': 6}) + + def test_with_log_overlap(self): + taskinfo = {'id': 101010, 'method': 'image'} + self.Task.return_value.getInfo.return_value = taskinfo + imgdata = { + 'arch': 'x86_64', + 'task_id': 1, + 'files': [ + 'some_file', + ], + 'rpmlist': [], + } + build_info = { + 'name': 'name', + 'version': 'version', + 'release': 'release', + 'id': 2 + } + cursor = mock.MagicMock() + self.context_db.cnx.cursor.return_value = cursor + self.context_db.session.host_id = 42 + self.get_build.return_value = build_info + self.get_archive_type.return_value = 4 + self.path_work.return_value = self.tempdir + self.build.return_value = self.tempdir + self.import_archive.return_value = { + 'id': 9, + 'filename': self.tempdir + '/foo.archive', + } + workdir = self.tempdir + "/tasks/1/1" + os.makedirs(workdir) + # Create a log file to exercise that code path + with open(workdir + '/foo.log', 'w'): + pass + + # Create a same named log file already present + # This should force the function to add arch to the final path + os.makedirs(self.tempdir + '/data/logs/image') + with open(self.tempdir + '/data/logs/image/foo.log', 'w'): + pass + + kojihub.importImageInternal(task_id=1, build_info=build_info, imgdata=imgdata) + + # Check that the log symlink made it to where it was supposed to. + dest = os.readlink(workdir + '/foo.log') + dest = os.path.abspath(os.path.join(workdir, dest)) + self.assertEqual(dest, self.tempdir + '/data/logs/image/x86_64/foo.log') + + def test_with_livemedia_task(self): + taskinfo = {'id': 101010, 'method': 'livemedia'} + self.Task.return_value.getInfo.return_value = taskinfo + imgdata = { + 'arch': 'x86_64', + 'task_id': 1, + 'files': [ + 'some_file', + ], + 'rpmlist': [], + } + build_info = { + 'name': 'name', + 'version': 'version', + 'release': 'release', + 'id': 2 + } + cursor = mock.MagicMock() + self.context_db.cnx.cursor.return_value = cursor + self.context_db.session.host_id = 42 + self.get_build.return_value = build_info + self.get_archive_type.return_value = 4 + self.path_work.return_value = self.tempdir + self.build.return_value = self.tempdir + self.import_archive.return_value = { + 'id': 9, + 'filename': self.tempdir + '/foo.archive', + } + workdir = self.tempdir + "/tasks/1/1" + os.makedirs(workdir) + # Create a log file to exercise that code path + with open(workdir + '/foo.log', 'w'): + pass + + kojihub.importImageInternal(task_id=1, build_info=build_info, imgdata=imgdata) + + # Check that the log symlink made it to where it was supposed to. + dest = os.readlink(workdir + '/foo.log') + dest = os.path.abspath(os.path.join(workdir, dest)) + self.assertEqual(dest, self.tempdir + '/data/logs/image/x86_64/foo.log') + + +# the end