Bug 596970
Summary: | capture non standard log files via syslog | |||
---|---|---|---|---|
Product: | Red Hat Enterprise Linux 5 | Reporter: | Adam Stokes <astokes> | |
Component: | sos | Assignee: | Bryn M. Reeves <bmr> | |
Status: | CLOSED ERRATA | QA Contact: | BaseOS QE - Apps <qe-baseos-apps> | |
Severity: | medium | Docs Contact: | ||
Priority: | low | |||
Version: | 5.7 | CC: | agk, bmr, dkutalek, gavin, hpetty, prc, zbrown | |
Target Milestone: | rc | |||
Target Release: | --- | |||
Hardware: | All | |||
OS: | Linux | |||
Whiteboard: | ||||
Fixed In Version: | sos-1.7-9.52.el5 | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | ||
Clone Of: | ||||
: | 771501 (view as bug list) | Environment: | ||
Last Closed: | 2011-07-21 07:59:53 UTC | Type: | --- | |
Regression: | --- | Mount Type: | --- | |
Documentation: | --- | CRM: | ||
Verified Versions: | Category: | --- | ||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | ||
Cloudforms Team: | --- | Target Upstream Version: | ||
Embargoed: | ||||
Bug Depends On: | ||||
Bug Blocks: | 771501 |
Description
Adam Stokes
2010-05-27 20:19:54 UTC
This bug also applies to one of my customers. We would love to see this applied to RHEL 5 and 6. Fixed in trunk through: if self.getOption('all_logs'): rhelver = self.policy().rhelVersion() if rhelver == 5 or rhelver == 4: logs=self.doRegexFindAll(r"^\S+\s+(\/.*log.*)\s+$", "/etc/syslog.conf") for i in logs: if not os.path.isfile(i): continue self.addCopySpec(i) if rhelver == 6: logs=self.doRegexFindAll(r"^\S+\s+(\/.*log.*)\s+$", "/etc/rsyslog.conf") for i in logs: if not os.path.isfile(i): continue self.addCopySpec(i) all_logs is currently OFF by default. Looks less icky now: 54 if self.getOption('all_logs'): 55 rhelver = self.policy().rhelVersion() 56 logconf = (rhelver in (4, 5)) and "/etc/syslog.conf" \ 57 or "/etc/rsyslog.conf" 58 logs = self.doRegexFindAll(r"^\S+\s+(\/.*log.*)\s+$", logconf) 59 for i in logs: 60 if os.path.isfile(i): 61 self.addCopySpec(i) I see two problems with attached patch: 1) Regexp used in this patch does not match cases with prepended character of '-'. This character can be used to alter syncing logging to given file, and it is by default used for /var/log/maillog: >>> re.findall(r"^\S+\s+(\/.*log.*)\s+$", open("/etc/rsyslog.conf", 'r').read(), re.MULTILINE) ['/var/log/messages', '/var/log/secure', '/var/log/cron', '/var/log/spooler', '/var/log/boot.log'] >>> re.findall(r"^\S+\s+-?(\/.*log.*)\s+$", open("/etc/rsyslog.conf", 'r').read(), re.MULTILINE) ['/var/log/messages', '/var/log/secure', '/var/log/maillog', '/var/log/cron', '/var/log/spooler', '/var/log/boot.log'] 2) Regexp assumes that user configured all log files into directory containing string 'log'. This works correctly for eg. /var/log. But user can log to where ever he wants, isn't it? What about having more universal regexp of r"^\S+\s+-?(\/.*)\s+$" and filter out /dev items afterwards? I think the first point is something we should address (and this should have been tested before the change was committed upstream as it's a common enough usage). The second I am not so sure about. I have seen a few users change this to /var/adm but imho that's crazy - it breaks e.g. logwatch and the standard logrotate setup as well as a few other tools that rely on finding a particular log in /var/log. If the change seems low risk I might be inclined to take it but I don't think we should go too far out of our way to support such configurations (particularly so close to deadlines :). Hmm, actually there may be no need to filter on /dev since we perform an os.path.isfile - that will only evaluate true for a regular file.. Yep, isfile catches that just fine: log: /dev/console log: /var/log/messages logfile: /var/log/messages log: /var/log/secure logfile: /var/log/secure log: /var/log/maillog logfile: /var/log/maillog log: /dev/stupidadm logfile: /dev/stupidadm log: /var/log/cron logfile: /var/log/cron log: /var/log/spooler logfile: /var/log/spooler log: /var/log/boot.log logfile: /var/log/boot.log The regex suggested in comment #10 doesn't seem to play nicely with our doRegexFindAll - instead I moved both the line anchor and -? inside the capture group (to ensure we don't munch multi-line matches or miss '-' prefixed files) and deal with the - afterwards. Ouch. Does not work on setups where rsyslog is installed, but not used. By default, rsyslog (after package install) is not turned on, and old syslog is used. Which means, in these cases, additional log files won't be grabbed. Hmm, yeah that's not ideal - thanks. The original change upstream didn't support rsyslog on older releases at all (see comment #5) and Pierre's revised version (comment #6) did not work in my testing. I think we should probably just simplify this - if either file exists we attempt to parse it and add the logs it defines. The other checks we could do (running process, rc symlinks etc.) all have their own limitations and are also vulnerable to possibly missing important data if a user has made a recent change in logging configuration. I do think that these are all pretty unlikely corner-cases however - I've looked through my sosreport archive this morning and haven't yet found a system with rsyslog installed but not configured. Submitted bug 717167 to take care of these bits. I agree with simplifying to "gather all files defined in both rsyslog.conf and syslog.conf, if those are present", since any diagnostics of which to choose would cause some troubles. I am happy that this is unusual corner-case, thanks for filing bug for it. An advisory has been issued which should help the problem described in this bug report. This report is therefore being closed with a resolution of ERRATA. For more information on therefore solution and/or where to find the updated files, please follow the link below. You may reopen this bug report if the solution does not work for you. http://rhn.redhat.com/errata/RHBA-2011-1028.html |