Adds functionality to cleanup juju daemons and lxc cruft #41

Merged
merged 2 commits into from Jan 12, 2015

Conversation

Projects
None yet
2 participants
Contributor

codersquid commented Jan 5, 2015

Please do not merge this. I have questions and it is not fully tested.

I'm making a pull request based on a conversation in irc about cleanup. When my local environment is in a bad state I've been running http://paste.ubuntu.com/9677575/ (cobbled from a discussion in irc and this answer). In irc we were discussing clean and it prompted a PR nudge.

Questions/Comments about this PR

  • This destroys any containers named ^juju.*template$ but I may have the naming convention wrong. I just guessed.
  • Maybe the lxc logic should be considered a separate concern and handled outside of this plugin, but to be honest I'd just keep using my script that destroys everything. Maybe that would be true for the majority of users.
  • Is this deleting more files than it should?
  • This plugin allows a user to specify a non-local environment. Should I check that the environment is local before raining destruction down on the local containers, caches, daemons, etc.?
  • I think you'll want to have a verbose option but maybe YAGNI
  • How forgiving should this be? should I let things fail if this fails to stop a service or destroy a container?
juju-clean
echo "Removing any extra files" >&2
+echo "WARNING: This plugin does not handle encrypted home directories yet." >&2
@marcoceppi

marcoceppi Jan 5, 2015

Owner

To handle encrypted home directories there should be a JUJU_HOME=${JUJU_HOME:-~/.juju} then reference everything with $JUJU_HOME in the script.

Owner

marcoceppi commented Jan 5, 2015

  • This destroys any containers named ^juju.*template$ but I may have the naming convention wrong. I just guessed.

That should work, on my machine it pull the proper list of templates. Trashing the templates won't cause any adverse effects to an existing deployment, when another machine is needed the template will be regenerated.

  • Maybe the lxc logic should be considered a separate concern and handled outside of this plugin, but to be honest I'd just keep using my script that destroys everything. Maybe that would be true for the majority of users.

I think this logic is fine, it won't burn any actual deployed LXC machines outside of juju templates.

  • Is this deleting more files than it should?

Doesn't appear so

  • This plugin allows a user to specify a non-local environment. Should I check that the environment is local before raining destruction down on the local containers, caches, daemons, etc.?

Probably. Ideally I'd like to dwarf and replace this command with the juju-local plugin that is under development as it's written in Python and is structured for better sanity checking. Parsing YAML within bash isn't always fun.

  • I think you'll want to have a verbose option but maybe YAGNI

Definitely, you're welcome to do so :)

  • How forgiving should this be? should I let things fail if this fails to stop a service or destroy a container?

It should just power through, it might be a good idea to let things fail for users to be aware but it shouldn't halt the script, just a message.

use JUJU_HOME with ~/.juju as a default
allow service stop to fail without bringing down the script
Contributor

codersquid commented Jan 12, 2015

hi @marcoceppi I started using JUJU_HOME as you suggested, and also added an || true to the service stop so that it would be more forgiving if that failed.

I'm leaving it as is with respect to checking for a local environment, I don't know the best way to do that yet and will leave it for a future commit.

but at any rate, you were going to have this functionality in a python script in the future? that will be nice.

Owner

marcoceppi commented Jan 12, 2015

LGTM, thanks!

marcoceppi added a commit that referenced this pull request Jan 12, 2015

Merge pull request #41 from codersquid/clean_with_services
Adds functionality to cleanup juju daemons and lxc cruft

@marcoceppi marcoceppi merged commit 54aa813 into juju:master Jan 12, 2015

@codersquid codersquid deleted the codersquid:clean_with_services branch Jan 12, 2015

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