Fix missing task info from failed record.log

.gitignore

@@ -27,3 +27,4 @@ cover
 
 # Ignore ocp-build-dat
 ocp-build-data/
+tests_functional.log

doozerlib/distgit.py

@@ -1035,14 +1035,18 @@ def wait(n):
 
                 if self.image_build_method == "osbs2":  # use OSBS 2
                     osbs2 = OSBS2Builder(self.runtime, scratch=scratch, dry_run=dry_run)
-                    task_id, task_url, nvr = osbs2.build(self.metadata, profile, retries=retries)
-                    record["task_id"] = task_id
-                    record["task_url"] = task_url
-                    record["nvrs"] = nvr
+                    try:
+                        osbs2.build(self.metadata, profile, retries=retries)
+                    except exectools.RetryException:
+                        raise
+                    finally:
+                        record["task_id"] = osbs2.task_id
+                        record["task_url"] = osbs2.task_url
+                        record["nvrs"] = osbs2.nvr
                     if not dry_run:
-                        self.update_build_db(True, task_id=task_id, scratch=scratch)
+                        self.update_build_db(True, task_id=osbs2.task_id, scratch=scratch)
                         if not scratch:
-                            nvr_dict = parse_nvr(nvr)
+                            nvr_dict = parse_nvr(osbs2.nvr)
                             push_version = nvr_dict["version"]
                             push_release = nvr_dict["release"]
                 else:  # use OSBS 1

doozerlib/osbs2_builder.py

@@ -27,19 +27,21 @@ def __init__(
         self._runtime = runtime
         self.scratch = scratch
         self.dry_run = dry_run
+        self.task_id: int = 0
+        self.task_url: str = ""
+        self.nvr: str = ""
 
     def build(self, image: "image.ImageMetadata", profile: Dict, retries: int = 3):
         dg: "distgit.ImageDistGitRepo" = image.distgit_repo()
         logger = dg.logger
         nvr = f"{dg.name}-{dg.org_version}-{dg.org_release}"
+        self.nvr = nvr
         if len(image.targets) > 1:
             # Currently we don't really support building images against multiple targets,
             # or we would overwrite the image tag when pushing to the registry.
             # `targets` is defined as an array just because we want to keep consistency with RPM build.
             raise DoozerFatalError("Building images against multiple targets is not currently supported.")
         target = image.targets[0]
-        task_id = 0
-        task_url = None
         build_id = 0
         build_info = None
         build_url = None
@@ -53,23 +55,23 @@ def build(self, image: "image.ImageMetadata", profile: Dict, retries: int = 3):
             error = None
             try:
                 # Submit build task
-                task_id, task_url = self._start_build(dg, target, profile, koji_api)
-                logger.info("Waiting for build task %s to complete...", task_id)
+                self._start_build(dg, target, profile, koji_api)
+                logger.info("Waiting for build task %s to complete...", self.task_id)
                 if self.dry_run:
-                    logger.warning("[DRY RUN] Build task %s would have completed", task_id)
+                    logger.warning("[DRY RUN] Build task %s would have completed", self.task_id)
                     error = None
                 else:
-                    error = brew.watch_task(koji_api, logger.info, task_id, terminate_event=threading.Event())
+                    error = brew.watch_task(koji_api, logger.info, self.task_id, terminate_event=threading.Event())
 
                 # Gather brew-logs
                 logger.info("Gathering brew-logs")
-                cmd = ["brew", "download-logs", "--recurse", "-d", dg._logs_dir(), task_id]
+                cmd = ["brew", "download-logs", "--recurse", "-d", dg._logs_dir(), self.task_id]
                 if self.dry_run:
                     logger.warning("[DRY RUN] Would have downloaded Brew logs with %s", cmd)
                 else:
                     logs_rc, _, logs_err = exectools.cmd_gather(cmd)
                     if logs_rc != 0:
-                        logger.warning("Error downloading build logs from brew for task %s: %s", task_id, logs_err)
+                        logger.warning("Error downloading build logs from brew for task %s: %s", self.task_id, logs_err)
 
                 if error:
                     # Looking for error message like the following to conclude the image has already been built:
@@ -97,7 +99,7 @@ def build(self, image: "image.ImageMetadata", profile: Dict, retries: int = 3):
                     build_info = {"id": build_id, "nvr": nvr}
                 elif not build_info:
                     # Unlike rpm build, koji_api.listBuilds(taskID=...) doesn't support image build. For now, let's use a different approach.
-                    taskResult = koji_api.getTaskResult(task_id)
+                    taskResult = koji_api.getTaskResult(self.task_id)
                     build_id = int(taskResult["koji_builds"][0])
                     build_info = koji_api.getBuild(build_id)
                 build_url = f"{BREWWEB_URL}/buildinfo?buildID={build_info['id']}"
@@ -119,10 +121,10 @@ def build(self, image: "image.ImageMetadata", profile: Dict, retries: int = 3):
         if error:
             raise exectools.RetryException(
                 f"Giving up after {retries} failed attempt(s): {message}",
-                (task_url, task_url),
+                (self.task_url, self.task_url),
             )
 
-        logger.info("Successfully built image %s; task: %s; build record: %s", nvr, task_url, build_url)
+        logger.info("Successfully built image %s; task: %s; build record: %s", nvr, self.task_url, build_url)
 
         if self._runtime.hotfix:
             # Tag the image so it won't get garbage collected.
@@ -132,7 +134,6 @@ def build(self, image: "image.ImageMetadata", profile: Dict, retries: int = 3):
             else:
                 koji_api.tagBuild(image.hotfix_brew_tag(), build_info["nvr"])
                 logger.warning("Build %s has been tagged into %s", nvr, image.hotfix_brew_tag())
-        return task_id, task_url, nvr
 
     def _start_build(self, dg: "distgit.ImageDistGitRepo", target: str, profile: Dict, koji_api: koji.ClientSession):
         logger = dg.logger
@@ -158,18 +159,16 @@ def _start_build(self, dg: "distgit.ImageDistGitRepo", target: str, profile: Dic
             'git_branch': dg.branch,
         }
 
-        task_id = 0
         logger.info("Starting OSBS 2 build with source %s and target %s...", src, target)
         if self.dry_run:
             logger.warning("[DRY RUN] Would have started container build")
         else:
             if not koji_api.logged_in:
                 koji_api.gssapi_login()
-            task_id: int = koji_api.buildContainer(src, target, opts=opts, channel="container-binary")
+            self.task_id = koji_api.buildContainer(src, target, opts=opts, channel="container-binary")
 
-        task_url = f"{BREWWEB_URL}/taskinfo?taskID={task_id}"
-        logger.info("OSBS 2 build started. Task ID: %s, url: %s", task_id, task_url)
-        return task_id, task_url
+        self.task_url = f"{BREWWEB_URL}/taskinfo?taskID={self.task_id}"
+        logger.info(f"OSBS2 build started. Task ID: {self.task_id}, url: {self.task_url}")
 
     def _construct_build_source_url(self, dg: "distgit.ImageDistGitRepo"):
         if not dg.sha:

tests/test_osbs2.py

@@ -54,7 +54,7 @@ def test_start_build(self):
         koji_api = MagicMock(logged_in=False)
         koji_api.buildContainer.return_value = 12345
 
-        task_id, task_url = osbs2._start_build(dg, "rhaos-4.12-rhel-8-containers-candidate", profile, koji_api)
+        osbs2._start_build(dg, "rhaos-4.12-rhel-8-containers-candidate", profile, koji_api)
         dg.cgit_file_available.assert_called_once_with(".oit/signed.repo")
         koji_api.gssapi_login.assert_called_once_with()
         koji_api.buildContainer.assert_called_once_with(
@@ -67,21 +67,27 @@ def test_start_build(self):
                 'git_branch': "rhaos-4.12-rhel-8",
             },
             channel="container-binary")
-        self.assertEqual(task_id, 12345)
-        self.assertEqual(task_url, f"{constants.BREWWEB_URL}/taskinfo?taskID=12345")
+        self.assertEqual(osbs2.task_id, 12345)
+        self.assertEqual(osbs2.task_url, f"{constants.BREWWEB_URL}/taskinfo?taskID=12345")
 
     @patch("doozerlib.exectools.cmd_gather", return_value=(0, "", ""))
     @patch("doozerlib.brew.watch_task", return_value=None)
-    @patch("doozerlib.osbs2_builder.OSBS2Builder._start_build", return_value=(12345, f"{constants.BREWWEB_URL}/taskinfo?taskID=12345"))
-    def test_build(self, _start_build: MagicMock, watch_task: MagicMock, cmd_gather: MagicMock):
+    def test_build(self, watch_task: MagicMock, cmd_gather: MagicMock):
+        class MockedOSBS2Builder(OSBS2Builder):
+            def _start_build(self, dg, target, profile, koji_api):
+                self.task_id = 12345
+                self.task_url = f'https://brewweb.engineering.redhat.com/brew/taskinfo?taskID={self.task_id}'
+
         koji_api = MagicMock(logged_in=False)
         koji_api.getTaskResult = MagicMock(return_value={"koji_builds": [42]})
         koji_api.getBuild = MagicMock(return_value={"id": 42, "nvr": "foo-v4.12.0-12345.p0.assembly.test"})
         runtime = MagicMock()
         runtime.build_retrying_koji_client = MagicMock(return_value=koji_api)
-        osbs2 = OSBS2Builder(runtime)
+        osbs2 = MockedOSBS2Builder(runtime)
         meta = self._make_image_meta(runtime)
         dg = ImageDistGitRepo(meta, autoclone=False)
+        dg.cgit_file_available = MagicMock(return_value=(True, "http://cgit.example.com/foo.repo"))
+        koji_api.buildContainer.return_value = 12345
         meta.distgit_repo = MagicMock(return_value=dg)
         dg.sha = "deadbeef"
         dg.branch = "rhaos-4.12-rhel-8"
@@ -93,13 +99,18 @@ def test_build(self, _start_build: MagicMock, watch_task: MagicMock, cmd_gather:
             "repo_list": [],
         }
 
-        task_id, task_url, nvr = osbs2.build(meta, profile, retries=1)
-        self.assertEqual((task_id, task_url, nvr), (12345, f"{constants.BREWWEB_URL}/taskinfo?taskID=12345", "foo-v4.12.0-12345.p0.assembly.test"))
-        koji_api.gssapi_login.assert_called_once_with()
+        osbs2.build(meta, profile, retries=1)
+
+        koji_api.gssapi_login.assert_called()
         koji_api.getTaskResult.assert_called_once_with(12345)
         koji_api.getBuild.assert_called_once_with(42)
         koji_api.tagBuild.assert_called_once_with('rhaos-4.12-rhel-8-hotfix', "foo-v4.12.0-12345.p0.assembly.test")
         runtime.build_retrying_koji_client.assert_called_once_with()
-        _start_build.assert_called_once_with(dg, 'rhaos-4.12-rhel-8-containers-candidate', {'signing_intent': 'release', 'repo_type': 'signed', 'repo_list': []}, koji_api)
         watch_task.assert_called_once_with(koji_api, ANY, 12345, terminate_event=ANY)
         cmd_gather.assert_called_once_with(['brew', 'download-logs', '--recurse', '-d', ANY, 12345])
+        self.assertEqual(osbs2.task_id, 12345)
+        self.assertEqual(osbs2.task_url, f"{constants.BREWWEB_URL}/taskinfo?taskID=12345")
+
+
+if __name__ == '__main__':
+    unittest.main()