-
Notifications
You must be signed in to change notification settings - Fork 125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Major overhaul #26
Open
zhimsel
wants to merge
76
commits into
evannuil:master
Choose a base branch
from
zhimsel:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Major overhaul #26
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Selects volumes to snapshot from tags on instances instead of volumes.
Fixed errors reported by flake8 (except the complexity of the for-loop on line 151)
It's more descriptive in the config file
Makes it a little more readable and removes redundant information
Reflects structural changes to script (e.g. it uses instance tags instead of volume tags).
TAGS: Autosnap will now tag the snapshots it makes with the same user-configured tag specified in the config file. It will also use that tag to filter out which snapshots to delete and not delete. DELETION: Autosnap now bases its deletion decisions based on the NUMBER of snapshots, not the AGE of the snapshots. This allows two things to happen: 1. This can be run in an on-demand basis instead of having to be run on a schedule. 2. This prevents autosnap from deleting all of the snapshots for a volume if it hasn't backed up in more than X number of days. For instance, if the script fails to run over a weekend and you only want to keep the last 3 days, it would previously have deleted all the snapshots if you came back on Monday and manually ran the script (or fixed the issue). INTERVAL: This commit also removes the concept of 'daily', 'weekly' or 'monthly' snapshots. The new setting only configures how many snapshots you want to keep, period. No more having different sets of snapshots to keep track of.
If the tag 'autosnap_limit' exists on any instance it's snapshotting, autosnap will use that value instead of the global setting in the config file.
Stupid dashes
Very useful for testing.
Any volume that is part of an instance that we snapshot will be ignored if the tag 'autosnap_ignore' is present (doesn't matter what the value is).
Major design change, but I feel it simplifies dealing with instance tags. Instead of setting two tags, autosnap:true and autosnap_frequency:X, you just set one tag, autosnap:X. Also changes what autosnap tags the new snapshots as to the custom 'tag_name' setting, instead of the hardcoded 'autosnap'. This allows users to run multiple copies of this script with different tags.
If there are no current snapshots, it would give an error and not take one (therefore "new" instances would never be snapshotted). This commit fixes that. If the snapshot list is empty, it forces a snapshot (skipping the date compare).
Fixes error with new instances having no snapshots, among other things
Use the config.get() function instead of reading directly. This necessitates a change in how a dry-run is checked for ('if' instead of 'try').
It's basically a whole new script, anyway...
That's a different tool now and less useful for me. I wouldn't merge it here. |
Yeah, this feels like a fork that you should release separately. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feel free to merge this PR if you'd like. It's a major overhaul of the script with lots of improvements, but also includes some architectual changes as well as a new name.
Changes (might not be exhaustive):