From aa6a8588453c5125d05aec8ad70506cdc47ffb89 Mon Sep 17 00:00:00 2001 From: Alina Buzachis Date: Fri, 18 Mar 2022 17:04:06 +0100 Subject: [PATCH] Improve error handling rds.py for RDS cluster and snapshot (#553) Improve error handling rds.py for RDS cluster and snapshot SUMMARY Improve error handling rds.py for rds_cluster Add handlers for rds_instance_snapshot and rds_cluster_snapshot ISSUE TYPE Feature Pull Request COMPONENT NAME module_utils/rds.py Reviewed-by: Mark Chappell Reviewed-by: Jill R Reviewed-by: Alina Buzachis Reviewed-by: Markus Bergholz Reviewed-by: Mark Woolley Reviewed-by: Joseph Torcasso --- plugins/module_utils/rds.py | 88 +++++++- tests/unit/module_utils/test_rds.py | 310 ++++++++++++++++++++++++++++ 2 files changed, 390 insertions(+), 8 deletions(-) create mode 100644 tests/unit/module_utils/test_rds.py diff --git a/plugins/module_utils/rds.py b/plugins/module_utils/rds.py index 221b92eff3e..e13ea778a47 100644 --- a/plugins/module_utils/rds.py +++ b/plugins/module_utils/rds.py @@ -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', @@ -35,12 +35,24 @@ '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: @@ -48,26 +60,42 @@ def get_rds_method_attribute(method_name, module): 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: @@ -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): @@ -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' @@ -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)) @@ -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: @@ -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: @@ -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): + 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") diff --git a/tests/unit/module_utils/test_rds.py b/tests/unit/module_utils/test_rds.py new file mode 100644 index 00000000000..4ff02454f0a --- /dev/null +++ b/tests/unit/module_utils/test_rds.py @@ -0,0 +1,310 @@ +# (c) 2021 Red Hat Inc. +# +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import absolute_import, division, print_function + +__metaclass__ = type + +from ansible_collections.amazon.aws.tests.unit.compat.mock import MagicMock +from ansible_collections.amazon.aws.plugins.module_utils import rds + + +from contextlib import nullcontext +import pytest + +try: + from botocore.exceptions import ClientError, WaiterError +except ImportError: + pass + + +def expected(x): + return x, nullcontext() + + +def error(*args, **kwargs): + return MagicMock(), pytest.raises(*args, **kwargs) + + +def build_exception( + operation_name, code=None, message=None, http_status_code=None, error=True +): + response = {} + if error or code or message: + response["Error"] = {} + if code: + response["Error"]["Code"] = code + if message: + response["Error"]["Message"] = message + if http_status_code: + response["ResponseMetadata"] = {"HTTPStatusCode": http_status_code} + return ClientError(response, operation_name) + + +@pytest.mark.parametrize("waiter_name", ["", "db_snapshot_available"]) +def test__wait_for_snapshot_status(waiter_name): + rds.wait_for_snapshot_status(MagicMock(), MagicMock(), "test", waiter_name) + + +@pytest.mark.parametrize( + "input, expected", + [ + ( + "db_snapshot_available", + "Failed to wait for DB snapshot test to be available", + ), + ( + "db_cluster_snapshot_available", + "Failed to wait for DB snapshot test to be available", + ), + ("db_snapshot_deleted", "Failed to wait for DB snapshot test to be deleted"), + ( + "db_cluster_snapshot_deleted", + "Failed to wait for DB snapshot test to be deleted", + ), + ], +) +def test__wait_for_snapshot_status_failed(input, expected): + spec = {"get_waiter.side_effect": [WaiterError(None, None, None)]} + client = MagicMock(**spec) + module = MagicMock() + + rds.wait_for_snapshot_status(client, module, "test", input) + module.fail_json_aws.assert_called_once + module.fail_json_aws.call_args[1]["msg"] == expected + + +@pytest.mark.parametrize( + "input, expected, error", + [ + ( + "delete_db_snapshot", + *expected( + rds.Boto3ClientMethod( + name="delete_db_snapshot", + waiter="db_snapshot_deleted", + operation_description="delete DB snapshot", + cluster=False, + instance=False, + snapshot=True, + ) + ), + ), + ( + "create_db_snapshot", + *expected( + rds.Boto3ClientMethod( + name="create_db_snapshot", + waiter="db_snapshot_available", + operation_description="create DB snapshot", + cluster=False, + instance=False, + snapshot=True, + ) + ), + ), + ( + "delete_db_cluster_snapshot", + *expected( + rds.Boto3ClientMethod( + name="delete_db_cluster_snapshot", + waiter="db_cluster_snapshot_deleted", + operation_description="delete DB cluster snapshot", + cluster=False, + instance=False, + snapshot=True, + ) + ), + ), + ( + "create_db_cluster_snapshot", + *expected( + rds.Boto3ClientMethod( + name="create_db_cluster_snapshot", + waiter="db_cluster_snapshot_available", + operation_description="create DB cluster snapshot", + cluster=False, + instance=False, + snapshot=True, + ) + ), + ), + ( + "fake_method", + *error( + NotImplementedError, + match="method fake_method hasn't been added to the list of accepted methods to use a waiter in module_utils/rds.py", + ), + ), + ], +) +def test__get_rds_method_attribute(input, expected, error): + with error: + assert rds.get_rds_method_attribute(input, MagicMock()) == expected + + +@pytest.mark.parametrize( + "method_name, params, expected", + [ + ("create_db_snapshot", {"db_snapshot_identifier": "test"}, "test"), + ( + "create_db_snapshot", + {"db_snapshot_identifier": "test", "apply_immediately": True}, + "test", + ), + ( + "create_db_instance", + { + "db_instance_identifier": "test", + "new_db_instance_identifier": "test_updated", + }, + "test", + ), + ( + "create_db_snapshot", + {"db_snapshot_identifier": "test", "apply_immediately": True}, + "test", + ), + ( + "create_db_instance", + { + "db_instance_identifier": "test", + "new_db_instance_identifier": "test_updated", + "apply_immediately": True, + }, + "test_updated", + ), + ( + "create_db_cluster", + { + "db_cluster_identifier": "test", + "new_db_cluster_identifier": "test_updated", + }, + "test", + ), + ( + "create_db_snapshot", + {"db_snapshot_identifier": "test", "apply_immediately": True}, + "test", + ), + ( + "create_db_cluster", + { + "db_cluster_identifier": "test", + "new_db_cluster_identifier": "test_updated", + "apply_immediately": True, + }, + "test_updated", + ), + ], +) +def test__get_final_identifier(method_name, params, expected): + module = MagicMock() + module.params = params + module.check_mode = False + + assert rds.get_final_identifier(method_name, module) == expected + + +@pytest.mark.parametrize( + "method_name, exception, expected", + [ + ( + "modify_db_instance", + build_exception( + "modify_db_instance", + code="InvalidParameterCombination", + message="No modifications were requested", + ), + False, + ), + ( + "promote_read_replica", + build_exception( + "promote_read_replica", + code="InvalidDBInstanceState", + message="DB Instance is not a read replica", + ), + False, + ), + ( + "promote_read_replica_db_cluster", + build_exception( + "promote_read_replica_db_cluster", + code="InvalidDBClusterStateFault", + message="DB Cluster that is not a read replica", + ), + False, + ), + ], +) +def test__handle_errors(method_name, exception, expected): + assert rds.handle_errors(MagicMock(), exception, method_name, {}) == expected + + +@pytest.mark.parametrize( + "method_name, exception, expected, error", + [ + ( + "modify_db_instance", + build_exception( + "modify_db_instance", + code="InvalidParameterCombination", + message="ModifyDbCluster API", + ), + *expected( + "It appears you are trying to modify attributes that are managed at the cluster level. Please see rds_cluster" + ), + ), + ( + "modify_db_instance", + build_exception("modify_db_instance", code="InvalidParameterCombination"), + *error( + NotImplementedError, + match="method modify_db_instance hasn't been added to the list of accepted methods to use a waiter in module_utils/rds.py", + ), + ), + ( + "promote_read_replica", + build_exception("promote_read_replica", code="InvalidDBInstanceState"), + *error( + NotImplementedError, + match="method promote_read_replica hasn't been added to the list of accepted methods to use a waiter in module_utils/rds.py", + ), + ), + ( + "promote_read_replica_db_cluster", + build_exception( + "promote_read_replica_db_cluster", code="InvalidDBClusterStateFault" + ), + *error( + NotImplementedError, + match="method promote_read_replica_db_cluster hasn't been added to the list of accepted methods to use a waiter in module_utils/rds.py", + ), + ), + ( + "create_db_instance", + build_exception("create_db_instance", code="InvalidParameterValue"), + *expected( + "DB engine fake_engine should be one of aurora, aurora-mysql, aurora-postgresql, mariadb, mysql, oracle-ee, oracle-se, oracle-se1, " + + "oracle-se2, postgres, sqlserver-ee, sqlserver-ex, sqlserver-se, sqlserver-web" + ), + ), + ( + "create_db_cluster", + build_exception("create_db_cluster", code="InvalidParameterValue"), + *expected( + "DB engine fake_engine should be one of aurora, aurora-mysql, aurora-postgresql" + ), + ), + ], +) +def test__handle_errors_failed(method_name, exception, expected, error): + module = MagicMock() + + with error: + rds.handle_errors(module, exception, method_name, {"Engine": "fake_engine"}) + module.fail_json_aws.assert_called_once + module.fail_json_aws.call_args[1]["msg"] == expected