Merge ~hloeung/charm-rsyslog-forwarder-ha:master into charm-rsyslog-forwarder-ha:master
- Git
- lp:~hloeung/charm-rsyslog-forwarder-ha
- master
- Merge into master
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) |
Related bugs: |
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
Description of the change
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote : | # |
FAILED: Continuous integration, rev:ebe22fe3e6f
https:/
Executed test runs:
FAILURE: https:/
None: https:/
Click here to trigger a rebuild:
https:/
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.
JamesLin (jneo8) wrote : | # |
We need more information about this config change.
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.
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:/
Eric Chen (eric-chen) wrote : | # |
Change the status to WIP until any new commit available.
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:/
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote : | # |
FAILED: Continuous integration, rev:34b89744660
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
JamesLin (jneo8) (last edit ): | # |
JamesLin (jneo8) wrote : | # |
Overall LGTM. Waiting the CI pass.
Rebuild the CI now.
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote : | # |
FAILED: Continuous integration, rev:34b89744660
https:/
Executed test runs:
FAILURE: https:/
None: https:/
Click here to trigger a rebuild:
https:/
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote : | # |
FAILED: Continuous integration, rev:509b9e069e8
https:/
Executed test runs:
FAILURE: https:/
None: https:/
Click here to trigger a rebuild:
https:/
Eric Chen (eric-chen) wrote : | # |
The default value of black is 88, we don't need the additional parameter.
https:/
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote : | # |
FAILED: Continuous integration, rev:e6a1d2ef41b
https:/
Executed test runs:
FAILURE: https:/
None: https:/
Click here to trigger a rebuild:
https:/
Eric Chen (eric-chen) wrote : | # |
Hi Haw, Can you rebase the code to latest one? Tianqi fixed the function test
https:/
Haw Loeung (hloeung) wrote : | # |
Done
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote : | # |
FAILED: Continuous integration, rev:59425ea225d
https:/
Executed test runs:
ABORTED: https:/
None: https:/
Click here to trigger a rebuild:
https:/
Tianqi Xiao (txiao) wrote : | # |
Code change LGTM. Re-triggering CI
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote : | # |
FAILED: Continuous integration, rev:59425ea225d
https:/
Executed test runs:
FAILURE: https:/
None: https:/
Click here to trigger a rebuild:
https:/
Haw Loeung (hloeung) wrote : | # |
Bump, any updates here?
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!
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.
JamesLin (jneo8) wrote : | # |
I manual verify the functional test. It's pass.
```
2024-05-27 03:57:32 [INFO] Deploying bundle '/home/
2024-05-27 03:57:32 [INFO] Rendered template '<Template 'local-
2024-05-27 03:57:32 [INFO] Rendered template '<Template 'bionic.yaml.j2'>' to file '/home/
2024-05-27 03:57:32 [INFO] Deploying overlay '/home/
2024-05-27 03:57:32 [INFO] Deploying overlay '/home/
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/
2024-05-27 03:57:37 [INFO] - deploy application rsyslog-
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-
2024-05-27 03:57:38 [INFO] - add relation rsyslog-
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-
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_
JamesLin (jneo8) : | # |
Eric Chen (eric-chen) : | # |
Preview Diff
1 | diff --git a/hooks/hooks.py b/hooks/hooks.py | |||
2 | index 19888c7..bb6a0fb 100755 | |||
3 | --- a/hooks/hooks.py | |||
4 | +++ b/hooks/hooks.py | |||
5 | @@ -6,7 +6,6 @@ import base64 | |||
6 | 6 | import os | 6 | import os |
7 | 7 | import subprocess | 7 | import subprocess |
8 | 8 | import sys | 8 | import sys |
9 | 9 | from shutil import copyfile | ||
10 | 10 | 9 | ||
11 | 11 | _HERE = os.path.abspath(os.path.dirname(__file__)) | 10 | _HERE = os.path.abspath(os.path.dirname(__file__)) |
12 | 12 | 11 | ||
13 | @@ -29,7 +28,7 @@ from charmhelpers.core.host import ( # noqa: E402 | |||
14 | 29 | from charmhelpers.fetch import apt_install # noqa:E402 | 28 | from charmhelpers.fetch import apt_install # noqa:E402 |
15 | 30 | 29 | ||
16 | 31 | from model import Server, session # noqa:E402 | 30 | from model import Server, session # noqa:E402 |
18 | 32 | from utils import get_template_dir, get_template # noqa:E402 | 31 | from utils import get_template # noqa:E402 |
19 | 33 | from utils import __die as die # noqa:E402 | 32 | from utils import __die as die # noqa:E402 |
20 | 34 | 33 | ||
21 | 35 | 34 | ||
22 | @@ -43,7 +42,6 @@ required_packages = [ | |||
23 | 43 | 42 | ||
24 | 44 | 43 | ||
25 | 45 | IMFILE_FILE = "/etc/rsyslog.d/40-rsyslog-imfile.conf" | 44 | IMFILE_FILE = "/etc/rsyslog.d/40-rsyslog-imfile.conf" |
26 | 46 | LOGS_TEMPLATE = "keep_local.template" | ||
27 | 47 | LOGS_SYSTEM_FILE = "/etc/rsyslog.d/50-default.conf" | 45 | LOGS_SYSTEM_FILE = "/etc/rsyslog.d/50-default.conf" |
28 | 48 | REPLICATION_FILE = "/etc/rsyslog.d/45-rsyslog-replication.conf" | 46 | REPLICATION_FILE = "/etc/rsyslog.d/45-rsyslog-replication.conf" |
29 | 49 | CERT_FILE = "/etc/rsyslog.d/42-cert-rsyslog.conf" | 47 | CERT_FILE = "/etc/rsyslog.d/42-cert-rsyslog.conf" |
30 | @@ -88,7 +86,30 @@ def update_imfile(imfiles): | |||
31 | 88 | 86 | ||
32 | 89 | def update_local_logs(keep=True): | 87 | def update_local_logs(keep=True): |
33 | 90 | if keep: | 88 | if keep: |
35 | 91 | copyfile(os.path.join(get_template_dir(), LOGS_TEMPLATE), LOGS_SYSTEM_FILE) | 89 | additional_logs = { |
36 | 90 | "cron.*": "/var/log/cron.log", | ||
37 | 91 | "daemon.*": "/var/log/daemon.log", | ||
38 | 92 | "lpr.*": "/var/log/lpr.log", | ||
39 | 93 | "user.*": "/var/log/user.log", | ||
40 | 94 | "mail.info": "/var/log/mail.info", | ||
41 | 95 | "mail.warn": "/var/log/mail.warn", | ||
42 | 96 | } | ||
43 | 97 | new = {} | ||
44 | 98 | for al, logf in additional_logs.items(): | ||
45 | 99 | if os.path.exists(logf): | ||
46 | 100 | new[al] = logf | ||
47 | 101 | debug_log = False | ||
48 | 102 | if os.path.exists("/var/log/debug"): | ||
49 | 103 | debug_log = True | ||
50 | 104 | messages_log = False | ||
51 | 105 | if os.path.exists("/var/log/messages"): | ||
52 | 106 | messages_log = True | ||
53 | 107 | |||
54 | 108 | with open(LOGS_SYSTEM_FILE, "w") as fd: | ||
55 | 109 | tmpl = get_template("keep_local").render( | ||
56 | 110 | additional=new, debug_log=debug_log, messages_log=messages_log | ||
57 | 111 | ) | ||
58 | 112 | fd.write(tmpl) | ||
59 | 92 | else: | 113 | else: |
60 | 93 | if os.path.exists(LOGS_SYSTEM_FILE): | 114 | if os.path.exists(LOGS_SYSTEM_FILE): |
61 | 94 | os.remove(LOGS_SYSTEM_FILE) | 115 | os.remove(LOGS_SYSTEM_FILE) |
62 | diff --git a/templates/keep_local.template b/templates/keep_local.template | |||
63 | index 5577674..8c6c76b 100644 | |||
64 | --- a/templates/keep_local.template | |||
65 | +++ b/templates/keep_local.template | |||
66 | @@ -4,22 +4,23 @@ kern.* -/var/log/kern.log | |||
67 | 4 | 4 | ||
68 | 5 | # Some defaults | 5 | # Some defaults |
69 | 6 | auth,authpriv.* /var/log/auth.log | 6 | auth,authpriv.* /var/log/auth.log |
74 | 7 | *.*;auth,authpriv.none -/var/log/syslog | 7 | *.*;auth,authpriv.none,kern.none,mail.none -/var/log/syslog |
71 | 8 | cron.* /var/log/cron.log | ||
72 | 9 | daemon.* -/var/log/daemon.log | ||
73 | 10 | lpr.* -/var/log/lpr.log | ||
75 | 11 | mail.* -/var/log/mail.log | 8 | mail.* -/var/log/mail.log |
76 | 12 | user.* -/var/log/user.log | ||
77 | 13 | |||
78 | 14 | mail.info -/var/log/mail.info | ||
79 | 15 | mail.warn -/var/log/mail.warn | ||
80 | 16 | mail.err /var/log/mail.err | 9 | mail.err /var/log/mail.err |
81 | 17 | 10 | ||
82 | 11 | {%- for key, log in additional.items() %} | ||
83 | 12 | {{ key }} -{{ log }} | ||
84 | 13 | {%- endfor %} | ||
85 | 14 | |||
86 | 18 | # Catch-alls | 15 | # Catch-alls |
87 | 16 | {%- if debug_log %} | ||
88 | 19 | *.=debug;\ | 17 | *.=debug;\ |
89 | 20 | auth,authpriv.none;\ | 18 | auth,authpriv.none;\ |
90 | 21 | news.none;mail.none -/var/log/debug | 19 | news.none;mail.none -/var/log/debug |
91 | 20 | {%- endif %} | ||
92 | 21 | {%- if messages_log %} | ||
93 | 22 | *.=info;*.=notice;*.=warn;\ | 22 | *.=info;*.=notice;*.=warn;\ |
94 | 23 | auth,authpriv.none;\ | 23 | auth,authpriv.none;\ |
95 | 24 | cron,daemon.none;\ | 24 | cron,daemon.none;\ |
96 | 25 | mail,news.none -/var/log/messages | 25 | mail,news.none -/var/log/messages |
97 | 26 | {%- endif %} |
This merge proposal is being monitored by mergebot. Change the status to Approved to merge.