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

Proposed by Haw Loeung
Status: Merged
Merged at revision: d0d424133833e1db49a16ab0eff9d3b7e8f0ab77
Proposed branch: ~hloeung/charm-rsyslog-forwarder-ha:master
Merge into: charm-rsyslog-forwarder-ha:master
Diff against target: 97 lines (+34/-12)
2 files modified
hooks/hooks.py (+25/-4)
templates/keep_local.template (+9/-8)
Reviewer Review Type Date Requested Status
Eric Chen Approve
JamesLin Approve
🤖 prod-jenkaas-bootstack (community) continuous-integration Needs Fixing
Tianqi Xiao (community) Approve
BootStack Reviewers Pending
Review via email: mp+448028@code.launchpad.net

Commit message

Fix rsyslog permission denied; remove duplicate logging

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

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

Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Robert Gildein (rgildein) wrote :

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

Revision history for this message
JamesLin (jneo8) wrote :

We need more information about this config change.

review: Needs Information
Revision history for this message
Haw Loeung (hloeung) wrote :

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.

Revision history for this message
Haw Loeung (hloeung) wrote :

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

Revision history for this message
Eric Chen (eric-chen) wrote :

Change the status to WIP until any new commit available.

Revision history for this message
Haw Loeung (hloeung) wrote :

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

Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
JamesLin (jneo8) (last edit ):
Revision history for this message
JamesLin (jneo8) wrote :

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

Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Eric Chen (eric-chen) wrote :

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
Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Eric Chen (eric-chen) wrote :

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

Revision history for this message
Haw Loeung (hloeung) wrote :

Done

Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Tianqi Xiao (txiao) wrote :

Code change LGTM. Re-triggering CI

review: Approve
Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Haw Loeung (hloeung) wrote :

Bump, any updates here?

Revision history for this message
Eric Chen (eric-chen) wrote :

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!

Revision history for this message
Haw Loeung (hloeung) wrote :

Any chance you could run the functional tests locally for me? I'm not sure how to and what dependency packages are required for it to run.

Revision history for this message
JamesLin (jneo8) wrote :
Download full text (3.7 KiB)

I manual verify the functional test. It's pass.

```
2024-05-27 03:57:32 [INFO] Deploying bundle '/home/ubuntu/CanonicalProjects/charm-rsyslog-forwarder-ha/tests/functional/tests/bundles/bionic.yaml' on to 'zaza-8b1c328f83d7' model
2024-05-27 03:57:32 [INFO] Rendered template '<Template 'local-charm-overlay.yaml.j2'>' to file '/home/ubuntu/tmpqa0560hb/local-charm-overlay.yaml'
2024-05-27 03:57:32 [INFO] Rendered template '<Template 'bionic.yaml.j2'>' to file '/home/ubuntu/tmpqa0560hb/bionic.yaml'
2024-05-27 03:57:32 [INFO] Deploying overlay '/home/ubuntu/tmpqa0560hb/local-charm-overlay.yaml' on to 'zaza-8b1c328f83d7' model
2024-05-27 03:57:32 [INFO] Deploying overlay '/home/ubuntu/tmpqa0560hb/bionic.yaml' on to 'zaza-8b1c328f83d7' model
2024-05-27 03:57:34 [INFO] Located charm "rsyslog" in charm-hub, channel latest/stable
2024-05-27 03:57:34 [INFO] Located charm "ubuntu" in charm-hub, channel latest/stable
2024-05-27 03:57:34 [INFO] Executing changes:
2024-05-27 03:57:34 [INFO] - upload charm rsyslog from charm-hub for base ubuntu@18.04/stable with architecture=amd64
2024-05-27 03:57:35 [INFO] - deploy application rsyslog from charm-hub on ubuntu@18.04/stable
2024-05-27 03:57:36 [INFO] - upload charm /home/ubuntu/CanonicalProjects/charm-rsyslog-forwarder-ha//rsyslog-forwarder-ha.charm for base ubuntu@18.04/stable with architecture=amd64
2024-05-27 03:57:37 [INFO] - deploy application rsyslog-forwarder-ha on ubuntu@18.04/stable
2024-05-27 03:57:37 [INFO] - upload charm ubuntu from charm-hub for base ubuntu@18.04/stable with architecture=amd64
2024-05-27 03:57:38 [INFO] - deploy application syslog-source from charm-hub on ubuntu@18.04/stable using ubuntu
2024-05-27 03:57:38 [INFO] - add relation rsyslog-forwarder-ha:syslog - rsyslog:aggregator
2024-05-27 03:57:38 [INFO] - add relation rsyslog-forwarder-ha:juju-info - syslog-source:juju-info
2024-05-27 03:57:38 [INFO] - add unit rsyslog/0 to new machine 0
2024-05-27 03:57:38 [INFO] - add unit syslog-source/0 to new machine 1
2024-05-27 03:57:39 [INFO] Deploy of bundle completed.
2024-05-27 03:57:39 [INFO] Waiting for environment to settle
2024-05-27 03:57:39 [INFO] Timeout for deployment to settle set to: 3600
2024-05-27 03:57:39 [INFO] Maximum resolve count for deployment set to: 0
2024-05-27 03:57:39 [INFO] Waiting for application states to reach targeted states.
2024-05-27 03:57:39 [INFO] Waiting for an application to be present
2024-05-27 03:57:39 [INFO] Now checking workload status and status messages
2024-05-27 03:59:41 [INFO] Application syslog-source is ready.
2024-05-27 03:59:43 [INFO] Application rsyslog-forwarder-ha is ready.
2024-05-27 03:59:51 [INFO] Application rsyslog is ready.
2024-05-27 03:59:51 [INFO] All applications reached approved status, number of units (where relevant), and workload status message checks.
2024-05-27 03:59:51 [INFO] Waiting for zaza-8b1c328f83d7 to settle
2024-05-27 03:59:51 [INFO] Model zaza-8b1c328f83d7 has settled
2024-05-27 03:59:51 [INFO] configuring zaza-8b1c328f83d7
2024-05-27 03:59:51 [INFO] Finished configuring zaza-8b1c328f83d7
2024-05-27 03:59:51 [INFO] Testing zaza-8b1c328f83d7
2024-05-27 03:59:51 [INFO] ## Running Test tests.test_rsyslog_forwa...

Read more...

Revision history for this message
JamesLin (jneo8) :
review: Approve
Revision history for this message
Eric Chen (eric-chen) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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
6import os6import os
7import subprocess7import subprocess
8import sys8import sys
9from shutil import copyfile
109
11_HERE = os.path.abspath(os.path.dirname(__file__))10_HERE = os.path.abspath(os.path.dirname(__file__))
1211
@@ -29,7 +28,7 @@ from charmhelpers.core.host import ( # noqa: E402
29from charmhelpers.fetch import apt_install # noqa:E40228from charmhelpers.fetch import apt_install # noqa:E402
3029
31from model import Server, session # noqa:E40230from model import Server, session # noqa:E402
32from utils import get_template_dir, get_template # noqa:E40231from utils import get_template # noqa:E402
33from utils import __die as die # noqa:E40232from utils import __die as die # noqa:E402
3433
3534
@@ -43,7 +42,6 @@ required_packages = [
4342
4443
45IMFILE_FILE = "/etc/rsyslog.d/40-rsyslog-imfile.conf"44IMFILE_FILE = "/etc/rsyslog.d/40-rsyslog-imfile.conf"
46LOGS_TEMPLATE = "keep_local.template"
47LOGS_SYSTEM_FILE = "/etc/rsyslog.d/50-default.conf"45LOGS_SYSTEM_FILE = "/etc/rsyslog.d/50-default.conf"
48REPLICATION_FILE = "/etc/rsyslog.d/45-rsyslog-replication.conf"46REPLICATION_FILE = "/etc/rsyslog.d/45-rsyslog-replication.conf"
49CERT_FILE = "/etc/rsyslog.d/42-cert-rsyslog.conf"47CERT_FILE = "/etc/rsyslog.d/42-cert-rsyslog.conf"
@@ -88,7 +86,30 @@ def update_imfile(imfiles):
8886
89def update_local_logs(keep=True):87def update_local_logs(keep=True):
90 if keep:88 if keep:
91 copyfile(os.path.join(get_template_dir(), LOGS_TEMPLATE), LOGS_SYSTEM_FILE)89 additional_logs = {
90 "cron.*": "/var/log/cron.log",
91 "daemon.*": "/var/log/daemon.log",
92 "lpr.*": "/var/log/lpr.log",
93 "user.*": "/var/log/user.log",
94 "mail.info": "/var/log/mail.info",
95 "mail.warn": "/var/log/mail.warn",
96 }
97 new = {}
98 for al, logf in additional_logs.items():
99 if os.path.exists(logf):
100 new[al] = logf
101 debug_log = False
102 if os.path.exists("/var/log/debug"):
103 debug_log = True
104 messages_log = False
105 if os.path.exists("/var/log/messages"):
106 messages_log = True
107
108 with open(LOGS_SYSTEM_FILE, "w") as fd:
109 tmpl = get_template("keep_local").render(
110 additional=new, debug_log=debug_log, messages_log=messages_log
111 )
112 fd.write(tmpl)
92 else:113 else:
93 if os.path.exists(LOGS_SYSTEM_FILE):114 if os.path.exists(LOGS_SYSTEM_FILE):
94 os.remove(LOGS_SYSTEM_FILE)115 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
44
5# Some defaults5# Some defaults
6auth,authpriv.* /var/log/auth.log6auth,authpriv.* /var/log/auth.log
7*.*;auth,authpriv.none -/var/log/syslog7*.*;auth,authpriv.none,kern.none,mail.none -/var/log/syslog
8cron.* /var/log/cron.log
9daemon.* -/var/log/daemon.log
10lpr.* -/var/log/lpr.log
11mail.* -/var/log/mail.log8mail.* -/var/log/mail.log
12user.* -/var/log/user.log
13
14mail.info -/var/log/mail.info
15mail.warn -/var/log/mail.warn
16mail.err /var/log/mail.err9mail.err /var/log/mail.err
1710
11{%- for key, log in additional.items() %}
12{{ key }} -{{ log }}
13{%- endfor %}
14
18# Catch-alls15# Catch-alls
16{%- if debug_log %}
19*.=debug;\17*.=debug;\
20 auth,authpriv.none;\18 auth,authpriv.none;\
21 news.none;mail.none -/var/log/debug19 news.none;mail.none -/var/log/debug
20{%- endif %}
21{%- if messages_log %}
22*.=info;*.=notice;*.=warn;\22*.=info;*.=notice;*.=warn;\
23 auth,authpriv.none;\23 auth,authpriv.none;\
24 cron,daemon.none;\24 cron,daemon.none;\
25 mail,news.none -/var/log/messages25 mail,news.none -/var/log/messages
26{%- endif %}

Subscribers

People subscribed via source and target branches

to all changes: