[YT-CC-1189] Overlay ey-backup attributes to move MySQL backup to DB slave. #280
Conversation
| @@ -41,7 +41,13 @@ | |||
| end | |||
| end | |||
|
|
|||
| if node.dna['backup_window'] != 0 && ['db_master','solo'].include?(node.dna['instance_role']) | |||
| is_backup_instance = if node['db_backup']['mysql']['backup_slave'] | |||
There was a problem hiding this comment.
It took me a minute to get my head around this. Would it be clearer to assign is_backup_instance explicitly in the two different cases, rather than assigning the result of the if statement? (It would have been clearer to me :D )
| @@ -41,7 +41,13 @@ | |||
| end | |||
| end | |||
|
|
|||
| if node.dna['backup_window'] != 0 && ['db_master','solo'].include?(node.dna['instance_role']) | |||
| is_backup_instance = if node['db_backup']['mysql']['backup_slave'] | |||
| ['db_slave'].include?(node.dna['instance_role']) && node['db_backup']['mysql']['db_slave_name'] == node['dna']['name'] | |||
There was a problem hiding this comment.
I think this change means that the customer must specify the name of the replica which should run the backup. What happens if they don't? What happens if the name they specify doesn't exist? Is it possible to gracefully fall back to the same behaviour as PostgreSQL in either of those cases, and just choose the first replica?
(If so then it would be good to have this behaviour for both, and simply set default['db_backup']['mysql']['backup_slave'] true for PostgreSQL while leaving default['db_backup']['mysql']['db_slave_name'] empty by default.)
There was a problem hiding this comment.
Obviously we would rename the variables if we were making this more general.
Perhaps:
default['db_backup']['run_backup_on_replica']
default['db_backup']['backup_replica_name']
jegasega
changed the title
Description of your patch
Adds attributes/default.rb to the ey-backup cookbook that determines if the MySQL backup is setup on the master (default) or replica. A sample cookbook in custom-cookbooks is provided to move it to replica.
Fixes #218.
Recommended Release Notes
MySQL backup location is now configurable through attributes overlay.
Estimated risk
Medium - changes the ey-backup/recipes/mysql.rb that's used it many existing environments.
Components involved
MySQL backup
Description of testing done
I setup a test environment running stack stable-v5-3.0.32. I used the custom-cookbooks/db-backup/cookbooks/ey-backup as the base custom cookbook. Then, I had to copy cookbooks/ey-lib due to recent RDS-related changes that's not yet released in stable-v5-3.0.32. After running chef, MySQL backup cron job was removed from the master and added to the replica, as expected. Removing this custom cookbook reverted the change.
QA Instructions
TODO (after code review, in case there are major changes needed)