Collect and compress fedora-review logs after run

This commit also adds support for using environment variables with `run_cmd`

backend/copr_backend/background_worker_build.py

@@ -513,14 +513,15 @@ def _transfer_log_file(self):
         except SSHConnectionError as exc:
             return "Stopped following builder for broken SSH: {}".format(exc)
 
-    def _compress_live_logs(self):
+    def _compress_logs(self):
         """
-        Compress builder-live.log and backend.log by gzip.
+        Compress builder-live.log, backend.log, and fedora-review.log using gzip.
         Never raise any exception!
         """
         logs = [
             self.job.builder_log,
             self.job.backend_log,
+            self.job.review_log,
         ]
 
         # For automatic redirect from log to log.gz, consider configuring
@@ -530,6 +531,8 @@ def _compress_live_logs(self):
         #   url.rewrite-if-not-file = ("^/(.*)/builder-live.log$" => "/$1/redirect-builder-live.log")
         #   url.redirect += ( "^/(.*)/redirect-backend.log$" => "/$1/backend.log.gz" )
         #   url.rewrite-if-not-file += ("^/(.*)/backend.log$" => "/$1/redirect-backend.log")
+        #   url.redirect += ( "^/(.*)/redirect-backend.log$" => "/$1/fedora-review.log.gz" )
+        #   url.rewrite-if-not-file += ("^/(.*)/backend.log$" => "/$1/redirect-fedora-review.log")
         #
         #   $HTTP["url"] =~ "\.log\.gz$" {
         #       magnet.attract-physical-path-to = ( "/etc/lighttpd/content-encoding-gzip-if-exists.lua" )
@@ -543,7 +546,7 @@ def _compress_live_logs(self):
         #   end
         #
         # Or Apache with:
-        #     <FilesMatch "^(builder-live|backend)\.log$">
+        #     <FilesMatch "^(builder-live|backend|fedora-review)\.log$">
         #     RewriteEngine on
         #     RewriteCond %{REQUEST_FILENAME} !-f
         #     RewriteRule ^(.*)$ %{REQUEST_URI}.gz [R]
@@ -558,6 +561,12 @@ def _compress_live_logs(self):
                 self.log.error("Compressed log %s exists", dest)
                 continue
 
+            if not os.path.exists(src):
+                # fedora-review.log has a good chance of not existing
+                # We should be ready for other similar files
+                self.log.warning("Not trying to compress %s as it does not exist", src)
+                continue
+
             self.log.info("Compressing %s by gzip", src)
             res = run_cmd(["gzip", src], logger=self.log)
             if res.returncode not in [0, 2]:
@@ -814,7 +823,7 @@ def handle_task(self):
             self._drop_host()
             if self.job:
                 self._mark_finished()
-                self._compress_live_logs()
+                self._compress_logs()
             else:
                 self.log.error("No job object from Frontend")
             self.redis_set_worker_flag("status", "done")

backend/copr_backend/job.py

@@ -144,6 +144,13 @@ def builder_log(self):
         """
         return os.path.join(self.results_dir, "builder-live.log")
 
+    @property
+    def review_log(self):
+        """
+        The log file from running fedora-review
+        """
+        return os.path.join(self.results_dir, "fedora-review/fedora-review.log")
+
     @property
     def rsync_log_name(self):
         return "build-{:08d}.rsync.log".format(self.build_id)

rpmbuild/copr_rpmbuild/automation/fedora_review.py

@@ -10,10 +10,29 @@
 
 import os
 import shutil
+from contextlib import contextmanager
+from typing import Generator
+
 from copr_rpmbuild.helpers import run_cmd
 from copr_rpmbuild.automation.base import AutomationTool
 
 
+@contextmanager
+def cache_directory(resultdir) -> Generator[str, None, None]:
+    """
+    Create a directory for a job to use as the XDG cache.
+
+    :param str resultdir: Parent directory for the run results
+    :return: Path to cache directory
+    """
+    cachedir = os.path.join(resultdir, "cache")
+    try:
+        os.makedirs(cachedir, exist_ok=True)
+        yield cachedir
+    finally:
+        shutil.rmtree(cachedir)
+
+
 class FedoraReview(AutomationTool):
     """
     Optionally run `fedora-review` tool after build
@@ -44,17 +63,17 @@ def run(self):
             "--name", self.package_name,
             "--mock-config", self.mock_config_file,
         ]
+        with cache_directory(self.resultdir) as cachedir:
+            try:
+                result = run_cmd(cmd, cwd=self.resultdir, env={"XDG_CACHE_HOME": cachedir})
+                self.log.info(result.stdout)
+            except RuntimeError as ex:
+                self.log.warning("Fedora review failed\nerr:\n%s", ex)
+                self.log.warning("The build itself will not be marked "
+                                 "as failed because of this")
+            self._filter_results_directory(cachedir)
 
-        try:
-            result = run_cmd(cmd, cwd=self.resultdir)
-            self.log.info(result.stdout)
-        except RuntimeError as ex:
-            self.log.warning("Fedora review failed\nerr:\n%s", ex)
-            self.log.warning("The build itself will not be marked "
-                             "as failed because of this")
-        self._filter_results_directory()
-
-    def _filter_results_directory(self):
+    def _filter_results_directory(self, cachedir: str):
         """
         Currently, fedora-review tool doesn't have an option to specify
         a destdir, and produces output to a directory called after the package
@@ -67,6 +86,18 @@ def _filter_results_directory(self):
         dstdir = os.path.join(self.resultdir, "fedora-review")
         os.makedirs(dstdir, exist_ok=True)
 
+        logfile = os.path.join(cachedir, "fedora-review.log")
+
+        # The fedora-review command failed to start so badly that it didn't even create a log file.
+        # Given how early that should happen, this merits investigation.
+        if not os.path.exists(logfile):
+            self.log.error("Can't find fedora-review log: %s", logfile)
+            self.log.error("Please raise a bug on https://github.com/fedora-copr/copr including a link to this build")
+            # If the log file doesn't exist, there's zero chance the results exist
+            return
+
+        os.rename(logfile, os.path.join(dstdir, "fedora-review.log"))
+
         # The fedora-review command failed so early that it didn't even create
         # the resultdir. Nothing to do here.
         if not os.path.exists(srcdir):

rpmbuild/copr_rpmbuild/helpers.py

@@ -38,13 +38,14 @@ def cmd_readable(cmd):
     return ' '.join([shlex.quote(part) for part in cmd])
 
 
-def run_cmd(cmd, cwd=".", preexec_fn=None):
+def run_cmd(cmd, cwd=".", preexec_fn=None, env=None):
     """
     Runs given command in a subprocess.
 
     :param list(str) cmd: command to be executed and its arguments
     :param str cwd: In which directory to execute the command
     :param func preexec_fn: a callback invoked before exec in subprocess
+    :param dict env: environment variables to set for process
 
     :raises RuntimeError
     :returns munch.Munch(cmd, stdout, stderr, returncode)
@@ -53,7 +54,7 @@ def run_cmd(cmd, cwd=".", preexec_fn=None):
 
     try:
         process = subprocess.Popen(
-            cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd, preexec_fn=preexec_fn)
+            cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd, preexec_fn=preexec_fn, env=env)
         (stdout, stderr) = process.communicate()
     except OSError as e:
         if e.errno == errno.ENOENT: