Added an option to skip adding /var/lib/juju to juju-crashdump. #3

Merged
merged 1 commit into from Feb 3, 2017

Conversation

Projects
None yet
5 participants
Collaborator

petevg commented Feb 2, 2017

Useful for automated testing frameworks, which don't necessarily want to
create a huge tarball each time.

@johnsca, @kwmonroe Thinking about this, I think that it makes sense to include /var/lib/juju in juju-crashdump when it is run manually. You are trying to diagnose a problem, and saving off the state of the /var/lib/juju dir is useful for that diagnosis.

I don't think that it's so useful that it needs to be included in the automated tests, though. If you agree with this approach, the next step is to pass "-s" in matrix (and bundletester).

... and the next step after that might be to only crashdump when we have a test failure.

Added an option to skip adding /var/lib/juju to juju-crashdump.
Useful for automated testing frameworks, which don't necessarily want to
create a huge tarball each time.

LGTM +1 . Not merging yet since you are asking for the feedback of @johnsca and @kwmonroe

johnsca approved these changes Feb 3, 2017

I'm fine with this approach.

Collaborator

lutostag commented Feb 3, 2017

Which files are large in there, we have a 5MB limit for files to include in the made bundle. Is that not being followed?

Member

kwmonroe commented Feb 3, 2017

@lutostag it's not a single file that's large -- it's the /var/lib/juju/agents/unit-foo/charm/.venv that hurts (that dir can easily get to 10s of MBs).

+1, lgtm

Member

johnsca commented Feb 3, 2017

I wasn't aware of the 5MB limit. Maybe it makes more sense to simply blacklist the .venv directory?

Collaborator

lutostag commented Feb 3, 2017

Should probably only be collected on actual test failures. But, I do like the idea of a --small flag, that we can have a better rule for what not to include in the future. This is a good start.
+1

Member

johnsca commented Feb 3, 2017

Per discussion on IRC, we're all +1 on this PR with the understanding that the heuristic for "small" is improved in the future.

@johnsca johnsca merged commit 02c7251 into master Feb 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment