From 19b8f82f5b5552ee59eb539f2187639e65925c5e Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <praiskup@redhat.com>
Date: Fri, 3 Dec 2021 17:03:03 +0100
Subject: [PATCH] copr-be: we shouldn't use both cronolog and logrotate

These implications constraint us:

    max-workers is used => cronolog needs to be used
    log counter enabled => we need logrotate

But:
    when logrotate is used => cronlog doesn't detect rotation

So the '4c4b63516da9128342e8705244a91a09b1b9ff77' is IMO explained.
Cronolog kept writing the log to the old descriptor, and the new
access.log link was just empty (before 4c4b63516da912834 when we just
reloaded, not restarted).  So I hope now that there isn't any other
problem that would require us to restart Lighty regularly.

The **UGLY** work-around is to terminate the cronlog process.  As I've
found out, cronlog is automatically restarted by mod_accesslog - though
this is somewhat gray zone and we should long-term do one of those:

    - stop using logrotate && rework log counter
    - stop using max-workers && cronlog
    - stop using Lighty

Resolves: https://pagure.io/copr/copr/issue/2001
Resolves: https://pagure.io/fedora-infrastructure/issue/10391
---
 .../copr/backend/templates/lighttpd/lighttpd.conf |  7 ++++++-
 .../copr/backend/templates/logrotate/lighttpd.j2  | 15 ++++++++++-----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/roles/copr/backend/templates/lighttpd/lighttpd.conf b/roles/copr/backend/templates/lighttpd/lighttpd.conf
index 6bb6433cc1..89210fe51a 100644
--- a/roles/copr/backend/templates/lighttpd/lighttpd.conf
+++ b/roles/copr/backend/templates/lighttpd/lighttpd.conf
@@ -532,7 +532,12 @@ $HTTP["url"] =~ "^/archive/spacewalk/" {
   index-file.names   = ( "index.html" )
   accesslog.filename = ""
 }
-server.max-worker = 6 # this may break many things
+
+# This may break many things, per: https://redmine.lighttpd.net/projects/1/wiki/Server_max-workerDetails
+# At least we have to use cronolog: https://redmine.lighttpd.net/projects/1/wiki/Docs_ModAccesslog
+# But we need logrotate for logcounter, see /etc/logrotate.d/lighttpd, and
+# logrotate doesn't work well with logcounter.
+server.max-worker = 6
 
 $HTTP["url"] !~ "^/archive/spacewalk($|/)" {
   accesslog.filename = "|/usr/sbin/cronolog /var/log/lighttpd/access.log"
diff --git a/roles/copr/backend/templates/logrotate/lighttpd.j2 b/roles/copr/backend/templates/logrotate/lighttpd.j2
index 11088c8c8e..ec3e04f08c 100644
--- a/roles/copr/backend/templates/logrotate/lighttpd.j2
+++ b/roles/copr/backend/templates/logrotate/lighttpd.j2
@@ -1,7 +1,12 @@
-# We don't want to reload lighttpd twice (for each log file), so we use
-# the sharedscripts option.  This means that hitcounter is run only once too
-# (which is good).  In (likely impossible) situations when access.log is empty
-# and error.log not, we would call hitcounter too on an empty file.
+# We don't want to reload twice (for each log file), so we use the sharedscripts
+# option.  This means that hitcounter is run only once too (which is good).  In
+# (likely impossible) situations when access.log is empty and error.log not, we
+# would call hitcounter too on an empty file.
+#
+# Note that lighttpd server runs with max-workers, and thus we pipe the access
+# logs through cronolog to the access.log.  It doesn't make sense to -HUP
+# Lighty, because then it doesn't restart cronolog.  We need to -HUP the
+# cronolog directly (which is fortunately re-started by Lighty/mod_accesslog).
 
 /var/log/lighttpd/*.log {
     rotate 5
@@ -18,6 +23,6 @@
 {% endif %}
     endscript
     postrotate
-        /usr/bin/killall -HUP lighttpd &>/dev/null || :
+        /usr/bin/killall -HUP cronolog &>>/var/log/copr-backend/cronolog-restart.log || :
     endscript
 }