From 38d414ec7dba34b4ce60d0dae56e42c7256f059e Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Fri, 24 Sep 2021 23:45:23 +0100 Subject: [PATCH 01/17] Add support for billing_mode --- plugins/modules/dynamodb_table.py | 74 ++++++++++++++----- .../targets/dynamodb_table/tasks/main.yml | 24 ++++++ 2 files changed, 78 insertions(+), 20 deletions(-) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index 86c08f90934..8010a0e024b 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -49,6 +49,12 @@ - Defaults to C('STRING') when creating a new range key. choices: ['STRING', 'NUMBER', 'BINARY'] type: str + billing_mode: + description: + - Controls whether provisoned pr on-demand tables are created. + choices: ['PROVISIONED', 'PAY_PER_REQUEST'] + default: 'PROVISIONED' + type: str read_capacity: description: - Read throughput capacity (units) to provision. @@ -165,6 +171,14 @@ read_capacity: 10 write_capacity: 10 +- name: Create pay-per-request table + community.aws.dynamodb_table: + name: my-table + region: us-east-1 + hash_key_name: id + hash_key_type: STRING + billing_mode: PAY_PER_REQUEST + - name: set index on existing dynamo table community.aws.dynamodb_table: name: my-table @@ -367,7 +381,10 @@ def compatability_results(current_table): if not current_table: return dict() - throughput = current_table.get('provisioned_throughput', {}) + billing_mode = current_table.get('billing_mode') + + if billing_mode == "PROVISIONED": + throughput = current_table.get('provisioned_throughput', {}) primary_indexes = _decode_primary_index(current_table) @@ -394,12 +411,13 @@ def compatability_results(current_table): range_key_name=range_key_name, range_key_type=range_key_type, indexes=indexes, + billing_mode=billing_mode, read_capacity=throughput.get('read_capacity_units', None), + write_capacity=throughput.get('write_capacity_units', None), region=module.region, table_name=current_table.get('table_name', None), table_status=current_table.get('table_status', None), tags=current_table.get('tags', {}), - write_capacity=throughput.get('write_capacity_units', None), ) return compat_results @@ -571,6 +589,13 @@ def _throughput_changes(current_table, params=None): def _generate_global_indexes(): index_exists = dict() indexes = list() + billing_mode = module.params.get('billing_mode') + + if billing_mode == "PROVISIONED": + is_provisioned = True + else: + is_provisioned = False + for index in module.params.get('indexes'): if index.get('type') not in ['global_all', 'global_include', 'global_keys_only']: continue @@ -579,7 +604,7 @@ def _generate_global_indexes(): module.fail_json(msg='Duplicate key {0} in list of global indexes'.format(name)) # Convert the type name to upper case and remove the global_ index['type'] = index['type'].upper()[7:] - index = _generate_index(index) + index = _generate_index(index, include_throughput=is_provisioned) index_exists[name] = True indexes.append(index) @@ -589,6 +614,7 @@ def _generate_global_indexes(): def _generate_local_indexes(): index_exists = dict() indexes = list() + for index in module.params.get('indexes'): index = dict() if index.get('type') not in ['all', 'include', 'keys_only']: @@ -708,20 +734,22 @@ def _local_index_changes(current_table): def _update_table(current_table): changes = dict() additional_global_index_changes = list() - - throughput_changes = _throughput_changes(current_table) - if throughput_changes: - changes['ProvisionedThroughput'] = throughput_changes - - global_index_changes = _global_index_changes(current_table) - if global_index_changes: - changes['GlobalSecondaryIndexUpdates'] = global_index_changes - # Only one index can be changed at a time, pass the first during the - # main update and deal with the others on a slow retry to wait for - # completion - if len(global_index_changes) > 1: - changes['GlobalSecondaryIndexUpdates'] = [global_index_changes[0]] - additional_global_index_changes = global_index_changes[1:] + billing_mode = module.params.get('billing_mode') + + if billing_mode == "PROVISIONED": + throughput_changes = _throughput_changes(current_table) + if throughput_changes: + changes['ProvisionedThroughput'] = throughput_changes + + global_index_changes = _global_index_changes(current_table) + if global_index_changes: + changes['GlobalSecondaryIndexUpdates'] = global_index_changes + # Only one index can be changed at a time, pass the first during the + # main update and deal with the others on a slow retry to wait for + # completion + if len(global_index_changes) > 1: + changes['GlobalSecondaryIndexUpdates'] = [global_index_changes[0]] + additional_global_index_changes = global_index_changes[1:] local_index_changes = _local_index_changes(current_table) if local_index_changes: @@ -818,6 +846,7 @@ def update_table(current_table): def create_table(): table_name = module.params.get('name') hash_key_name = module.params.get('hash_key_name') + billing_mode = module.params.get('billing_mode') tags = ansible_dict_to_boto3_tag_list(module.params.get('tags') or {}) @@ -827,7 +856,9 @@ def create_table(): if module.check_mode: return True - throughput = _generate_throughput() + if billing_mode == "PROVISIONED": + throughput = _generate_throughput() + attributes = _generate_attributes() key_schema = _generate_schema() local_indexes = _generate_local_indexes() @@ -837,13 +868,15 @@ def create_table(): TableName=table_name, AttributeDefinitions=attributes, KeySchema=key_schema, - ProvisionedThroughput=throughput, Tags=tags, + BillingMode=billing_mode # TODO (future) - # BillingMode, # StreamSpecification, # SSESpecification, ) + + if billing_mode == "PROVISIONED": + params['ProvisionedThroughput'] = throughput if local_indexes: params['LocalSecondaryIndexes'] = local_indexes if global_indexes: @@ -919,6 +952,7 @@ def main(): hash_key_type=dict(type='str', choices=KEY_TYPE_CHOICES), range_key_name=dict(type='str'), range_key_type=dict(type='str', choices=KEY_TYPE_CHOICES), + billing_mode=dict(default='PROVISIONED', type='str', choices=['PROVISIONED', 'PAY_PER_REQUEST']), read_capacity=dict(type='int'), write_capacity=dict(type='int'), indexes=dict(default=[], type='list', elements='dict', options=index_options), diff --git a/tests/integration/targets/dynamodb_table/tasks/main.yml b/tests/integration/targets/dynamodb_table/tasks/main.yml index 0cda98bcdbd..d0a9fce3c95 100644 --- a/tests/integration/targets/dynamodb_table/tasks/main.yml +++ b/tests/integration/targets/dynamodb_table/tasks/main.yml @@ -660,6 +660,30 @@ # ============================================== + - name: Create table - pay-per-request + dynamodb_table: + state: present + name: '{{ table_name }}' + hash_key_name: '{{ table_index }}' + hash_key_type: '{{ table_index_type }}' + billing_mode: PAY_PER_REQUEST + register: create_table + + - name: Check results - Create table + assert: + that: + - create_table is successful + - create_table is not changed + - create_table.billing_mode == "PAY_PER_REQUEST" + + - name: Delete table + dynamodb_table: + state: absent + name: '{{ table_name }}' + register: delete_table + + # ============================================== + - name: Create complex table - check_mode dynamodb_table: state: present From 07e42f261d118e4871b63c61636a2230d3a533bf Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Fri, 24 Sep 2021 23:56:52 +0100 Subject: [PATCH 02/17] split pay_per_request --- .../targets/dynamodb_table/defaults/main.yml | 16 ++++ .../targets/dynamodb_table/tasks/main.yml | 25 +---- .../tasks/test_pay_per_request.yml | 96 +++++++++++++++++++ 3 files changed, 113 insertions(+), 24 deletions(-) create mode 100644 tests/integration/targets/dynamodb_table/tasks/test_pay_per_request.yml diff --git a/tests/integration/targets/dynamodb_table/defaults/main.yml b/tests/integration/targets/dynamodb_table/defaults/main.yml index 55c8441bfa6..5618b0fdcc3 100644 --- a/tests/integration/targets/dynamodb_table/defaults/main.yml +++ b/tests/integration/targets/dynamodb_table/defaults/main.yml @@ -27,6 +27,22 @@ indexes: read_capacity: 5 write_capacity: 5 +indexes_pay_per_request: + - name: NamedIndex + type: global_include + hash_key_name: idx + range_key_name: create_time + includes: + - other_field + - other_field2 + - name: AnotherIndex + type: global_all + hash_key_name: foo + range_key_name: bar + includes: + - another_field + - another_field2 + index_updated: - name: NamedIndex type: global_include diff --git a/tests/integration/targets/dynamodb_table/tasks/main.yml b/tests/integration/targets/dynamodb_table/tasks/main.yml index d0a9fce3c95..c18c2a161b1 100644 --- a/tests/integration/targets/dynamodb_table/tasks/main.yml +++ b/tests/integration/targets/dynamodb_table/tasks/main.yml @@ -12,6 +12,7 @@ security_token: '{{ security_token | default(omit) }}' region: '{{ aws_region }}' block: + - include: "test_pay_per_request.yml" # ============================================== @@ -660,30 +661,6 @@ # ============================================== - - name: Create table - pay-per-request - dynamodb_table: - state: present - name: '{{ table_name }}' - hash_key_name: '{{ table_index }}' - hash_key_type: '{{ table_index_type }}' - billing_mode: PAY_PER_REQUEST - register: create_table - - - name: Check results - Create table - assert: - that: - - create_table is successful - - create_table is not changed - - create_table.billing_mode == "PAY_PER_REQUEST" - - - name: Delete table - dynamodb_table: - state: absent - name: '{{ table_name }}' - register: delete_table - - # ============================================== - - name: Create complex table - check_mode dynamodb_table: state: present diff --git a/tests/integration/targets/dynamodb_table/tasks/test_pay_per_request.yml b/tests/integration/targets/dynamodb_table/tasks/test_pay_per_request.yml new file mode 100644 index 00000000000..ca7bf567ccc --- /dev/null +++ b/tests/integration/targets/dynamodb_table/tasks/test_pay_per_request.yml @@ -0,0 +1,96 @@ +--- +- name: Create table - pay-per-request - check_mode + dynamodb_table: + state: present + name: "{{ table_name }}" + hash_key_name: "{{ table_index }}" + hash_key_type: "{{ table_index_type }}" + billing_mode: PAY_PER_REQUEST + register: create_table + check_mode: True + +- name: Check results - Create table - check_mode + assert: + that: + - create_table is successful + - create_table is changed + +- name: Create table - pay-per-request + dynamodb_table: + state: present + name: "{{ table_name }}" + hash_key_name: "{{ table_index }}" + hash_key_type: "{{ table_index_type }}" + billing_mode: PAY_PER_REQUEST + register: create_table + +- name: Check results - Create table + assert: + that: + - create_table is successful + - create_table is not changed + - create_table.billing_mode == "PAY_PER_REQUEST" + +# ============================================== + +- name: Create complex table - check_mode + dynamodb_table: + state: present + name: "{{ table_name }}" + hash_key_name: "{{ table_index }}" + hash_key_type: "{{ table_index_type }}" + range_key_name: "{{ range_index }}" + range_key_type: "{{ range_index_type }}" + billing_mode: PAY_PER_REQUEST + tags: "{{ tags_default }}" + indexes: "{{ indexes_pay_per_request }}" + register: create_complex_table + check_mode: True + +- name: Check results - Create complex table - check_mode + assert: + that: + - create_complex_table is successful + - create_complex_table is changed + +- name: Create complex table + dynamodb_table: + state: present + name: "{{ table_name }}" + hash_key_name: "{{ table_index }}" + hash_key_type: "{{ table_index_type }}" + range_key_name: "{{ range_index }}" + range_key_type: "{{ range_index_type }}" + billing_mode: PAY_PER_REQUEST + tags: "{{ tags_default }}" + indexes: "{{ indexes_pay_per_request }}" + register: create_complex_table + +- name: Check results - Create complex table + assert: + that: + - create_complex_table is successful + - create_complex_table is changed + - '"hash_key_name" in create_complex_table' + - '"hash_key_type" in create_complex_table' + - '"indexes" in create_complex_table' + - '"range_key_name" in create_complex_table' + - '"range_key_type" in create_complex_table' + - '"billing_mode" in create_complex_table' + - '"region" in create_complex_table' + - '"table_name" in create_complex_table' + - '"table_status" in create_complex_table' + - '"tags" in create_complex_table' + - create_complex_table.hash_key_name == table_index + - create_complex_table.hash_key_type == table_index_type + - create_complex_table.indexes | length == 2 + - create_complex_table.range_key_name == range_index + - create_complex_table.range_key_type == range_index_type + - create_complex_table.table_name == table_name + - create_complex_table.tags == tags_default + +- name: Delete table + dynamodb_table: + state: absent + name: "{{ table_name }}" + register: delete_table From fdad53d2646c7725cc409c6b1e0a3d4f7446edd7 Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Sat, 25 Sep 2021 00:16:05 +0100 Subject: [PATCH 03/17] fix assignment issue --- plugins/modules/dynamodb_table.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index 8010a0e024b..2cd380f2107 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -412,14 +412,16 @@ def compatability_results(current_table): range_key_type=range_key_type, indexes=indexes, billing_mode=billing_mode, - read_capacity=throughput.get('read_capacity_units', None), - write_capacity=throughput.get('write_capacity_units', None), region=module.region, table_name=current_table.get('table_name', None), table_status=current_table.get('table_status', None), tags=current_table.get('tags', {}), ) + if billing_mode == "PROVISIONED": + compat_results['read_capacity'] = throughput.get('read_capacity_units', None) + compat_results['write_capacity'] = throughput.get('write_capacity_units', None) + return compat_results From a31259a056598a015be057f49b4a1d7f117068ac Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Mon, 11 Oct 2021 17:32:23 +0100 Subject: [PATCH 04/17] fixes & support move between provisioned & pay-per-request --- plugins/modules/dynamodb_table.py | 43 +++++++++++++++++-------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index 2cd380f2107..c0823bfbe8b 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -2,10 +2,6 @@ # Copyright: Ansible Project # 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 - - DOCUMENTATION = r''' --- module: dynamodb_table @@ -455,6 +451,12 @@ def get_dynamodb_table(): table['size'] = table['table_size_bytes'] table['tags'] = tags + # billing_mode_summary is only set if the table is already PAY_PER_REQUEST + if 'billing_mode_summary' in table: + table['billing_mode'] = table['billing_mode_summary']['billing_mode'] + else: + table['billing_mode'] = "PROVISIONED" + # convert indexes into something we can easily search against attributes = table['attribute_definitions'] global_index_map = dict() @@ -736,22 +738,23 @@ def _local_index_changes(current_table): def _update_table(current_table): changes = dict() additional_global_index_changes = list() - billing_mode = module.params.get('billing_mode') - if billing_mode == "PROVISIONED": - throughput_changes = _throughput_changes(current_table) - if throughput_changes: - changes['ProvisionedThroughput'] = throughput_changes - - global_index_changes = _global_index_changes(current_table) - if global_index_changes: - changes['GlobalSecondaryIndexUpdates'] = global_index_changes - # Only one index can be changed at a time, pass the first during the - # main update and deal with the others on a slow retry to wait for - # completion - if len(global_index_changes) > 1: - changes['GlobalSecondaryIndexUpdates'] = [global_index_changes[0]] - additional_global_index_changes = global_index_changes[1:] + throughput_changes = _throughput_changes(current_table) + if throughput_changes: + changes['ProvisionedThroughput'] = throughput_changes + + if current_table.get('billing_mode') != module.params.get('billing_mode'): + changes['BillingMode'] = billing_mode + + global_index_changes = _global_index_changes(current_table) + if global_index_changes: + changes['GlobalSecondaryIndexUpdates'] = global_index_changes + # Only one index can be changed at a time, pass the first during the + # main update and deal with the others on a slow retry to wait for + # completion + if len(global_index_changes) > 1: + changes['GlobalSecondaryIndexUpdates'] = [global_index_changes[0]] + additional_global_index_changes = global_index_changes[1:] local_index_changes = _local_index_changes(current_table) if local_index_changes: @@ -961,7 +964,7 @@ def main(): tags=dict(type='dict'), purge_tags=dict(type='bool', default=True), wait=dict(type='bool', default=True), - wait_timeout=dict(default=120, type='int', aliases=['wait_for_active_timeout']), + wait_timeout=dict(default=300, type='int', aliases=['wait_for_active_timeout']), ) module = AnsibleAWSModule( From d993f22f2664e2c8950e26a07bd71d9dfd4e4705 Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Mon, 11 Oct 2021 17:42:28 +0100 Subject: [PATCH 05/17] update tests --- .../targets/dynamodb_table/defaults/main.yml | 1 + .../targets/dynamodb_table/tasks/main.yml | 2 + .../tasks/test_pay_per_request.yml | 38 +++++++++++++++---- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/tests/integration/targets/dynamodb_table/defaults/main.yml b/tests/integration/targets/dynamodb_table/defaults/main.yml index 5618b0fdcc3..d9e8b6995a0 100644 --- a/tests/integration/targets/dynamodb_table/defaults/main.yml +++ b/tests/integration/targets/dynamodb_table/defaults/main.yml @@ -1,5 +1,6 @@ --- table_name: '{{ resource_prefix }}' +table_name_on_demand: '{{ resource_prefix }}-pay-per-request' table_index: 'id' table_index_type: 'NUMBER' diff --git a/tests/integration/targets/dynamodb_table/tasks/main.yml b/tests/integration/targets/dynamodb_table/tasks/main.yml index c18c2a161b1..a5f3f7a3882 100644 --- a/tests/integration/targets/dynamodb_table/tasks/main.yml +++ b/tests/integration/targets/dynamodb_table/tasks/main.yml @@ -12,6 +12,7 @@ security_token: '{{ security_token | default(omit) }}' region: '{{ aws_region }}' block: + - include: "test_pay_per_request.yml" # ============================================== @@ -907,4 +908,5 @@ dynamodb_table: state: absent name: '{{ table_name }}' + wait: false register: delete_table diff --git a/tests/integration/targets/dynamodb_table/tasks/test_pay_per_request.yml b/tests/integration/targets/dynamodb_table/tasks/test_pay_per_request.yml index ca7bf567ccc..c202602e6b4 100644 --- a/tests/integration/targets/dynamodb_table/tasks/test_pay_per_request.yml +++ b/tests/integration/targets/dynamodb_table/tasks/test_pay_per_request.yml @@ -2,7 +2,7 @@ - name: Create table - pay-per-request - check_mode dynamodb_table: state: present - name: "{{ table_name }}" + name: "{{ table_name_on_demand }}" hash_key_name: "{{ table_index }}" hash_key_type: "{{ table_index_type }}" billing_mode: PAY_PER_REQUEST @@ -18,7 +18,7 @@ - name: Create table - pay-per-request dynamodb_table: state: present - name: "{{ table_name }}" + name: "{{ table_name_on_demand }}" hash_key_name: "{{ table_index }}" hash_key_type: "{{ table_index_type }}" billing_mode: PAY_PER_REQUEST @@ -28,7 +28,7 @@ assert: that: - create_table is successful - - create_table is not changed + - create_table is changed - create_table.billing_mode == "PAY_PER_REQUEST" # ============================================== @@ -36,7 +36,7 @@ - name: Create complex table - check_mode dynamodb_table: state: present - name: "{{ table_name }}" + name: "{{ table_name_on_demand }}" hash_key_name: "{{ table_index }}" hash_key_type: "{{ table_index_type }}" range_key_name: "{{ range_index }}" @@ -56,7 +56,7 @@ - name: Create complex table dynamodb_table: state: present - name: "{{ table_name }}" + name: "{{ table_name_on_demand }}" hash_key_name: "{{ table_index }}" hash_key_type: "{{ table_index_type }}" range_key_name: "{{ range_index }}" @@ -86,11 +86,35 @@ - create_complex_table.indexes | length == 2 - create_complex_table.range_key_name == range_index - create_complex_table.range_key_type == range_index_type - - create_complex_table.table_name == table_name + - create_complex_table.table_name == table_name_on_demand - create_complex_table.tags == tags_default +- name: Update complex table billing_mode + dynamodb_table: + state: present + name: "{{ table_name_on_demand }}" + hash_key_name: "{{ table_index }}" + hash_key_type: "{{ table_index_type }}" + range_key_name: "{{ range_index }}" + range_key_type: "{{ range_index_type }}" + billing_mode: PROVISIONED + read_capacity: 1 + write_capacity: 1 + tags: "{{ tags_default }}" + indexes: "{{ indexes }}" + register: convert_complex_table + +- name: Check results - Update complex table billing_mode + assert: + that: + - convert_complex_table is successful + - convert_complex_table is changed + - '"billing_mode" in convert_complex_table' + - convert_complex_table.billing_mode == "PROVISIONED" + - name: Delete table dynamodb_table: state: absent - name: "{{ table_name }}" + name: "{{ table_name_on_demand }}" + wait: false register: delete_table From bb03d72ce6d9ee6da99a99943d0a3d0424e00fae Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Mon, 11 Oct 2021 17:58:16 +0100 Subject: [PATCH 06/17] fix missed update --- plugins/modules/dynamodb_table.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index c0823bfbe8b..fe4366e3b6e 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -453,7 +453,7 @@ def get_dynamodb_table(): # billing_mode_summary is only set if the table is already PAY_PER_REQUEST if 'billing_mode_summary' in table: - table['billing_mode'] = table['billing_mode_summary']['billing_mode'] + table['billing_mode'] = "PAY_PER_REQUEST" else: table['billing_mode'] = "PROVISIONED" @@ -744,7 +744,7 @@ def _update_table(current_table): changes['ProvisionedThroughput'] = throughput_changes if current_table.get('billing_mode') != module.params.get('billing_mode'): - changes['BillingMode'] = billing_mode + changes['BillingMode'] = module.params.get('billing_mode') global_index_changes = _global_index_changes(current_table) if global_index_changes: From 5dfa6517e7bed1455b76b1881c65b538c131db3c Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Mon, 11 Oct 2021 20:40:40 +0100 Subject: [PATCH 07/17] fixes --- plugins/modules/dynamodb_table.py | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index fe4366e3b6e..b1212cf6647 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -132,7 +132,7 @@ description: - How long (in seconds) to wait for creation / update / deletion to complete. aliases: ['wait_for_active_timeout'] - default: 120 + default: 300 type: int wait: description: @@ -593,12 +593,6 @@ def _throughput_changes(current_table, params=None): def _generate_global_indexes(): index_exists = dict() indexes = list() - billing_mode = module.params.get('billing_mode') - - if billing_mode == "PROVISIONED": - is_provisioned = True - else: - is_provisioned = False for index in module.params.get('indexes'): if index.get('type') not in ['global_all', 'global_include', 'global_keys_only']: @@ -608,7 +602,7 @@ def _generate_global_indexes(): module.fail_json(msg='Duplicate key {0} in list of global indexes'.format(name)) # Convert the type name to upper case and remove the global_ index['type'] = index['type'].upper()[7:] - index = _generate_index(index, include_throughput=is_provisioned) + index = _generate_index(index) index_exists[name] = True indexes.append(index) @@ -666,7 +660,7 @@ def _generate_local_index_map(current_table): return local_index_map -def _generate_index(index, include_throughput=True): +def _generate_index(index): key_schema = _generate_schema(index) throughput = _generate_throughput(index) non_key_attributes = index['includes'] or [] @@ -689,7 +683,8 @@ def _generate_index(index, include_throughput=True): KeySchema=key_schema, Projection=projection, ) - if include_throughput: + + if module.params.get('billing_mode') == "PROVISIONED": idx['ProvisionedThroughput'] = throughput return idx @@ -717,13 +712,15 @@ def _global_index_changes(current_table): # rather than dropping other changes on the floor _current = current_global_index_map[name] _new = global_index_map[name] - change = dict(_throughput_changes(_current, _new)) - if change: - update = dict( - IndexName=name, - ProvisionedThroughput=change, - ) - index_changes.append(dict(Update=update)) + + if module.params.get('billing_mode') == "PROVISIONED": + change = dict(_throughput_changes(_current, _new)) + if change: + update = dict( + IndexName=name, + ProvisionedThroughput=change, + ) + index_changes.append(dict(Update=update)) return index_changes From f1005847d639de939291e4a66fc871129838e8af Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Mon, 11 Oct 2021 20:47:53 +0100 Subject: [PATCH 08/17] re-add removal of param --- plugins/modules/dynamodb_table.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index b1212cf6647..c353ada60c5 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -660,7 +660,7 @@ def _generate_local_index_map(current_table): return local_index_map -def _generate_index(index): +def _generate_index(index, include_throughput=True): key_schema = _generate_schema(index) throughput = _generate_throughput(index) non_key_attributes = index['includes'] or [] From dfc9b7232bd761ef956481e52d9009b0ab0a7b96 Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Tue, 12 Oct 2021 09:25:09 +0100 Subject: [PATCH 09/17] re-add removed section --- plugins/modules/dynamodb_table.py | 6 +++++- .../targets/dynamodb_table/defaults/main.yml | 21 +++++++++---------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index c353ada60c5..b2621dd79af 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -2,6 +2,10 @@ # Copyright: Ansible Project # 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 + + DOCUMENTATION = r''' --- module: dynamodb_table @@ -684,7 +688,7 @@ def _generate_index(index, include_throughput=True): Projection=projection, ) - if module.params.get('billing_mode') == "PROVISIONED": + if include_throughput and module.params.get('billing_mode') == "PROVISIONED": idx['ProvisionedThroughput'] = throughput return idx diff --git a/tests/integration/targets/dynamodb_table/defaults/main.yml b/tests/integration/targets/dynamodb_table/defaults/main.yml index d9e8b6995a0..96df5538146 100644 --- a/tests/integration/targets/dynamodb_table/defaults/main.yml +++ b/tests/integration/targets/dynamodb_table/defaults/main.yml @@ -1,12 +1,12 @@ --- -table_name: '{{ resource_prefix }}' -table_name_on_demand: '{{ resource_prefix }}-pay-per-request' +table_name: "{{ resource_prefix }}" +table_name_on_demand: "{{ resource_prefix }}-pay-per-request" -table_index: 'id' -table_index_type: 'NUMBER' +table_index: "id" +table_index_type: "NUMBER" -range_index: 'variety' -range_index_type: 'STRING' +range_index: "variety" +range_index_type: "STRING" indexes: - name: NamedIndex @@ -53,13 +53,12 @@ index_updated: type: global_all read_capacity: 4 - tags_default: snake_case_key: snake_case_value camelCaseKey: camelCaseValue PascalCaseKey: PascalCaseValue - 'key with spaces': value with spaces - 'Upper With Spaces': Upper With Spaces + "key with spaces": value with spaces + "Upper With Spaces": Upper With Spaces partial_tags: snake_case_key: snake_case_value @@ -69,5 +68,5 @@ updated_tags: updated_snake_case_key: updated_snake_case_value updatedCamelCaseKey: updatedCamelCaseValue UpdatedPascalCaseKey: UpdatedPascalCaseValue - 'updated key with spaces': updated value with spaces - 'updated Upper With Spaces': Updated Upper With Spaces + "updated key with spaces": updated value with spaces + "updated Upper With Spaces": Updated Upper With Spaces From 4760c6c87dee6ba30b99bc93fdf08d4d85eff9c7 Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Tue, 12 Oct 2021 10:31:14 +0100 Subject: [PATCH 10/17] adjust tests can't update primary index --- .../targets/dynamodb_table/tasks/test_pay_per_request.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/integration/targets/dynamodb_table/tasks/test_pay_per_request.yml b/tests/integration/targets/dynamodb_table/tasks/test_pay_per_request.yml index c202602e6b4..059561dea05 100644 --- a/tests/integration/targets/dynamodb_table/tasks/test_pay_per_request.yml +++ b/tests/integration/targets/dynamodb_table/tasks/test_pay_per_request.yml @@ -59,8 +59,6 @@ name: "{{ table_name_on_demand }}" hash_key_name: "{{ table_index }}" hash_key_type: "{{ table_index_type }}" - range_key_name: "{{ range_index }}" - range_key_type: "{{ range_index_type }}" billing_mode: PAY_PER_REQUEST tags: "{{ tags_default }}" indexes: "{{ indexes_pay_per_request }}" @@ -84,8 +82,6 @@ - create_complex_table.hash_key_name == table_index - create_complex_table.hash_key_type == table_index_type - create_complex_table.indexes | length == 2 - - create_complex_table.range_key_name == range_index - - create_complex_table.range_key_type == range_index_type - create_complex_table.table_name == table_name_on_demand - create_complex_table.tags == tags_default @@ -95,8 +91,6 @@ name: "{{ table_name_on_demand }}" hash_key_name: "{{ table_index }}" hash_key_type: "{{ table_index_type }}" - range_key_name: "{{ range_index }}" - range_key_type: "{{ range_index_type }}" billing_mode: PROVISIONED read_capacity: 1 write_capacity: 1 From 84f4e1b3e0520b0378e72efbe09a85ecdb8fae04 Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Tue, 12 Oct 2021 11:40:57 +0100 Subject: [PATCH 11/17] multiple indexes can now be updated at once --- plugins/modules/dynamodb_table.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index b2621dd79af..a0eb632be91 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -455,9 +455,10 @@ def get_dynamodb_table(): table['size'] = table['table_size_bytes'] table['tags'] = tags - # billing_mode_summary is only set if the table is already PAY_PER_REQUEST + # billing_mode_summary doesn't always seem to be set but is always set for PAY_PER_REQUEST + # and when updating the billing_mode if 'billing_mode_summary' in table: - table['billing_mode'] = "PAY_PER_REQUEST" + table['billing_mode'] = table['billing_mode_summary']['billing_mode'] else: table['billing_mode'] = "PROVISIONED" @@ -750,12 +751,6 @@ def _update_table(current_table): global_index_changes = _global_index_changes(current_table) if global_index_changes: changes['GlobalSecondaryIndexUpdates'] = global_index_changes - # Only one index can be changed at a time, pass the first during the - # main update and deal with the others on a slow retry to wait for - # completion - if len(global_index_changes) > 1: - changes['GlobalSecondaryIndexUpdates'] = [global_index_changes[0]] - additional_global_index_changes = global_index_changes[1:] local_index_changes = _local_index_changes(current_table) if local_index_changes: From 4fff25cd5b46eb3ff4f34150225467fa2b9ba2b6 Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Tue, 12 Oct 2021 14:26:26 +0100 Subject: [PATCH 12/17] indexes indexes --- plugins/modules/dynamodb_table.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index a0eb632be91..299e684b4a1 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -751,6 +751,14 @@ def _update_table(current_table): global_index_changes = _global_index_changes(current_table) if global_index_changes: changes['GlobalSecondaryIndexUpdates'] = global_index_changes + # Only one index can be changed at a time except if changing the billing mode, pass the first during the + # main update and deal with the others on a slow retry to wait for + # completion + + if current_table.get('billing_mode') == module.params.get('billing_mode'): + if len(global_index_changes) > 1: + changes['GlobalSecondaryIndexUpdates'] = [global_index_changes[0]] + additional_global_index_changes = global_index_changes[1:] local_index_changes = _local_index_changes(current_table) if local_index_changes: From c9572de579669563d1d44f9fdb9214d717b7f9b0 Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Tue, 12 Oct 2021 21:33:46 +0100 Subject: [PATCH 13/17] PR feedback --- plugins/modules/dynamodb_table.py | 41 ++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index 299e684b4a1..a9e19268db5 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -53,7 +53,6 @@ description: - Controls whether provisoned pr on-demand tables are created. choices: ['PROVISIONED', 'PAY_PER_REQUEST'] - default: 'PROVISIONED' type: str read_capacity: description: @@ -383,9 +382,6 @@ def compatability_results(current_table): billing_mode = current_table.get('billing_mode') - if billing_mode == "PROVISIONED": - throughput = current_table.get('provisioned_throughput', {}) - primary_indexes = _decode_primary_index(current_table) hash_key_name = primary_indexes.get('hash_key_name') @@ -419,6 +415,7 @@ def compatability_results(current_table): ) if billing_mode == "PROVISIONED": + throughput = current_table.get('provisioned_throughput', {}) compat_results['read_capacity'] = throughput.get('read_capacity_units', None) compat_results['write_capacity'] = throughput.get('write_capacity_units', None) @@ -595,10 +592,15 @@ def _throughput_changes(current_table, params=None): return dict() -def _generate_global_indexes(): +def _generate_global_indexes(billing_mode): index_exists = dict() indexes = list() + include_throughput = True + + if billing_mode == "PAY_PER_REQUEST": + include_throughput = False + for index in module.params.get('indexes'): if index.get('type') not in ['global_all', 'global_include', 'global_keys_only']: continue @@ -607,7 +609,7 @@ def _generate_global_indexes(): module.fail_json(msg='Duplicate key {0} in list of global indexes'.format(name)) # Convert the type name to upper case and remove the global_ index['type'] = index['type'].upper()[7:] - index = _generate_index(index) + index = _generate_index(index, include_throughput) index_exists[name] = True indexes.append(index) @@ -689,7 +691,7 @@ def _generate_index(index, include_throughput=True): Projection=projection, ) - if include_throughput and module.params.get('billing_mode') == "PROVISIONED": + if include_throughput: idx['ProvisionedThroughput'] = throughput return idx @@ -704,11 +706,19 @@ def _global_index_changes(current_table): current_global_index_map = current_table['_global_index_map'] global_index_map = _generate_global_index_map(current_table) + current_billing_mode = current_table.get('billing_mode') + + include_throughput = True + + if module.params.get('billing_mode', current_billing_mode) == "PAY_PER_REQUEST": + include_throughput = False + index_changes = list() # TODO (future) it would be nice to add support for deleting an index for name in global_index_map: - idx = dict(_generate_index(global_index_map[name])) + + idx = dict(_generate_index(global_index_map[name], include_throughput=include_throughput)) if name not in current_global_index_map: index_changes.append(dict(Create=idx)) else: @@ -718,7 +728,7 @@ def _global_index_changes(current_table): _current = current_global_index_map[name] _new = global_index_map[name] - if module.params.get('billing_mode') == "PROVISIONED": + if include_throughput: change = dict(_throughput_changes(_current, _new)) if change: update = dict( @@ -745,8 +755,11 @@ def _update_table(current_table): if throughput_changes: changes['ProvisionedThroughput'] = throughput_changes - if current_table.get('billing_mode') != module.params.get('billing_mode'): - changes['BillingMode'] = module.params.get('billing_mode') + current_billing_mode = current_table.get('billing_mode') + new_billing_mode = module.params.get('billing_mode', current_billing_mode) + + if current_billing_mode != new_billing_mode: + changes['BillingMode'] = new_billing_mode global_index_changes = _global_index_changes(current_table) if global_index_changes: @@ -755,7 +768,7 @@ def _update_table(current_table): # main update and deal with the others on a slow retry to wait for # completion - if current_table.get('billing_mode') == module.params.get('billing_mode'): + if current_billing_mode == new_billing_mode: if len(global_index_changes) > 1: changes['GlobalSecondaryIndexUpdates'] = [global_index_changes[0]] additional_global_index_changes = global_index_changes[1:] @@ -871,7 +884,7 @@ def create_table(): attributes = _generate_attributes() key_schema = _generate_schema() local_indexes = _generate_local_indexes() - global_indexes = _generate_global_indexes() + global_indexes = _generate_global_indexes(billing_mode) params = dict( TableName=table_name, @@ -961,7 +974,7 @@ def main(): hash_key_type=dict(type='str', choices=KEY_TYPE_CHOICES), range_key_name=dict(type='str'), range_key_type=dict(type='str', choices=KEY_TYPE_CHOICES), - billing_mode=dict(default='PROVISIONED', type='str', choices=['PROVISIONED', 'PAY_PER_REQUEST']), + billing_mode=dict(type='str', choices=['PROVISIONED', 'PAY_PER_REQUEST']), read_capacity=dict(type='int'), write_capacity=dict(type='int'), indexes=dict(default=[], type='list', elements='dict', options=index_options), From 4c60973e97f615cbd0acae97716d17afabaac7e0 Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Wed, 13 Oct 2021 10:33:07 +0100 Subject: [PATCH 14/17] fix creation when billing_mode is omitted --- plugins/modules/dynamodb_table.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index a9e19268db5..3f19afd4f89 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -868,7 +868,7 @@ def update_table(current_table): def create_table(): table_name = module.params.get('name') hash_key_name = module.params.get('hash_key_name') - billing_mode = module.params.get('billing_mode') + billing_mode = module.params.get('billing_mode', 'PROVISIONED') tags = ansible_dict_to_boto3_tag_list(module.params.get('tags') or {}) From 9e350c1aafba45bee5d925fc147310e7de3cbc6d Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Wed, 13 Oct 2021 17:39:07 +0100 Subject: [PATCH 15/17] handle the fact the param is always set but to None --- plugins/modules/dynamodb_table.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index 3f19afd4f89..8696161c58e 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -868,7 +868,10 @@ def update_table(current_table): def create_table(): table_name = module.params.get('name') hash_key_name = module.params.get('hash_key_name') - billing_mode = module.params.get('billing_mode', 'PROVISIONED') + billing_mode = module.params.get('billing_mode') + + if billing_mode == None: + billing_mode = "PROVISIONED" tags = ansible_dict_to_boto3_tag_list(module.params.get('tags') or {}) From 7502fcf8d3ecfb3531f50f76cc4fbad9134d8363 Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Wed, 13 Oct 2021 20:13:27 +0100 Subject: [PATCH 16/17] tests fix --- plugins/modules/dynamodb_table.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index 8696161c58e..a8725ab7d32 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -708,9 +708,14 @@ def _global_index_changes(current_table): current_billing_mode = current_table.get('billing_mode') + if module.params.get('billing_mode') is None: + billing_mode = current_billing_mode + else: + billing_mode = module.params.get('billing_mode') + include_throughput = True - if module.params.get('billing_mode', current_billing_mode) == "PAY_PER_REQUEST": + if billing_mode == "PAY_PER_REQUEST": include_throughput = False index_changes = list() @@ -756,7 +761,10 @@ def _update_table(current_table): changes['ProvisionedThroughput'] = throughput_changes current_billing_mode = current_table.get('billing_mode') - new_billing_mode = module.params.get('billing_mode', current_billing_mode) + new_billing_mode = module.params.get('billing_mode') + + if new_billing_mode is None: + new_billing_mode = current_billing_mode if current_billing_mode != new_billing_mode: changes['BillingMode'] = new_billing_mode From 51b0fc5b6ad2978f03d07b9c07734b14b420f4f6 Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Wed, 13 Oct 2021 22:20:22 +0100 Subject: [PATCH 17/17] lint fix --- plugins/modules/dynamodb_table.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/dynamodb_table.py b/plugins/modules/dynamodb_table.py index a8725ab7d32..98d6fa632f9 100644 --- a/plugins/modules/dynamodb_table.py +++ b/plugins/modules/dynamodb_table.py @@ -878,7 +878,7 @@ def create_table(): hash_key_name = module.params.get('hash_key_name') billing_mode = module.params.get('billing_mode') - if billing_mode == None: + if billing_mode is None: billing_mode = "PROVISIONED" tags = ansible_dict_to_boto3_tag_list(module.params.get('tags') or {})