From 1c9b27690bcb2edf9cf516ba3d87ab3436df3e54 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
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 <mikem@redhat.com>
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

