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.
What if logrotate is not installed on the system?
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
This looks more like syslogrotatedir - nothing in this path is related to vdsm, so better
update the name.
Line 41:
Line 42: vdsmconfrotate_DATA = \
Line 43: ./etc/logrotate.d/vdsm \
Line 44: $(NULL)
https://gerrit.ovirt.org/#/c/63682/1/vdsm.spec.in
File vdsm.spec.in:
Line 954: %{_tmpfilesdir}/%{vdsm_name}.conf
Line 955: %{_sysconfdir}/dhcp/dhclient.d/sourceRoute.sh
Line 956: %{_sysconfdir}/modprobe.d/vdsm-bonding-modprobe.conf
Line 957: %{_sysconfdir}/sudoers.d/50_vdsm
Line 958: %{_sysconfdir}/cron.hourly/vdsm-remove-logs
Why do we need to remove logs if logrotate is handling the logs?
Line 959: %{_sysconfdir}/libvirt/hooks/qemu
Line 960: %{_libexecdir}/%{vdsm_name}/curl-img-wrap
Line 961: %{_libexecdir}/%{vdsm_name}/fc-scan
Line 962: %{_libexecdir}/%{vdsm_name}/persist-vdsm-hooks
https://gerrit.ovirt.org/#/c/63682/1/vdsm/vdsm-remove-logs
File vdsm/vdsm-remove-logs:
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]"
Line 23: fi
Line 24: fi
Can we use logrotate for this?
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: 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: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes