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.

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

rds_instance - add snapshot tests, update docs, refactor tests #1081

Conversation

jatorcasso
Copy link
Contributor

@jatorcasso jatorcasso commented Apr 20, 2022

Depends-On: ansible-collections/amazon.aws#776
Depends-On: #1105

SUMMARY
  • add snapshot tests to test restoring db from snapshot and fix bugs associated
  • fix some typos in documentation and remove duplicate parameter (added as alias so no breaking change)
  • remove unused IAM role in tests and add some missing cleanups
ISSUE TYPE
  • Bugfix Pull Request
  • Feature Pull Request
COMPONENT NAME

rds_instance

ADDITIONAL INFORMATION

this module had both db_snapshot_identifier and snapshot_identifier as separate params, with the latter being required to restore from snapshot, resulting in some parameter missing errors. moving snapshot_identifier as an alias of db_snapshot_identifier fixes this issue.

@ansibullbot
Copy link

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review integration tests/integration module module needs_triage plugins plugin (any type) tests tests labels Apr 20, 2022
@markuman
Copy link
Member

recheck

@jatorcasso
Copy link
Contributor Author

Failing due to An error occurred (AccessDenied) when calling the RestoreDBInstanceFromDBSnapshot operation

@jatorcasso
Copy link
Contributor Author

recheck

@jatorcasso jatorcasso closed this Apr 20, 2022
@jatorcasso jatorcasso reopened this Apr 20, 2022
@jatorcasso jatorcasso closed this Apr 20, 2022
@jatorcasso jatorcasso reopened this Apr 20, 2022
@jatorcasso jatorcasso marked this pull request as draft April 20, 2022 22:54
@gravesm
Copy link
Member

gravesm commented Apr 21, 2022

recheck

@jatorcasso jatorcasso closed this Apr 21, 2022
@jatorcasso jatorcasso reopened this Apr 21, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 4m 25s (non-voting)
✔️ build-ansible-collection SUCCESS in 6m 08s
✔️ ansible-test-sanity-docker-devel SUCCESS in 9m 42s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 11m 07s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 11m 51s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 11m 25s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 43s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 35s
✔️ ansible-test-splitter SUCCESS in 3m 01s
✔️ integration-community.aws-1 SUCCESS in 47m 26s
✔️ integration-community.aws-2 SUCCESS in 11m 25s
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@jatorcasso jatorcasso requested a review from alinabuzachis May 10, 2022 02:12
@jatorcasso jatorcasso force-pushed the rds_instance/update-testing branch from a0093f2 to b70e8f3 Compare May 10, 2022 04:08
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 4m 31s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 46s
✔️ ansible-test-sanity-docker-devel SUCCESS in 9m 39s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 50s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 13m 57s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 10m 49s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 07s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 56s
✔️ ansible-test-splitter SUCCESS in 3m 52s
✔️ integration-community.aws-1 SUCCESS in 41m 20s
✔️ integration-community.aws-2 SUCCESS in 13m 52s
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@@ -1290,7 +1300,6 @@ def main():
s3_ingestion_role_arn=dict(),
s3_prefix=dict(),
skip_final_snapshot=dict(type='bool', default=False),
snapshot_identifier=dict(),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if this needs to be handled in a different way. @tremble @markuman

jatorcasso and others added 3 commits May 10, 2022 10:45
Co-authored-by: Alina Buzachis <abuzachis@redhat.com>
Co-authored-by: Alina Buzachis <abuzachis@redhat.com>
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 4m 20s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 56s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 25s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 46s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 12m 33s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 9m 53s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 25s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 8m 28s
✔️ ansible-test-splitter SUCCESS in 4m 18s
✔️ integration-community.aws-1 SUCCESS in 47m 50s
✔️ integration-community.aws-2 SUCCESS in 11m 21s
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@@ -1290,7 +1300,6 @@ def main():
s3_ingestion_role_arn=dict(),
s3_prefix=dict(),
skip_final_snapshot=dict(type='bool', default=False),
snapshot_identifier=dict(),
Copy link
Contributor

Choose a reason for hiding this comment

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

The key question IMO is "Could this break an existing playbook?" I've tried poking through the logic and I can't spot any way that this would break an existing playbook...

Copy link
Collaborator

@s-hertel s-hertel left a comment

Choose a reason for hiding this comment

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

Looks good!

@jatorcasso jatorcasso added the mergeit Merge the PR (SoftwareFactory) label May 24, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

ansible-galaxy-importer FAILURE in 4m 21s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 20s
✔️ ansible-test-sanity-docker-devel SUCCESS in 9m 06s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 11m 46s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 11m 01s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 10m 47s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 17s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 5m 50s
✔️ ansible-test-splitter SUCCESS in 5m 51s
✔️ integration-community.aws-1 SUCCESS in 46m 48s
✔️ integration-community.aws-2 SUCCESS in 12m 42s
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 5d5bca9 into ansible-collections:main May 24, 2022
jatorcasso added a commit to jatorcasso/community.aws that referenced this pull request Jun 6, 2022
…le-collections#1081)

rds_instance - add snapshot tests, update docs, refactor tests

Depends-On: ansible-collections/amazon.aws#776
Depends-On: ansible-collections#1105
SUMMARY

add snapshot tests to test restoring db from snapshot and fix bugs associated
fix some typos in documentation and remove duplicate parameter (added as alias so no breaking change)
remove unused IAM role in tests and add some missing cleanups

ISSUE TYPE

Bugfix Pull Request
Feature Pull Request

COMPONENT NAME
rds_instance
ADDITIONAL INFORMATION
this module had both db_snapshot_identifier and snapshot_identifier as separate params, with the latter being required to restore from snapshot, resulting in some parameter missing errors. moving snapshot_identifier as an alias of db_snapshot_identifier fixes this issue.

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: Sloane Hertel <None>
(cherry picked from commit 5d5bca9)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jun 7, 2022
Backport stable-3: rds_instance

SUMMARY
Manual backports of #1081 #1196
ISSUE TYPE

Bugfix Pull Request
Feature Pull Request

COMPONENT NAME
rds_instance

Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
…le-collections#1081)

rds_instance - add snapshot tests, update docs, refactor tests

Depends-On: ansible-collections/amazon.aws#776
Depends-On: ansible-collections#1105
SUMMARY

add snapshot tests to test restoring db from snapshot and fix bugs associated
fix some typos in documentation and remove duplicate parameter (added as alias so no breaking change)
remove unused IAM role in tests and add some missing cleanups

ISSUE TYPE

Bugfix Pull Request
Feature Pull Request

COMPONENT NAME
rds_instance
ADDITIONAL INFORMATION
this module had both db_snapshot_identifier and snapshot_identifier as separate params, with the latter being required to restore from snapshot, resulting in some parameter missing errors. moving snapshot_identifier as an alias of db_snapshot_identifier fixes this issue.

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: Sloane Hertel <None>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@5d5bca9
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
…le-collections#1081)

rds_instance - add snapshot tests, update docs, refactor tests

Depends-On: ansible-collections/amazon.aws#776
Depends-On: ansible-collections#1105
SUMMARY

add snapshot tests to test restoring db from snapshot and fix bugs associated
fix some typos in documentation and remove duplicate parameter (added as alias so no breaking change)
remove unused IAM role in tests and add some missing cleanups

ISSUE TYPE

Bugfix Pull Request
Feature Pull Request

COMPONENT NAME
rds_instance
ADDITIONAL INFORMATION
this module had both db_snapshot_identifier and snapshot_identifier as separate params, with the latter being required to restore from snapshot, resulting in some parameter missing errors. moving snapshot_identifier as an alias of db_snapshot_identifier fixes this issue.

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: Sloane Hertel <None>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@5d5bca9
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
…ons#1209)

lambda_event: Add support for FunctionResponseTypes

SUMMARY

Resolves ansible-collections#1081
Added support to set FunctionResponseTypes when creating lambda event source mappings

ISSUE TYPE


Feature Pull Request

COMPONENT NAME

lambda_event
ADDITIONAL INFORMATION


Can be tested using below playbook (needs a dynamo db table and lambda function with appropriate access/policy/role)

---
- name: lambda_event
  hosts: localhost
  gather_facts: false
  tasks:
    - name: Create DynamoDB stream event mapping (trigger)
      amazon.aws.lambda_event:
        state: present
        event_source: stream
        function_name: my-test-function
        source_params:
          source_arn: arn:aws:dynamodb:us-east-2:721234567890:table/my-test-table/stream/2022-10-26T17:55:25.232
          enabled: True
          batch_size: 500
          starting_position: LATEST
          function_response_types:
            - ReportBatchItemFailures
      register: event

    - name: Get info on above trigger
      command: 'aws lambda get-event-source-mapping --uuid {{ event.events.uuid }}'
      environment:
        AWS_ACCESS_KEY_ID: "{{ aws_access_key }}"
        AWS_SECRET_ACCESS_KEY: "{{ aws_secret_key }}"
        AWS_SESSION_TOKEN: "{{ security_token | default('') }}"
        AWS_DEFAULT_REGION: "{{ aws_region }}"
      register: my_function_details

    - name: convert it to an object
      set_fact:
        my_function_details_obj: "{{ my_function_details.stdout | from_json }}"

    - assert:
        that:
          - my_function_details_obj.FunctionResponseTypes is defined
          - my_function_details_obj.FunctionResponseTypes | length > 0
          - my_function_details_obj.FunctionResponseTypes[0] == "ReportBatchItemFailures"

Reviewed-by: Bikouo Aubin <None>
Reviewed-by: Mike Graves <mgraves@redhat.com>
Reviewed-by: Mandar Kulkarni <mandar242@gmail.com>
Reviewed-by: Alina Buzachis <None>
Reviewed-by: Gonéri Le Bouder <goneri@lebouder.net>
Reviewed-by: GomathiselviS <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug community_review integration tests/integration mergeit Merge the PR (SoftwareFactory) module module plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants