Skip to content
This repository has been archived by the owner on Jan 22, 2023. It is now read-only.

Add command line params and dockerize #14

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

bkrodgers
Copy link

Hi, great work on your script! I made a few changes I'd like to submit back to you.

  1. I added an optional command line parameter to specify which volume on the machine you want to back up, based on the device name it's attached as. If it's not specified, it will back up everything as it normally would.
  2. I added an optional command line parameter to specify retention days.
  3. I created a Dockerfile to build a docker image for this script. (I use coreOS). For now I just am publishing to my private docker repo, but if you're interested in having this on docker hub, you can look into setting up an automated build that would be triggered when you make changes: https://docs.docker.com/docker-hub/builds/
  4. I added an explicit exit 0. I'm not sure if it was docker or systemd, but one or the other wasn't seeing it as successfully completing without it and was going into a restart loop.
  5. Instead of tagging the snapshots with $(hostname), I switched it to use the instance ID. This was primarily to make it docker friendly, as hostname in a docker container gives back a meaningless internal docker hostname.

Let me know if there's anything you'd like me to change!

@jboeshart
Copy link
Contributor

For 5., there is value in many cases in using the hostname, so I think it should probably stay, but do like the idea of having the instance ID in there, so maybe a compromise would be to have both? Something like:

snapshot_description="$(hostname)-$instance_id-$device_name-backup-$(date +%Y-%m-%d)"

It's a little long, but gives a pretty comprehensive summary of what the particular snapshot represents. Another option would be to add it as a tag to the snapshot, rather than the description.

@bkrodgers
Copy link
Author

OK, I just pushed an option that checks if you're running inside docker (docker containers get a /.dockerinit file mounted inside them). If inside docker, it'll leave the hostname out, otherwise it includes it along with the instance id.

I also added two tags "CreatedFromInstance" and "InstanceDevice" to the snapshot, to make it a bit easier to search by those if needed. We could add a hostname tag as well if you think that's valuable.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants