Closed 4a6f656c closed 7 years ago
First two commits look good, thanks for the cleanup. Looking at the compression code now (obviously a bigger and more complicate change).
Ack, thanks.
@natefinch any further progress? Anything I can assist with or clarify?
Hey @natefinch, was really hoping to get this into Juju 2.2-rc1 (tomorrow).
@howbazaar crap, sorry. Reviewing now.
Thanks @natefinch, I really appreciate it.
@4a6f656c tested this with Juju and the fill-logs charm that the CI test uses.
$ ll -h
total 7.6M
drwxr-xr-x 2 syslog adm 5 May 31 02:53 ./
drwxrwxr-x 8 root syslog 18 May 31 02:47 ../
-rw------- 1 syslog syslog 25K May 31 02:47 machine-0.log
-rw-r--r-- 1 root root 1.5M May 31 02:53 unit-fill-logs-0-2017-05-31T02-53-23.026.log.gz
-rw------- 1 syslog syslog 6.0M May 31 02:53 unit-fill-logs-0.log
We'd want the rolled file to have the same owner/group as the source file.
@natefinch I don't suppose you recall how the syslog user is set as the file owner/group in Juju do you?
ahh yeah, the file is initially created by something else as syslog, I forget what, and there's code in lumberjack when it rotates a file to copy over the mode and chown it to what the last file was. That mode and chown code will need to be reused for the zipped file.
See this chunk here: https://github.com/natefinch/lumberjack/blob/v2.0/lumberjack.go#L206
Other than that, this looks good.
@natefinch, @howbazaar - done.
Thanks for all your work, Joel. This looks really good.
This change adds support for compressing rotated log files.
Several other clean ups and specifically test improvements are included as separate commits.
Fixes issue #13