From 8c35a87d58bbd7fe76cf035cdb36edf166923d90 Mon Sep 17 00:00:00 2001 From: Colin Panisset Date: Wed, 23 Aug 2017 08:14:33 +1000 Subject: [PATCH 1/3] Proposed fix for issue #5 --- lambda/backuplambda.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/lambda/backuplambda.py b/lambda/backuplambda.py index 6a43ed5..719d29b 100755 --- a/lambda/backuplambda.py +++ b/lambda/backuplambda.py @@ -324,9 +324,22 @@ def snapshot_resource(self, resource, description, tags): date = datetime.today().strftime('%d-%m-%Y-%H-%M-%S') snapshot_id = self.period+'-'+self.resolve_backupable_id(resource)+"-"+date+"-"+self.date_suffix - current_snap = self.conn.create_db_snapshot(DBInstanceIdentifier=self.resolve_backupable_id(resource), - DBSnapshotIdentifier=snapshot_id, - Tags=aws_tagset) + if is_cluster(resource): + current_snap = self.conn.create_db_cluster_snapshot( + DBClusterIdentifier=self.resolve_backupable_id(resource), + DBClusterSnapshotIdentifier=snapshot_id, + Tags=aws_tagset) + else: + current_snap = self.conn.create_db_snapshot( + DBInstanceIdentifier=self.resolve_backupable_id(resource), + DBSnapshotIdentifier=snapshot_id, + Tags=aws_tagset) + + def is_cluster(self, resource): + try: + return resource['DBClusterIdentifier'] is not None + except: + return False def list_snapshots_for_resource(self, resource): snapshots = self.conn.describe_db_snapshots(DBInstanceIdentifier=self.resolve_backupable_id(resource), From ad27d82cce2ba92b1970e872deea143c70a769f2 Mon Sep 17 00:00:00 2001 From: Steve Mactaggart Date: Wed, 23 Aug 2017 22:41:49 +1000 Subject: [PATCH 2/3] Fixed issues and added new tests to cover changes. - missing `self` call - removed exception for flow control. --- development.txt | 1 + lambda/backuplambda.py | 7 ++---- tests/tests.py | 53 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/development.txt b/development.txt index 80fb273..71eb2ed 100644 --- a/development.txt +++ b/development.txt @@ -20,3 +20,4 @@ s3transfer==0.1.10 six==1.10.0 Werkzeug==0.11.15 xmltodict==0.10.2 +mock==2.0.0 diff --git a/lambda/backuplambda.py b/lambda/backuplambda.py index 719d29b..37dabc8 100755 --- a/lambda/backuplambda.py +++ b/lambda/backuplambda.py @@ -324,7 +324,7 @@ def snapshot_resource(self, resource, description, tags): date = datetime.today().strftime('%d-%m-%Y-%H-%M-%S') snapshot_id = self.period+'-'+self.resolve_backupable_id(resource)+"-"+date+"-"+self.date_suffix - if is_cluster(resource): + if self.is_cluster(resource): current_snap = self.conn.create_db_cluster_snapshot( DBClusterIdentifier=self.resolve_backupable_id(resource), DBClusterSnapshotIdentifier=snapshot_id, @@ -336,10 +336,7 @@ def snapshot_resource(self, resource, description, tags): Tags=aws_tagset) def is_cluster(self, resource): - try: - return resource['DBClusterIdentifier'] is not None - except: - return False + return 'DBClusterIdentifier' in resource def list_snapshots_for_resource(self, resource): snapshots = self.conn.describe_db_snapshots(DBInstanceIdentifier=self.resolve_backupable_id(resource), diff --git a/tests/tests.py b/tests/tests.py index 4237e39..7b6cf69 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -2,6 +2,8 @@ import boto3 import json +from mock import MagicMock, call, ANY + from moto import mock_ec2, mock_sns from backuplambda import * @@ -48,6 +50,57 @@ def test_resolve_resource_bytag(self): assert len(volumes) == 1 +class RDSBackupManagerTest(unittest.TestCase): + + def test_snapshot_resource_for_standard(self): + + mgr = RDSBackupManager(rds_region_name="ap-southeast-1", + period="day", + tag_name="Snapshot", + tag_value="True", + date_suffix="dd", + keep_count="2") + + mgr.conn = MagicMock() + + resource = {"DBInstanceIdentifier": "db-1234"} + + description = "" + tags = {"value1": "value2"} + + mgr.snapshot_resource(resource, description, tags) + + self.assertEqual(mgr.conn.mock_calls, [ + call.create_db_snapshot(DBInstanceIdentifier='db-1234', + DBSnapshotIdentifier=ANY, + Tags=[{'Value': 'value2', 'Key': 'value1'}]) + ]) + + def test_snapshot_resource_for_cluster(self): + + mgr = RDSBackupManager(rds_region_name="ap-southeast-1", + period="day", + tag_name="Snapshot", + tag_value="True", + date_suffix="dd", + keep_count="2") + + mgr.conn = MagicMock() + + resource = {"DBInstanceIdentifier": "db-1234", "DBClusterIdentifier": "cluster-5678"} + + description = "" + tags = {"value1": "value2"} + + mgr.snapshot_resource(resource, description, tags) + + self.assertEqual(mgr.conn.mock_calls, [ + call.create_db_cluster_snapshot(DBClusterIdentifier='db-1234', + DBClusterSnapshotIdentifier=ANY, + Tags=[{'Value': 'value2', 'Key': 'value1'}]) + ]) + + class LambdaHandlerTest(unittest.TestCase): @mock_ec2 From a6ea17dd17c1c2db6415858b325b85ac5a04cc0b Mon Sep 17 00:00:00 2001 From: Steve Mactaggart Date: Thu, 24 Aug 2017 00:01:18 +1000 Subject: [PATCH 3/3] Added more logic to determine RDS cluster for listing and deleting snapshots. --- lambda/backuplambda.py | 38 +++++++++++--- tests/tests.py | 114 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 142 insertions(+), 10 deletions(-) diff --git a/lambda/backuplambda.py b/lambda/backuplambda.py index 37dabc8..02a8c47 100755 --- a/lambda/backuplambda.py +++ b/lambda/backuplambda.py @@ -123,7 +123,8 @@ def process_backup(self): for i in range(delta): self.message += ' Deleting snapshot ' + self.resolve_snapshot_name(deletelist[i]) + '\n' - self.delete_snapshot(deletelist[i]) + self.delete_snapshot(resource=backup_item, + snapshot=deletelist[i]) total_deletes += 1 # time.sleep(3) except Exception as ex: @@ -156,7 +157,7 @@ def process_backup(self): "total_deletes": total_deletes, } - def delete_snapshot(self, snapshot): + def delete_snapshot(self, resource, snapshot): pass @@ -242,7 +243,7 @@ def resolve_snapshot_name(self, resource): def resolve_snapshot_time(self, resource): return resource['StartTime'] - def delete_snapshot(self, snapshot): + def delete_snapshot(self, resource, snapshot): self.conn.delete_snapshot(SnapshotId=snapshot["SnapshotId"]) @@ -336,25 +337,46 @@ def snapshot_resource(self, resource, description, tags): Tags=aws_tagset) def is_cluster(self, resource): - return 'DBClusterIdentifier' in resource + return 'DBClusterIdentifier' in resource or 'DBClusterSnapshotIdentifier' in resource def list_snapshots_for_resource(self, resource): - snapshots = self.conn.describe_db_snapshots(DBInstanceIdentifier=self.resolve_backupable_id(resource), - SnapshotType='manual') + + if self.is_cluster(resource): + snapshots = self.conn.describe_db_cluster_snapshots(DBClusterIdentifier=self.resolve_backupable_id(resource), + SnapshotType='manual') + else: + snapshots = self.conn.describe_db_snapshots(DBInstanceIdentifier=self.resolve_backupable_id(resource), + SnapshotType='manual') return snapshots['DBSnapshots'] def resolve_backupable_id(self, resource): + + if self.is_cluster(resource): + return resource["DBClusterIdentifier"] + return resource["DBInstanceIdentifier"] def resolve_snapshot_name(self, resource): + + if self.is_cluster(resource): + return resource['DBClusterSnapshotIdentifier'] + return resource['DBSnapshotIdentifier'] def resolve_snapshot_time(self, resource): now = datetime.utcnow() return resource.get('SnapshotCreateTime', now) - def delete_snapshot(self, snapshot): - self.conn.delete_db_snapshot(DBSnapshotIdentifier=snapshot["DBSnapshotIdentifier"]) + def delete_snapshot(self, resource, snapshot): + + if self.is_cluster(resource): + self.conn.delete_db_cluster_snapshot( + DBClusterSnapshotIdentifier=snapshot['DBClusterSnapshotIdentifier'] + ) + else: + self.conn.delete_db_snapshot( + DBSnapshotIdentifier=snapshot["DBSnapshotIdentifier"] + ) def db_has_tag(self, db_instance): arn = self.build_arn(db_instance) diff --git a/tests/tests.py b/tests/tests.py index 7b6cf69..376b5a0 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -43,7 +43,7 @@ def test_resolve_resource_bytag(self): tag_name="Snapshot", tag_value="True", date_suffix="dd", - keep_count="2") + keep_count=2) volumes = mgr.get_backable_resources() @@ -52,6 +52,116 @@ def test_resolve_resource_bytag(self): class RDSBackupManagerTest(unittest.TestCase): + def test_process_backup_no_instances(self): + + mgr = RDSBackupManager(rds_region_name="ap-southeast-1", + period="day", + tag_name="Snapshot", + tag_value="True", + date_suffix="dd", + keep_count=2) + + mgr.conn = MagicMock() + mgr.conn.describe_db_instances = MagicMock(return_value={"DBInstances": []}) + + result = mgr.process_backup() + + self.assertEqual(mgr.conn.mock_calls, [ + call.describe_db_instances() + ]) + + self.assertEqual({'total_errors': 0, + 'total_creates': 0, + 'total_deletes': 0, + 'total_resources': 0}, + result) + + def test_process_backup_single_instance(self): + + mgr = RDSBackupManager(rds_region_name="ap-southeast-1", + period="day", + tag_name="Snapshot", + tag_value="True", + date_suffix="dd", + keep_count=2) + + instance = {'DBInstanceIdentifier': "db-id1234"} + list_tags = {"TagList": [{"Key": "Snapshot", "Value": "True"}]} + + mock_sg = MagicMock() + mock_db_snapshots = {'DBSnapshots': []} + + mgr.conn = MagicMock() + mgr.conn.describe_db_instances = MagicMock(return_value={"DBInstances": [instance]}) + mgr.conn.describe_db_security_groups = MagicMock(return_value=mock_sg) + mgr.conn.meta.region_name = "blart-bling-1" + mgr.conn.list_tags_for_resource = MagicMock(return_value=list_tags) + mgr.conn.describe_db_snapshots = MagicMock(return_value=mock_db_snapshots) + + result = mgr.process_backup() + + self.assertEqual(mgr.conn.mock_calls, [ + call.describe_db_instances(), + call.describe_db_security_groups(), + call.list_tags_for_resource( + ResourceName="arn:aws:rds:blart-bling-1:0:db:db-id1234"), + call.list_tags_for_resource( + ResourceName="arn:aws:rds:blart-bling-1:0:db:db-id1234"), + call.create_db_snapshot(DBInstanceIdentifier='db-id1234', + DBSnapshotIdentifier=ANY, + Tags=[{'Value': 'True', 'Key': 'Snapshot'}]), + call.describe_db_snapshots(DBInstanceIdentifier='db-id1234', SnapshotType='manual'), + ]) + + self.assertEqual({'total_errors': 0, + 'total_creates': 1, + 'total_deletes': 0, + 'total_resources': 1}, + result) + + def test_process_backup_single_cluster_instance(self): + + mgr = RDSBackupManager(rds_region_name="ap-southeast-1", + period="day", + tag_name="Snapshot", + tag_value="True", + date_suffix="dd", + keep_count=2) + + instance = {'DBInstanceIdentifier': "db-id1234", "DBClusterIdentifier": "cluster-5678"} + list_tags = {"TagList": [{"Key": "Snapshot", "Value": "True"}]} + + mock_sg = MagicMock() + mock_db_snapshots = {'DBSnapshots': []} + + mgr.conn = MagicMock() + mgr.conn.describe_db_instances = MagicMock(return_value={"DBInstances": [instance]}) + mgr.conn.describe_db_security_groups = MagicMock(return_value=mock_sg) + mgr.conn.meta.region_name = "blart-bling-1" + mgr.conn.list_tags_for_resource = MagicMock(return_value=list_tags) + mgr.conn.describe_db_cluster_snapshots = MagicMock(return_value=mock_db_snapshots) + + result = mgr.process_backup() + + self.assertEqual(mgr.conn.mock_calls, [ + call.describe_db_instances(), + call.describe_db_security_groups(), + call.list_tags_for_resource( + ResourceName="arn:aws:rds:blart-bling-1:0:db:db-id1234"), + call.list_tags_for_resource( + ResourceName="arn:aws:rds:blart-bling-1:0:db:db-id1234"), + call.create_db_cluster_snapshot(DBClusterIdentifier='cluster-5678', + DBClusterSnapshotIdentifier=ANY, + Tags=[{'Value': 'True', 'Key': 'Snapshot'}]), + call.describe_db_cluster_snapshots(DBClusterIdentifier='cluster-5678', SnapshotType='manual'), + ]) + + self.assertEqual({'total_errors': 0, + 'total_creates': 1, + 'total_deletes': 0, + 'total_resources': 1}, + result) + def test_snapshot_resource_for_standard(self): mgr = RDSBackupManager(rds_region_name="ap-southeast-1", @@ -95,7 +205,7 @@ def test_snapshot_resource_for_cluster(self): mgr.snapshot_resource(resource, description, tags) self.assertEqual(mgr.conn.mock_calls, [ - call.create_db_cluster_snapshot(DBClusterIdentifier='db-1234', + call.create_db_cluster_snapshot(DBClusterIdentifier='cluster-5678', DBClusterSnapshotIdentifier=ANY, Tags=[{'Value': 'value2', 'Key': 'value1'}]) ])