Nir Soffer has posted comments on this change.
Change subject: vdsm: Rely on system for logrotation
......................................................................
Patch Set 1:
(4 comments)
https://gerrit.ovirt.org/#/c/63682/1//COMMIT_MSG
Commit Message:
Line 10: is no real reason to invoke logrotate manualy.
Line 11:
Line 12: Also if we rely on system to run logrotate we can remove the dependency
Line 13: on it from RPMs. It is enough to install the configuration file into the
Line 14: the directory with logrotate configuration.
It's not our problem. Whether logrotate is installed or not
should be admin
This approach is problematic.
Previously we rotated the log, now you want to let the user decide. We don't have any
user or customer bug complaining about rotated logs, and we know that vdsm logs are huge.
I don't think we can do this, this can be harmful on machines with small harddisks
(like node).
Using logrotate is little nicer, filling /var/log is not. Please require logrotate in this
patch.
If a user want to disable vdsm log rotation she can disable the vdsm logrotate
configuration or edit it.
Line 15:
Line 16: The patch also renames script vdsm-logrotate to vdsm-remove-logs because
Line 17: it doesn't run logrotate anymore. It only removes old log files.
Line 18:
https://gerrit.ovirt.org/#/c/63682/1/static/Makefile.am
File static/Makefile.am:
Line 36: ./etc/vdsm/mom.d/04-cputune.policy \
Line 37: ./etc/vdsm/mom.d/05-iotune.policy \
Line 38: $(NULL)
Line 39:
Line 40: vdsmconfrotatedir = $(sysconfdir)/logrotate.d
You mean to define new variable like:
Yes
Line 41:
Line 42: vdsmconfrotate_DATA = \
Line 43: ./etc/logrotate.d/vdsm \
Line 44: $(NULL)
https://gerrit.ovirt.org/#/c/63682/1/vdsm/vdsm-remove-logs
File vdsm/vdsm-remove-logs:
Line 10: if [ -d /var/log/core ] ; then
Line 11: /usr/bin/find /var/log/core -type f -name '*xz' -mtime +7 -exec
/bin/rm -f '{}' \;
Line 12: EXITVALUE=$?
Line 13: if [ $EXITVALUE != 0 ]; then
Line 14: /usr/bin/logger -t logrotate "ALERT clean old core files exited
abnormally with [$EXITVALUE]"
If we depend on logrotate running from cron, why do we need to run it here?
Line 15: fi
Line 16: fi
Line 17:
Line 18: if [ -d /var/log/vdsm/import ] ; then
Line 18: if [ -d /var/log/vdsm/import ] ; then
Line 19: /usr/bin/find /var/log/vdsm/import -type f -mtime +30 -exec /bin/rm -f
'{}' \;
Line 20: EXITVALUE=$?
Line 21: if [ $EXITVALUE != 0 ]; then
Line 22: /usr/bin/logger -t logrotate "ALERT clean of old import log files
exited abnormally with [$EXITVALUE]"
Same
Line 23: fi
Line 24: fi
Line 25:
--
To view, visit
https://gerrit.ovirt.org/63682
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica888244bd7c65121f55983e5716a6eae5662879
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky <tgolembi(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Tomas Golembiovsky <tgolembi(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes