Compress logs during rotation #7430

Merged
merged 2 commits into from Jun 1, 2017

Conversation

Projects
None yet
6 participants
Owner

howbazaar commented Jun 1, 2017

Description of change

Rotated logs take up a significant amount of disk space, and compressing the logs has been long asked for. Yesterday lumberjack acquired a fix that adds a Compress flag to the configuration struct.

This branch sets that flag, updates the dependency for lumberjack, and fixes the CI tests for log rotation.

QA steps

Manually, I bootstrapped a lxd provider, deployed the fill-logs charm from the CI tests, and ran the fill-logs-units action, and observed compressed logs. I also deployed an older Juju, made sure there were already rotated logs that were uncompressed, then upgraded juju, then forced another rotate, and observed that all existing logs were also compressed.

Documentation changes

I'm not sure it is documentation worthy.

Bug reference

https://bugs.launchpad.net/juju/+bug/1494661

Owner

howbazaar commented Jun 1, 2017

To run the CI test, I had to do the following:

cd acceptancetests
export JUJU_HOME=~/canonical/cloud-city  # as this is kinda needed
export JUJU_REPOSITORY=./repository
mkdir -p ~/sandbox/logs
./assess_log_rotation.py parallel-lxd ~/go/bin/juju ~/sandbox/logs logrotate unit
Owner

Veebers commented Jun 1, 2017

acceptancetests changes look good to me.

axw approved these changes Jun 1, 2017

Member

axw commented Jun 1, 2017

$$merge$$

Contributor

jujubot commented Jun 1, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@@ -158,9 +158,9 @@ def check_expected_backup(key, logprefix, action_output):
(log_name, backup_pattern))
size = int(log["size"])
- if size < 299 or size > 301:
+ if size > 30:
@4a6f656c

4a6f656c Jun 1, 2017

Contributor

As an aside, it could be worth checking both the compressed version, then decompressing it and checking the uncompressed version.. this also has the advantage of ensuring that the log format is understood and decompressable.

@@ -148,7 +148,7 @@ def check_expected_backup(key, logprefix, action_output):
raise LogRotateError(
"Missing backup log '{}' after rotation.".format(key))
- backup_pattern = "/var/log/juju/%s-(.+?)\.log" % logprefix
+ backup_pattern = "/var/log/juju/%s-(.+?)\.log.gz" % logprefix
@jameinel

jameinel Jun 1, 2017

Owner

this feels like it should be .log(.gz)?

something that says we might see it as .log or .log.gz

raise LogRotateError(
- "Backup log '%s' should be ~300MB, but is %sMB." %
+ "Backup log '%s' should be less than 30MB (as gzipped), but is %sMB." %
@jameinel

jameinel Jun 1, 2017

Owner

but I guess this would fail if it was .log

Contributor

jujubot commented Jun 1, 2017

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/11051

Member

axw commented Jun 1, 2017

$$merge$$

Contributor

jujubot commented Jun 1, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Jun 1, 2017

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/11053

Member

axw commented Jun 1, 2017

$$scream$$

Contributor

jujubot commented Jun 1, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit a51b372 into juju:develop Jun 1, 2017

1 check failed

github-check-merge-juju Built PR, ran unit tests, and tested LXD deploy. Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment