From 6998ffe694ab2bcb3b7d9245c4002c7734f09a7e Mon Sep 17 00:00:00 2001
From: Christian Kellner <christian@kellner.me>
Date: Nov 30 2020 08:57:48 +0000
Subject: osbuild: use task result to get build info


Instead of parsing the log file to get the NVR and then in turn
use that to get to the build info use the structured return value
from the koji task. The return value of the osbuild plugin is:
    result = {
        "composer": {
            "server": <COMPOSER_URL>,
            "id": <COMPOSE_ID>
        },
        "koji": {
            "build": <BUILD_ID>
        }
    }
This means we have direct access to the koji build id, which was
returned by composer to the plugin via its status API. Using that
removes the need to parse the log file.

Adapt the test accordingly.

Merges: https://pagure.io/pungi/pull-request/1475
Signed-off-by: Christian Kellner <christian@kellner.me>

---

diff --git a/pungi/phases/osbuild.py b/pungi/phases/osbuild.py
index c8322cc..f3cfa2e 100644
--- a/pungi/phases/osbuild.py
+++ b/pungi/phases/osbuild.py
@@ -1,7 +1,6 @@
 # -*- coding: utf-8 -*-
 
 import os
-import re
 from kobo.threads import ThreadPool, WorkerThread
 from kobo import shortcuts
 from productmd.images import Image
@@ -139,21 +138,21 @@ class RunOSBuildThread(WorkerThread):
                 "OSBuild: task %s failed: see %s for details" % (task_id, log_file)
             )
 
-        # Parse NVR from the task output. If release part of NVR was generated
-        # by Koji, we don't have enough information in the configuration.
-        nvr = get_nvr(log_file)
-
         # Refresh koji session which may have timed out while the task was
         # running. Watching is done via a subprocess, so the session is
         # inactive.
         koji = kojiwrapper.KojiWrapper(compose.conf["koji_profile"])
 
+        # Get build id via the task's result json data
+        result = koji.koji_proxy.getTaskResult(task_id)
+        build_id = result["koji"]["build"]
+
         linker = Linker(logger=self.pool._logger)
 
         # Process all images in the build. There should be one for each
         # architecture, but we don't verify that.
-        build_info = koji.koji_proxy.getBuild(nvr)
-        for archive in koji.koji_proxy.listArchives(buildID=build_info["build_id"]):
+        build_info = koji.koji_proxy.getBuild(build_id)
+        for archive in koji.koji_proxy.listArchives(buildID=build_id):
             if archive["type_name"] not in config["image_types"]:
                 # Ignore values that are not of required types.
                 continue
@@ -205,12 +204,3 @@ class RunOSBuildThread(WorkerThread):
             compose.im.add(variant=variant.uid, arch=arch, image=img)
 
         self.pool.log_info("[DONE ] %s (task id: %s)" % (msg, task_id))
-
-
-def get_nvr(log_file):
-    with open(log_file) as f:
-        for line in f:
-            match = re.search("Creating compose: ([^ ]+) ", line)
-            if match:
-                return match.group(1)
-    raise RuntimeError("Failed to find image NVR in the output")
diff --git a/tests/test_osbuild_phase.py b/tests/test_osbuild_phase.py
index a81472f..30be2a2 100644
--- a/tests/test_osbuild_phase.py
+++ b/tests/test_osbuild_phase.py
@@ -133,11 +133,16 @@ class RunOSBuildThreadTest(helpers.PungiTestCase):
     @mock.patch("pungi.phases.osbuild.kojiwrapper.KojiWrapper")
     def test_process(self, KojiWrapper, Linker):
         cfg = {"name": "test-image", "distro": "rhel-8", "image_types": ["qcow2"]}
+        build_id = 5678
         koji = KojiWrapper.return_value
         koji.watch_task.side_effect = self.make_fake_watch(0)
         koji.koji_proxy.osbuildImage.return_value = 1234
+        koji.koji_proxy.getTaskResult.return_value = {
+            "composer": {"server": "https://composer.osbuild.org", "id": ""},
+            "koji": {"build": build_id},
+        }
         koji.koji_proxy.getBuild.return_value = {
-            "build_id": 5678,
+            "build_id": build_id,
             "name": "test-image",
             "version": "1",
             "release": "1",
@@ -192,8 +197,9 @@ class RunOSBuildThreadTest(helpers.PungiTestCase):
                     },
                 ),
                 mock.call.watch_task(1234, mock.ANY),
-                mock.call.koji_proxy.getBuild("test-image-1-1"),
-                mock.call.koji_proxy.listArchives(buildID=5678),
+                mock.call.koji_proxy.getTaskResult(1234),
+                mock.call.koji_proxy.getBuild(build_id),
+                mock.call.koji_proxy.listArchives(buildID=build_id),
             ],
         )
 
@@ -247,11 +253,16 @@ class RunOSBuildThreadTest(helpers.PungiTestCase):
     @mock.patch("pungi.phases.osbuild.kojiwrapper.KojiWrapper")
     def test_process_without_release(self, KojiWrapper, Linker):
         cfg = {"name": "test-image", "distro": "rhel-8", "image_types": ["qcow2"]}
+        build_id = 5678
         koji = KojiWrapper.return_value
         koji.watch_task.side_effect = self.make_fake_watch(0)
         koji.koji_proxy.osbuildImage.return_value = 1234
+        koji.koji_proxy.getTaskResult.return_value = {
+            "composer": {"server": "https://composer.osbuild.org", "id": ""},
+            "koji": {"build": build_id},
+        }
         koji.koji_proxy.getBuild.return_value = {
-            "build_id": 5678,
+            "build_id": build_id,
             "name": "test-image",
             "version": "1",
             "release": "1",
@@ -302,8 +313,9 @@ class RunOSBuildThreadTest(helpers.PungiTestCase):
                     opts={"repo": [self.topdir + "/compose/Everything/$arch/os"]},
                 ),
                 mock.call.watch_task(1234, mock.ANY),
-                mock.call.koji_proxy.getBuild("test-image-1-1"),
-                mock.call.koji_proxy.listArchives(buildID=5678),
+                mock.call.koji_proxy.getTaskResult(1234),
+                mock.call.koji_proxy.getBuild(build_id),
+                mock.call.koji_proxy.listArchives(buildID=build_id),
             ],
         )