Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve error handling rds.py for RDS cluster and snapshot #553

Merged
merged 5 commits into from
Mar 18, 2022
Merged

Improve error handling rds.py for RDS cluster and snapshot #553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
20c1a98
Improve error handling rds.py for rds_cluster
alinabuzachis Oct 27, 2021
ab4d26b
Update with rds_cluster_snapshot and rds_instance_snapshot helpers
alinabuzachis Nov 1, 2021
6d18c1d
Add some unit tests
alinabuzachis Jan 3, 2022
bf29f9f
Fix linting
alinabuzachis Jan 24, 2022
fc3cd21
Improve error handling rds.py for rds_cluster
alinabuzachis Oct 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 80 additions & 8 deletions plugins/module_utils/rds.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from .ec2 import compare_aws_tags
from .waiters import get_waiter

Boto3ClientMethod = namedtuple('Boto3ClientMethod', ['name', 'waiter', 'operation_description', 'cluster', 'instance'])
Boto3ClientMethod = namedtuple('Boto3ClientMethod', ['name', 'waiter', 'operation_description', 'cluster', 'instance', 'snapshot'])
# Whitelist boto3 client methods for cluster and instance resources
cluster_method_names = [
'create_db_cluster', 'restore_db_cluster_from_db_snapshot', 'restore_db_cluster_from_s3',
Expand All @@ -35,39 +35,67 @@
'promote_read_replica', 'stop_db_instance', 'start_db_instance', 'reboot_db_instance'
]

snapshot_cluster_method_names = [
'create_db_cluster_snapshot', 'delete_db_cluster_snapshot', 'add_tags_to_resource', 'remove_tags_from_resource',
'list_tags_for_resource'
]

snapshot_instance_method_names = [
'create_db_snapshot', 'delete_db_snapshot', 'add_tags_to_resource', 'remove_tags_from_resource',
'list_tags_for_resource'
]


def get_rds_method_attribute(method_name, module):
waiter = ''
readable_op = method_name.replace('_', ' ').replace('db', 'DB')
if method_name in cluster_method_names and 'new_db_cluster_identifier' in module.params:
cluster = True
instance = False
snapshot = False
if method_name == 'delete_db_cluster':
waiter = 'cluster_deleted'
else:
waiter = 'cluster_available'
elif method_name in instance_method_names and 'new_db_instance_identifier' in module.params:
cluster = False
instance = True
snapshot = False
if method_name == 'delete_db_instance':
waiter = 'db_instance_deleted'
elif method_name == 'stop_db_instance':
waiter = 'db_instance_stopped'
else:
waiter = 'db_instance_available'
elif method_name in snapshot_cluster_method_names or method_name in snapshot_instance_method_names:
cluster = False
instance = False
snapshot = True
if method_name == 'delete_db_snapshot':
waiter = 'db_snapshot_deleted'
elif method_name == 'delete_db_cluster_snapshot':
waiter = 'db_cluster_snapshot_deleted'
elif method_name == 'create_db_snapshot':
waiter = 'db_snapshot_available'
elif method_name == 'create_db_cluster_snapshot':
waiter = 'db_cluster_snapshot_available'
else:
raise NotImplementedError("method {0} hasn't been added to the list of accepted methods to use a waiter in module_utils/rds.py".format(method_name))

return Boto3ClientMethod(name=method_name, waiter=waiter, operation_description=readable_op, cluster=cluster, instance=instance)
return Boto3ClientMethod(name=method_name, waiter=waiter, operation_description=readable_op, cluster=cluster, instance=instance, snapshot=snapshot)


def get_final_identifier(method_name, module):
apply_immediately = module.params['apply_immediately']
updated_identifier = None
apply_immediately = module.params.get('apply_immediately')
if get_rds_method_attribute(method_name, module).cluster:
identifier = module.params['db_cluster_identifier']
updated_identifier = module.params['new_db_cluster_identifier']
elif get_rds_method_attribute(method_name, module).instance:
identifier = module.params['db_instance_identifier']
updated_identifier = module.params['new_db_instance_identifier']
elif get_rds_method_attribute(method_name, module).snapshot:
identifier = module.params['db_snapshot_identifier']
else:
raise NotImplementedError("method {0} hasn't been added to the list of accepted methods in module_utils/rds.py".format(method_name))
if not module.check_mode and updated_identifier and apply_immediately:
Expand All @@ -82,7 +110,10 @@ def handle_errors(module, exception, method_name, parameters):

changed = True
error_code = exception.response['Error']['Code']
if method_name == 'modify_db_instance' and error_code == 'InvalidParameterCombination':
if (
method_name in ('modify_db_instance', 'modify_db_cluster') and
error_code == 'InvalidParameterCombination'
):
if 'No modifications were requested' in to_text(exception):
changed = False
elif 'ModifyDbCluster API' in to_text(exception):
Expand All @@ -94,7 +125,12 @@ def handle_errors(module, exception, method_name, parameters):
changed = False
else:
module.fail_json_aws(exception, msg='Unable to {0}'.format(get_rds_method_attribute(method_name, module).operation_description))
elif method_name == 'create_db_instance' and exception.response['Error']['Code'] == 'InvalidParameterValue':
elif method_name == 'promote_read_replica_db_cluster' and error_code == 'InvalidDBClusterStateFault':
if 'DB Cluster that is not a read replica' in to_text(exception):
changed = False
else:
module.fail_json_aws(exception, msg='Unable to {0}'.format(get_rds_method_attribute(method_name, module).operation_description))
elif method_name == 'create_db_instance' and error_code == 'InvalidParameterValue':
accepted_engines = [
'aurora', 'aurora-mysql', 'aurora-postgresql', 'mariadb', 'mysql', 'oracle-ee', 'oracle-se',
'oracle-se1', 'oracle-se2', 'postgres', 'sqlserver-ee', 'sqlserver-ex', 'sqlserver-se', 'sqlserver-web'
Expand All @@ -103,6 +139,14 @@ def handle_errors(module, exception, method_name, parameters):
module.fail_json_aws(exception, msg='DB engine {0} should be one of {1}'.format(parameters.get('Engine'), accepted_engines))
else:
module.fail_json_aws(exception, msg='Unable to {0}'.format(get_rds_method_attribute(method_name, module).operation_description))
elif method_name == 'create_db_cluster' and error_code == 'InvalidParameterValue':
accepted_engines = [
'aurora', 'aurora-mysql', 'aurora-postgresql'
]
if parameters.get('Engine') not in accepted_engines:
module.fail_json_aws(exception, msg='DB engine {0} should be one of {1}'.format(parameters.get('Engine'), accepted_engines))
else:
module.fail_json_aws(exception, msg='Unable to {0}'.format(get_rds_method_attribute(method_name, module).operation_description))
else:
module.fail_json_aws(exception, msg='Unable to {0}'.format(get_rds_method_attribute(method_name, module).operation_description))

Expand All @@ -113,7 +157,7 @@ def call_method(client, module, method_name, parameters):
result = {}
changed = True
if not module.check_mode:
wait = module.params['wait']
wait = module.params.get('wait')
# TODO: stabilize by adding get_rds_method_attribute(method_name).extra_retry_codes
method = getattr(client, method_name)
try:
Expand All @@ -122,6 +166,11 @@ def call_method(client, module, method_name, parameters):
if wait:
wait_for_status(client, module, module.params['db_instance_identifier'], method_name)
result = AWSRetry.jittered_backoff(catch_extra_error_codes=['InvalidDBInstanceState'])(method)(**parameters)
elif method_name == 'modify_db_cluster':
# check if cluster is in an available state first, if possible
if wait:
wait_for_status(client, module, module.params['db_cluster_identifier'], method_name)
result = AWSRetry.jittered_backoff(catch_extra_error_codes=['InvalidDBClusterStateFault'])(method)(**parameters)
else:
result = AWSRetry.jittered_backoff()(method)(**parameters)
except (BotoCoreError, ClientError) as e:
Expand Down Expand Up @@ -181,20 +230,43 @@ def wait_for_cluster_status(client, module, db_cluster_id, waiter_name):
module.fail_json_aws(e, msg="Failed with an unexpected error while waiting for the DB cluster {0}".format(db_cluster_id))


def wait_for_snapshot_status(client, module, db_snapshot_id, waiter_name):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please create a new unit test file for this module_util and cover this new function? Eventually we should try to cover the rest of the util but it's a lot easier to ask for folks to contribute tests for changes if the test file already exists.

We should also run the integration tests for the RDS modules with a depends-on for this change, to ensure there's no regressions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jillr Done that!

params = {}
# Covers the corner case when tags have to be updated and 'wait: yes'. There is no waiter defined.
if not waiter_name:
return
if "cluster" in waiter_name:
params = {"DBClusterSnapshotIdentifier": db_snapshot_id}
else:
params = {"DBSnapshotIdentifier": db_snapshot_id}
try:
client.get_waiter(waiter_name).wait(**params)
except WaiterError as e:
if waiter_name in ('db_snapshot_deleted', 'db_cluster_snapshot_deleted'):
msg = "Failed to wait for DB snapshot {0} to be deleted".format(db_snapshot_id)
else:
msg = "Failed to wait for DB snapshot {0} to be available".format(db_snapshot_id)
module.fail_json_aws(e, msg=msg)
except (BotoCoreError, ClientError) as e:
module.fail_json_aws(e, msg="Failed with an unexpected error while waiting for the DB snapshot {0}".format(db_snapshot_id))


def wait_for_status(client, module, identifier, method_name):
waiter_name = get_rds_method_attribute(method_name, module).waiter
if get_rds_method_attribute(method_name, module).cluster:
wait_for_cluster_status(client, module, identifier, waiter_name)
elif get_rds_method_attribute(method_name, module).instance:
wait_for_instance_status(client, module, identifier, waiter_name)
elif get_rds_method_attribute(method_name, module).snapshot:
wait_for_snapshot_status(client, module, identifier, waiter_name)
else:
raise NotImplementedError("method {0} hasn't been added to the whitelist of handled methods".format(method_name))


def get_tags(client, module, cluster_arn):
def get_tags(client, module, resource_arn):
try:
return boto3_tag_list_to_ansible_dict(
client.list_tags_for_resource(ResourceName=cluster_arn)['TagList']
client.list_tags_for_resource(ResourceName=resource_arn)['TagList']
)
except (BotoCoreError, ClientError) as e:
module.fail_json_aws(e, msg="Unable to describe tags")
Expand Down
Loading