Merge ~hloeung/charm-rsyslog-forwarder-ha:master into charm-rsyslog-forwarder-ha:master

[Haw Loeung]

Fix rsyslog permission denied; remove duplicate logging

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:ebe22fe3e6fbf71c6ae7def6b2ac18906a558670
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/1/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/1421/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/389/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/1//rebuild

review: Needs Fixing (continuous-integration)

[Robert Gildein (rgildein)]

I'm not expert here, but to me it seems that you commented most of
configuration and that do not looks good to me.

[JamesLin (jneo8)]

We need more information about this config change.

review: Needs Information

[Haw Loeung (hloeung)]

First of all, sorry about the delay in replying here.

Logging to these log files are not enabled by default in Ubuntu.

I still need to investigate why the `rsyslog` daemon is failing when
these are enabled but the files don't exist. It's likely AppArmor.

I'm going to update this MP to check if these files exist on disk and
only enable logging to these if they do.

[Haw Loeung (hloeung)]

On Thu, Aug 17, 2023 at 09:30:20PM -0000, Haw Loeung wrote:
> Logging to these log files are not enabled by default in Ubuntu.
>

See here for what's enabled by default in Ubuntu:

| https://git.launchpad.net/ubuntu/+source/rsyslog/tree/debian/50-default.conf

[Eric Chen (eric-chen)]

Change the status to WIP until any new commit available.

[Haw Loeung (hloeung)]

Okay, read for re-review please?

This changes it so it detects the existence of additional log files not normally enabled by default[1]. If they exist on disk, then we update the rsyslog conf to include and write out to those.

[1]https://git.launchpad.net/ubuntu/+source/rsyslog/tree/debian/50-default.conf

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:34b897446602f0a594b47fc8c1cd319e6edda8a8
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/2/
Executed test runs:
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/500/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/2//rebuild

review: Needs Fixing (continuous-integration)

[JamesLin (jneo8)]

Overall LGTM. Waiting the CI pass.
Rebuild the CI now.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:34b897446602f0a594b47fc8c1cd319e6edda8a8
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/3/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/1795/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/508/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/3//rebuild

review: Needs Fixing (continuous-integration)

e6a1d2e... by Haw Loeung on 2023-09-12

Fix lint errors

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:509b9e069e8dc8f13bb7cc447e6c3c862052e5d0
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/4/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/1796/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/509/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/4//rebuild

review: Needs Fixing (continuous-integration)

[Eric Chen (eric-chen)]

The default value of black is 88, we don't need the additional parameter.
https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length

review: Needs Fixing

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:e6a1d2ef41b92d2cd8a5bebdd6e6d0ace53a5a8d
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/5/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/1797/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/510/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/5//rebuild

review: Needs Fixing (continuous-integration)

[Eric Chen (eric-chen)]

Hi Haw, Can you rebase the code to latest one? Tianqi fixed the function test
https://code.launchpad.net/~txiao/charm-rsyslog-forwarder-ha/+git/charm-rsyslog-forwarder-ha/+merge/452565

59425ea... by Haw Loeung on 2023-10-05

Merge remote-tracking branch 'hloeung/master'

[Haw Loeung (hloeung)]

Done

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:59425ea225d0a61242d3bf24bef6788958f5ea08
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/7/
Executed test runs:
    ABORTED: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/1906/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/570/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/7//rebuild

review: Needs Fixing (continuous-integration)

[Tianqi Xiao (txiao)]

Code change LGTM. Re-triggering CI

review: Approve

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:59425ea225d0a61242d3bf24bef6788958f5ea08
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/12/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest31/262/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/665/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-rsyslog-forwarder-ha-ci/12//rebuild

review: Needs Fixing (continuous-integration)

[Haw Loeung (hloeung)]

Bump, any updates here?

[Eric Chen (eric-chen)]

Unfortunately, our Jenkins server was gone followed by the 3FP Data Centre decommissioning.
Could you please run the functional tests locally and attach the log?
Once both the unit and functional tests pass, we can proceed with merging it.
Thank you!

Unmerged commits

59425ea... by Haw Loeung on 2023-10-05

Merge remote-tracking branch 'hloeung/master'

e6a1d2e... by Haw Loeung on 2023-09-12

Fix lint errors

34b8974... by Haw Loeung on 2023-09-09

Rework additional logs

ebe22fe... by Haw Loeung on 2023-07-30

Remove duplicate logging for kern.* and mail.*

We're logging kern.* and mail.* to separate log files so no need to
also log them to /var/log/syslog.

6b8844f... by Haw Loeung on 2023-07-30

Comment out output to files that don't exist by default

This fixes issues with newer hosts and permission denied. E.g.:

Jul 28 00:00:01 csb-fw1 rsyslogd: file '/var/log/cron.log': open error: Permission denied [v8.2112.0 try https://www.rsyslog.com/e/2433 ]
Jul 28 00:00:03 csb-fw1 rsyslogd: file '/var/log/user.log': open error: Permission denied [v8.2112.0 try https://www.rsyslog.com/e/2433 ]
Jul 28 00:00:38 csb-fw1 rsyslogd: file '/var/log/daemon.log': open error: Permission denied [v8.2112.0 try https://www.rsyslog.com/e/2433 ]
Jul 28 00:00:38 csb-fw1 rsyslogd: file '/var/log/messages': open error: Permission denied [v8.2112.0 try https://www.rsyslog.com/e/2433 ]

diff --git a/hooks/hooks.py b/hooks/hooks.py
index 19888c7..bb6a0fb 100755
--- a/hooks/hooks.py
+++ b/hooks/hooks.py
@@ -6,7 +6,6 @@ import base64
 import os
 import subprocess
 import sys
-from shutil import copyfile
 
 _HERE = os.path.abspath(os.path.dirname(__file__))
 
@@ -29,7 +28,7 @@ from charmhelpers.core.host import (  # noqa: E402
 from charmhelpers.fetch import apt_install  # noqa:E402
 
 from model import Server, session  # noqa:E402
-from utils import get_template_dir, get_template  # noqa:E402
+from utils import get_template  # noqa:E402
 from utils import __die as die  # noqa:E402
 
 
@@ -43,7 +42,6 @@ required_packages = [
 
 
 IMFILE_FILE = "/etc/rsyslog.d/40-rsyslog-imfile.conf"
-LOGS_TEMPLATE = "keep_local.template"
 LOGS_SYSTEM_FILE = "/etc/rsyslog.d/50-default.conf"
 REPLICATION_FILE = "/etc/rsyslog.d/45-rsyslog-replication.conf"
 CERT_FILE = "/etc/rsyslog.d/42-cert-rsyslog.conf"
@@ -88,7 +86,30 @@ def update_imfile(imfiles):
 
 def update_local_logs(keep=True):
     if keep:
-        copyfile(os.path.join(get_template_dir(), LOGS_TEMPLATE), LOGS_SYSTEM_FILE)
+        additional_logs = {
+            "cron.*": "/var/log/cron.log",
+            "daemon.*": "/var/log/daemon.log",
+            "lpr.*": "/var/log/lpr.log",
+            "user.*": "/var/log/user.log",
+            "mail.info": "/var/log/mail.info",
+            "mail.warn": "/var/log/mail.warn",
+        }
+        new = {}
+        for al, logf in additional_logs.items():
+            if os.path.exists(logf):
+                new[al] = logf
+        debug_log = False
+        if os.path.exists("/var/log/debug"):
+            debug_log = True
+        messages_log = False
+        if os.path.exists("/var/log/messages"):
+            messages_log = True
+
+        with open(LOGS_SYSTEM_FILE, "w") as fd:
+            tmpl = get_template("keep_local").render(
+                additional=new, debug_log=debug_log, messages_log=messages_log
+            )
+            fd.write(tmpl)
     else:
         if os.path.exists(LOGS_SYSTEM_FILE):
             os.remove(LOGS_SYSTEM_FILE)
diff --git a/templates/keep_local.template b/templates/keep_local.template
index 5577674..8c6c76b 100644
--- a/templates/keep_local.template
+++ b/templates/keep_local.template
@@ -4,22 +4,23 @@ kern.*                          -/var/log/kern.log
 
 # Some defaults
 auth,authpriv.*                 /var/log/auth.log
-*.*;auth,authpriv.none          -/var/log/syslog
-cron.*                         /var/log/cron.log
-daemon.*                        -/var/log/daemon.log
-lpr.*                           -/var/log/lpr.log
+*.*;auth,authpriv.none,kern.none,mail.none          -/var/log/syslog
 mail.*                          -/var/log/mail.log
-user.*                          -/var/log/user.log
-
-mail.info                       -/var/log/mail.info
-mail.warn                       -/var/log/mail.warn
 mail.err                        /var/log/mail.err
 
+{%- for key, log in additional.items() %}
+{{ key }}                       -{{ log }}
+{%- endfor %}
+
 # Catch-alls
+{%- if debug_log %}
 *.=debug;\
         auth,authpriv.none;\
         news.none;mail.none     -/var/log/debug
+{%- endif %}
+{%- if messages_log %}
 *.=info;*.=notice;*.=warn;\
         auth,authpriv.none;\
         cron,daemon.none;\
         mail,news.none          -/var/log/messages
+{%- endif %}