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

Fix RDS Enhanced Monitoring bug #712

Closed
wants to merge 31 commits into from

Conversation

marknet15
Copy link
Contributor

@marknet15 marknet15 commented Sep 15, 2021

SUMMARY

This is a fix for an issue when an RDS instance already exists and you wish to enable enhanced monitoring, for the full details see the linked old reported issue:
ansible/ansible#51772

But in summary currently if you enable enhanced monitoring on an RDS instance that already exists where it isn't already enabled then the following is returned:

An exception occurred during task execution. To see the full traceback, use -vvv. The error was: KeyError: 'MonitoringRoleArn'
fatal: [localhost_eu-west-1-pdv-qa-1 -> 127.0.0.1]: FAILED! => changed=false 
  module_stderr: |-
    Traceback (most recent call last):
      File "master:/opt/mitogen/mitogen-0.2.9/ansible_mitogen/runner.py", line 975, in _run
        self._run_code(code, mod)
      File "master:/opt/mitogen/mitogen-0.2.9/ansible_mitogen/runner.py", line 939, in _run_code
        exec(code, vars(mod))
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 1245, in <module>
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 1210, in main
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 855, in get_parameters
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 885, in get_options_with_changing_values
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 983, in get_changing_options_with_consistent_keys
    KeyError: 'MonitoringRoleArn'
  module_stdout: ''
  msg: |-
    MODULE FAILURE
    See stdout/stderr for the exact error

Depends-On: mattclay/aws-terminator#164

Other changes
A load of issues have surfaced in the integration tests due to how slow RDS is to create / modify etc. I've condensed down the tests where possible reducing the number of inventory jobs to 6 and bumped serial to 6 so that hopefully all tests can run at once and finish within the 1 hr AWS session duration.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

rds_instance

@ansibullbot
Copy link

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review module module needs_triage plugins plugin (any type) small_patch Hopefully easy to review traceback labels Sep 15, 2021
@alinabuzachis
Copy link
Contributor

@marknet15 Thank you for working on this. Would you be so kind to add some integration tests to test this functionality?

@ansibullbot ansibullbot added integration tests/integration tests tests and removed small_patch Hopefully easy to review labels Sep 17, 2021
@marknet15
Copy link
Contributor Author

@alinabuzachis thanks for the comment 👍🏻 I've added some integration tests, but upon inspection it doesn't look like zuul has run any, it seems to be skipping and I can't see why, would you be able to assist? I'm a little stuck, as "tests/integration/targets/" seems correct:

source ~/venv/bin/activate; ansible-test integration --diff --no-temp-workdir --skip-tags False --retry-on-error --continue-on-error --python 3.6 -vvvv rds_instance
stdout
RLIMIT_NOFILE: (1024, 524288)
MAXFD: -1
Falling back to tests in "tests/integration/targets/" because "roles/test/" was not found.
WARNING: Excluding tests marked "unsupported" which require --allow-unsupported or prefixing with "unsupported/": rds_instance
Run command: ssh-keygen -m PEM -q -t rsa -N '' -f /home/zuul/.ansible/test/id_rsa
Working directory: /home/zuul/.ansible/collections/ansible_collections/community/aws
Program found: /usr/bin/ssh-keygen
HOME=/home/zuul
LC_ALL=en_US.UTF-8
PATH=/home/zuul/venv/bin:/home/zuul/.local/bin:/home/zuul/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin
Command exited with status 0 after 0.33289432525634766 seconds.
WARNING: All targets skipped.

https://dashboard.zuul.ansible.com/t/ansible/build/8075a26688a749f69fed570912827d6a/console

@marknet15
Copy link
Contributor Author

marknet15 commented Sep 18, 2021

@alinabuzachis I think If I'm following this correctly the rds_instance integration tests are actually disabled:
file: community.aws/tests/integration/targets/rds_instance/aliases

# reason: missing-policy
# reason: slow
unsupported

What would be the next steps ?

@alinabuzachis
Copy link
Contributor

recheck

marknet15 and others added 4 commits September 27, 2021 11:52
Co-authored-by: Mark Chappell <mchappel@redhat.com>
- Condense tests where possible
- Remove irrelevant snapshot tests
- Up concurrency to 6
Copy link
Contributor

@jillr jillr left a comment

Choose a reason for hiding this comment

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

A few things I ran across while testing the policy PR. I did still run into the 60 minutes STS timeout but it got much further this time. Thanks so much for all you're doing for these tests!

@@ -100,6 +108,8 @@
# TODO: test modifying db_subnet_group_name, db_security_groups, db_parameter_group_name, option_group_name,
# monitoring_role_arn, monitoring_interval, domain, domain_iam_role_name, cloudwatch_logs_export_configuration

# Test multiple modifications including enabling enhanced monitoring
Copy link
Contributor

Choose a reason for hiding this comment

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

RDS doesn't like the version or parameter combination in this one.
"msg": "Unable to modify DB instance: An error occurred (InvalidParameterCombination) when calling the ModifyDBInstance operation: Cannot upgrade mariadb from 10.3.20 to 10.4.8",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm according to the docs AWS allows 10.3 to 10.4 as a major version upgrade:
https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_UpgradeDBInstance.MariaDB.html

But might be some inconsistencies in the tests, I've updated things slightly

@ansibullbot
Copy link

@marknet15 this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html and removed community_review labels Oct 8, 2021
@ansibullbot
Copy link

@marknet15 this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot
Copy link

@marknet15 This PR was evaluated as a potentially problematic PR for the following reasons:

  • More than 50 changed files.

Such PR can only be merged by human. Contact a Core team member to review this PR on IRC: #ansible-devel on irc.freenode.net

click here for bot help

@ansibullbot ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Oct 8, 2021
@marknet15
Copy link
Contributor Author

Apologies I completely messed up my branch 👎🏻 and created a clean one:
#747

@marknet15 marknet15 closed this Oct 8, 2021
@marknet15 marknet15 deleted the rds-fix branch October 8, 2021 08:54
ansible-zuul bot pushed a commit that referenced this pull request Oct 22, 2021
Rds enhanced monitoring bug fix

SUMMARY
(a copy of #712 as I messed up my branch by accident)
This is a fix for an issue when an RDS instance already exists and you wish to enable enhanced monitoring, for the full details see the linked old reported issue:
ansible/ansible#51772
But in summary currently if you enable enhanced monitoring on an RDS instance that already exists where it isn't already enabled then the following is returned:
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: KeyError: 'MonitoringRoleArn'
fatal: [localhost_eu-west-1-pdv-qa-1 -> 127.0.0.1]: FAILED! => changed=false 
  module_stderr: |-
    Traceback (most recent call last):
      File "master:/opt/mitogen/mitogen-0.2.9/ansible_mitogen/runner.py", line 975, in _run
        self._run_code(code, mod)
      File "master:/opt/mitogen/mitogen-0.2.9/ansible_mitogen/runner.py", line 939, in _run_code
        exec(code, vars(mod))
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 1245, in <module>
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 1210, in main
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 855, in get_parameters
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 885, in get_options_with_changing_values
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 983, in get_changing_options_with_consistent_keys
    KeyError: 'MonitoringRoleArn'
  module_stdout: ''
  msg: |-
    MODULE FAILURE
    See stdout/stderr for the exact error


Originally-Depends-On: mattclay/aws-terminator#164
Other changes
A load of issues have surfaced in the integration tests due to how slow RDS is to create / modify etc. I've condensed down the tests where possible reducing the number of inventory jobs to 6 and bumped serial to 6 so that hopefully all tests can run at once and finish within the 1 hr AWS session duration.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
rds_instance

Reviewed-by: Mark Chappell <None>
Reviewed-by: Mark Woolley <mw@marknet15.com>
Reviewed-by: None <None>
alinabuzachis added a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
Pin pytest version

SUMMARY


ISSUE TYPE


Bugfix Pull Request
Docs Pull Request
Feature Pull Request
New Module Pull Request

COMPONENT NAME

ADDITIONAL INFORMATION
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
Rds enhanced monitoring bug fix

SUMMARY
(a copy of ansible-collections#712 as I messed up my branch by accident)
This is a fix for an issue when an RDS instance already exists and you wish to enable enhanced monitoring, for the full details see the linked old reported issue:
ansible/ansible#51772
But in summary currently if you enable enhanced monitoring on an RDS instance that already exists where it isn't already enabled then the following is returned:
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: KeyError: 'MonitoringRoleArn'
fatal: [localhost_eu-west-1-pdv-qa-1 -> 127.0.0.1]: FAILED! => changed=false
  module_stderr: |-
    Traceback (most recent call last):
      File "master:/opt/mitogen/mitogen-0.2.9/ansible_mitogen/runner.py", line 975, in _run
        self._run_code(code, mod)
      File "master:/opt/mitogen/mitogen-0.2.9/ansible_mitogen/runner.py", line 939, in _run_code
        exec(code, vars(mod))
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 1245, in <module>
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 1210, in main
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 855, in get_parameters
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 885, in get_options_with_changing_values
      File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 983, in get_changing_options_with_consistent_keys
    KeyError: 'MonitoringRoleArn'
  module_stdout: ''
  msg: |-
    MODULE FAILURE
    See stdout/stderr for the exact error

Originally-Depends-On: mattclay/aws-terminator#164
Other changes
A load of issues have surfaced in the integration tests due to how slow RDS is to create / modify etc. I've condensed down the tests where possible reducing the number of inventory jobs to 6 and bumped serial to 6 so that hopefully all tests can run at once and finish within the 1 hr AWS session duration.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
rds_instance

Reviewed-by: Mark Chappell <None>
Reviewed-by: Mark Woolley <mw@marknet15.com>
Reviewed-by: None <None>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@8fe00cb
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 integration tests/integration merge_commit This PR contains at least one merge commit. Please resolve! module module needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR plugins plugin (any type) tests tests traceback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants