From 7a2ca2f068da913751a1290970383c7ad86be740 Mon Sep 17 00:00:00 2001 From: FCO Date: Sun, 7 Mar 2021 19:50:00 +0100 Subject: [PATCH] Improve module mq_broker --- plugins/modules/mq_broker.py | 114 +++++++++-- .../targets/mq/tasks/test_mq_broker.yml | 190 ++++++++++++++++-- 2 files changed, 272 insertions(+), 32 deletions(-) diff --git a/plugins/modules/mq_broker.py b/plugins/modules/mq_broker.py index c12fa04f7cf..c32fb553fc0 100644 --- a/plugins/modules/mq_broker.py +++ b/plugins/modules/mq_broker.py @@ -314,6 +314,57 @@ def _fill_kwargs(module, apply_defaults=True, ignore_create_params=False): return kwargs +def __list_needs_change(current, desired): + if len(current) != len(desired): + return True + # equal length: + c_sorted = sorted(current) + d_sorted = sorted(desired) + for index, value in enumerate(current): + if value != desired[index]: + return True + # + return False + + +def __dict_needs_change(current, desired): + # values contained in 'current' but not specified in 'desired' are ignored + # value contained in 'desired' but not in 'current' (unsupported attributes) are ignored + for key in desired: + if key in current: + if desired[key] != current[key]: + return True + # + return False + + +def _needs_change(current, desired): + needs_change = False + for key in desired: + current_value = current[key] + desired_value = desired[key] + if type(current_value) in [int, str, bool]: + if current_value != desired_value: + needs_change = True + break + elif isinstance(current_value, list): + # assumption: all 'list' type settings we allow changes for have scalar values + if __list_needs_change(current_value, desired_value): + needs_change = True + break + elif isinstance(current_value, dict): + # assumption: all 'dict' type settings we allow changes for have scalar values + if __dict_needs_change(current_value, desired_value): + needs_change = True + break + else: + # unexpected type + needs_change = True + break + # + return needs_change + + def get_broker_id(conn, module): try: broker_name = module.params['broker_name'] @@ -388,20 +439,56 @@ def create_broker(conn, module): def update_broker(conn, module, broker_id): kwargs = _fill_kwargs(module, apply_defaults=False, ignore_create_params=True) # replace name with id + broker_name = kwargs['BrokerName'] del kwargs['BrokerName'] kwargs['BrokerId'] = broker_id - # TODO: get current state and check whether change is necessary at all - changed = True - if not module.check_mode: - result = conn.update_broker(**kwargs) - else: - result = { - 'BrokerId': broker_id - } + # get current state for comparison: + api_result = get_broker_info(conn, module, broker_id) + if api_result['BrokerState'] != 'RUNNING': + module.fail_json_aws(RuntimeError, + msg="Cannot trigger update while broker ({0}) is in state {1}".format( + broker_id, api_result['BrokerState'] + )) + result = { + 'BrokerId': broker_id, + 'BrokerName': broker_name + } + changed = False + if _needs_change(api_result, kwargs): + changed = True + if not module.check_mode: + api_result = conn.update_broker(**kwargs) + # # return {'broker': result, 'changed': changed} + +def ensure_absent(conn, module): + result = { + 'BrokerName': module.params['broker_name'], + 'BrokerId': None + } + if module.check_mode: + return {'broker': result, 'changed': True} + broker_id = get_broker_id(conn, module) + result['BrokerId'] = broker_id + if broker_id: + try: + # check for pending delete (small race condition possible here + api_result = get_broker_info(conn, module, broker_id) + if api_result['BrokerState'] == 'DELETION_IN_PROGRESS': + return {'broker': result, 'changed': False} + delete_broker(conn, module, broker_id) + except botocore.exceptions.ClientError as e: + module.fail_json_aws(e) + # + return {'broker': result, 'changed': True} + else: + # silently ignore delete of unknown broker (to make it idempotent) + return {'broker': result, 'changed': False} + + def ensure_present(conn, module): broker_id = get_broker_id(conn, module) if broker_id: @@ -447,19 +534,12 @@ def main(): # module.exit_json(**compound_result) elif module.params['state'] == 'absent': - broker_id = get_broker_id(connection, module) - if not broker_id: - module.fail_json_aws(RuntimeError, - msg="Cannot find broker with name {0}.".format(module.params['broker_name'])) - result = get_broker_info(connection, module, broker_id) try: - changed = True - if not module.check_mode: - delete_broker(connection, module, broker_id) + compound_result = ensure_absent(connection, module) except botocore.exceptions.ClientError as e: module.fail_json_aws(e) # - module.exit_json(broker=result, changed=changed) + module.exit_json(**compound_result) elif module.params['state'] == 'restarted': broker_id = get_broker_id(connection, module) if not broker_id: diff --git a/tests/integration/targets/mq/tasks/test_mq_broker.yml b/tests/integration/targets/mq/tasks/test_mq_broker.yml index 0903b1a5f08..21136116b98 100644 --- a/tests/integration/targets/mq/tasks/test_mq_broker.yml +++ b/tests/integration/targets/mq/tasks/test_mq_broker.yml @@ -3,7 +3,7 @@ hosts: dummy gather_facts: false vars: - broker_name: "{{ lookup('env', 'MQ_BROKER_NAME') }}" + broker_name: "{{ lookup('env', 'MQ_BROKER_NAME') | default('mq_broker_ci') }}" broker_sg_ids: "{{ lookup('env', 'MQ_SG_IDS') | default('') }}" broker_subnet_ids: "{{ lookup('env', 'MQ_SUBNET_IDS') | default('') }}" aws_access_key_id: "{{ lookup('env', 'AWS_ACCESS_KEY_ID') }}" @@ -16,7 +16,7 @@ - name: show env debug: msg: "Will run tests against broker '{{ broker_name }}' in AWS region '{{ aws_region }}'" - - name: test - create broker with minimal parameters + - name: create broker with minimal parameters # amazon.aws.mq_broker: mq_broker: broker_name: "{{ broker_name }}" @@ -25,26 +25,30 @@ subnet_ids: "{{ broker_subnet_ids.split(',') }}" aws_access_key: "{{ aws_access_key_id }}" aws_secret_key: "{{ aws_secret_access_key }}" + security_token: "{{ aws_session_token }}" register: result - set_fact: broker_id: "{{ result.broker['BrokerId'] }}" - - name: test1 - show broker details + - name: get broker details by id # amazon.aws.mq_broker_info: mq_broker_info: broker_id: "{{ broker_id }}" region: "{{ aws_region }}" aws_access_key: "{{ aws_access_key_id }}" aws_secret_key: "{{ aws_secret_access_key }}" - register: result - - name: verify test1 + security_token: "{{ aws_session_token }}" + register: result_c1 + - name: verify creation result #ansible.builtin.assert: assert: - fail_msg: test1 failed + fail_msg: broker creation failed that: - - not (result.changed | bool) - - result.broker['BrokerId'] == broker_id - - result.broker['BrokerName'] == broker_name - - result.broker['BrokerState'] == 'CREATION_IN_PROGRESS' + # change state is from previous operation: + - ( result.changed | bool ) + - result_c1.broker['BrokerId'] == broker_id + - result_c1.broker['BrokerName'] == broker_name + - result_c1.broker['BrokerState'] == 'CREATION_IN_PROGRESS' + - ( result_c1.broker['StorageType'] | upper ) == 'EFS' - debug: msg: "Wait until broker {{ broker_name }} ({{ broker_id }}) enters running state. This may take several minutes" - name: wait for startup @@ -54,19 +58,62 @@ region: "{{ aws_region }}" aws_access_key: "{{ aws_access_key_id }}" aws_secret_key: "{{ aws_secret_access_key }}" + security_token: "{{ aws_session_token }}" register: result until: result.broker['BrokerState'] == 'RUNNING' retries: 15 delay: 60 + - name: repeat creation + # amazon.aws.mq_broker: + mq_broker: + broker_name: "{{ broker_name }}" + region: "{{ aws_region }}" + security_groups: "{{ broker_sg_ids.split(',') }}" + subnet_ids: "{{ broker_subnet_ids.split(',') }}" + aws_access_key: "{{ aws_access_key_id }}" + aws_secret_key: "{{ aws_secret_access_key }}" + security_token: "{{ aws_session_token }}" + register: result + - set_fact: + broker_id: "{{ result.broker['BrokerId'] }}" + - name: get broker details - this time by name + # amazon.aws.mq_broker_info: + mq_broker_info: + broker_name: "{{ broker_name }}" + region: "{{ aws_region }}" + aws_access_key: "{{ aws_access_key_id }}" + aws_secret_key: "{{ aws_secret_access_key }}" + security_token: "{{ aws_session_token }}" + register: result_c2 + - name: verify broker re-creation + #ansible.builtin.assert: + assert: + fail_msg: broker re-creation failed + that: + # change state is from previous operation: + - not ( result.changed | bool) + - result_c2.broker['BrokerId'] == broker_id + - result_c2.broker['BrokerName'] == broker_name + - ( result_c2.broker['StorageType'] | upper ) == 'EFS' - name: update broker # amazon.aws.mq_broker: mq_broker: broker_name: "{{ broker_name }}" region: "{{ aws_region }}" auto_minor_version_upgrade: false + storage_type: EBS aws_access_key: "{{ aws_access_key_id }}" aws_secret_key: "{{ aws_secret_access_key }}" - - name: test3 - reboot broker + security_token: "{{ aws_session_token }}" + register: result + - name: verify broker update + #ansible.builtin.assert: + assert: + fail_msg: broker update failed + that: + - ( result.changed | bool) + - result.broker['BrokerId'] == broker_id + - name: reboot broker to make pending changes active # amazon.aws.mq_broker: mq_broker: broker_name: "{{ broker_name }}" @@ -76,26 +123,85 @@ aws_secret_key: "{{ aws_secret_access_key }}" security_token: "{{ aws_session_token }}" register: result - - name: verify test3 + - name: get broker details by id + # amazon.aws.mq_broker_info: + mq_broker_info: + broker_id: "{{ broker_id }}" + region: "{{ aws_region }}" + aws_access_key: "{{ aws_access_key_id }}" + aws_secret_key: "{{ aws_secret_access_key }}" + security_token: "{{ aws_session_token }}" + register: result_r1 + - name: check for pending reboot #ansible.builtin.assert: assert: - fail_msg: test3 failed + fail_msg: trigger reboot failed that: - result.changed | bool - - result.broker['BrokerState'] == 'REBOOT_IN_PROGRESS' + - result_r1.broker['BrokerState'] == 'REBOOT_IN_PROGRESS' - debug: msg: "Wait until reboot of broker {{ broker_name }} ({{ broker_id }}) is finished. This may take several minutes" - - name: wait for startup + - name: wait for reboot # amazon.aws.mq_broker_info: mq_broker_info: broker_id: "{{ broker_id }}" region: "{{ aws_region }}" aws_access_key: "{{ aws_access_key_id }}" aws_secret_key: "{{ aws_secret_access_key }}" + security_token: "{{ aws_session_token }}" register: result until: result.broker['BrokerState'] == 'RUNNING' retries: 15 delay: 60 + - name: get details after update + # amazon.aws.mq_broker_info: + mq_broker_info: + broker_name: "{{ broker_name }}" + region: "{{ aws_region }}" + aws_access_key: "{{ aws_access_key_id }}" + aws_secret_key: "{{ aws_secret_access_key }}" + security_token: "{{ aws_session_token }}" + register: result_u1 + - name: verify broker update + #ansible.builtin.assert: + assert: + fail_msg: broker update failed + that: + - result_u1.broker['BrokerId'] == broker_id + - result_u1.broker['BrokerName'] == broker_name + - not ( result_u1.broker['AutoMinorVersionUpgrade'] | bool ) + # the next one checks that changes to create-only parameters are silently ignore + - result_u1.broker['StorageType'] == result_c1.broker['StorageType'] + - name: repeat update broker + # amazon.aws.mq_broker: + mq_broker: + broker_name: "{{ broker_name }}" + region: "{{ aws_region }}" + auto_minor_version_upgrade: false + storage_type: EBS + aws_access_key: "{{ aws_access_key_id }}" + aws_secret_key: "{{ aws_secret_access_key }}" + security_token: "{{ aws_session_token }}" + register: result + - name: get details after re-update + # amazon.aws.mq_broker_info: + mq_broker_info: + broker_name: "{{ broker_name }}" + region: "{{ aws_region }}" + aws_access_key: "{{ aws_access_key_id }}" + aws_secret_key: "{{ aws_secret_access_key }}" + security_token: "{{ aws_session_token }}" + register: result_u2 + - name: verify broker re-update + #ansible.builtin.assert: + assert: + fail_msg: broker update failed + that: + - not ( result.changed | bool) + - result_u2.broker['BrokerId'] == result_u1.broker['BrokerId'] + - result_u2.broker['StorageType'] == result_u1.broker['StorageType'] + - result_u2.broker['EngineVersion'] == result_u1.broker['EngineVersion'] + # here we can put in tests for mq_broker_config - name: delete broker # amazon.aws.mq_broker: mq_broker: @@ -106,3 +212,57 @@ aws_secret_key: "{{ aws_secret_access_key }}" security_token: "{{ aws_session_token }}" register: result + - name: verify broker delete + #ansible.builtin.assert: + assert: + fail_msg: broker delete failed + that: + - ( result.changed | bool) + - name: get details after delete + # amazon.aws.mq_broker_info: + mq_broker_info: + broker_name: "{{ broker_name }}" + region: "{{ aws_region }}" + aws_access_key: "{{ aws_access_key_id }}" + aws_secret_key: "{{ aws_secret_access_key }}" + security_token: "{{ aws_session_token }}" + register: result_d1 + - name: verify broker deletion on progress + #ansible.builtin.assert: + assert: + fail_msg: broker delete too fast? + that: + - result_d1.broker['BrokerState'] == 'DELETION_IN_PROGRESS' + - name: repeat broker deletion + # amazon.aws.mq_broker: + mq_broker: + broker_name: "{{ broker_name }}" + state: "absent" + region: "{{ aws_region }}" + aws_access_key: "{{ aws_access_key_id }}" + aws_secret_key: "{{ aws_secret_access_key }}" + security_token: "{{ aws_session_token }}" + register: result + - name: verify broker repeated delete + #ansible.builtin.assert: + assert: + fail_msg: didn't detect DELETION_IN_PROGRESS in progress + that: + - not ( result.changed | bool) + - name: deletion unknown broker - simulates re-deletion of completely deleted broker + # amazon.aws.mq_broker: + mq_broker: + broker_name: "{{ broker_name }}__unknown_broker__" + state: "absent" + region: "{{ aws_region }}" + aws_access_key: "{{ aws_access_key_id }}" + aws_secret_key: "{{ aws_secret_access_key }}" + security_token: "{{ aws_session_token }}" + register: result + - name: verify delete unknown broker + #ansible.builtin.assert: + assert: + fail_msg: deletion of unknown broker return unexpected result + that: + - not ( result.changed | bool) +