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.

Sign up for GitHub

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

Jump to bottom

Fix IOPs io1 DB instance updates and integration tests also #878

Merged
merged 8 commits into from
Feb 4, 2022
Merged

Fix IOPs io1 DB instance updates and integration tests also #878

merged 8 commits into from
Feb 4, 2022

Conversation

marknet15
Copy link
Contributor

@marknet15 marknet15 commented Jan 20, 2022 •
edited
Loading

SUMMARY

Primary this PR is to fix updates when updating iops or allocated_storage on io1 DB instances when only one param is changing.

Secondarily this fixes up the tests again and is test against some improvements to the waiter configuration see linked PR.

IOPs error on update attempts if only one param is being updated:

  error:
    code: InvalidParameterCombination
    message: You must specify both the storage size and iops when modifying the storage size or iops on a DB instance that has iops.
    type: Sender
  msg: 'Unable to modify DB instance: An error occurred (InvalidParameterCombination) when calling the ModifyDBInstance operation: You must specify both the storage size and iops when modifying the storage size or iops on a DB instance that has iops.'
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

rds_instance

ADDITIONAL INFORMATION

These tests are very slow and still a little flakey but generally all pass as expected now locally.

marknet15
marknet15 commented Jan 20, 2022
View reviewed changes
tests/integration/targets/rds_instance/inventory
tagging
replica
Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed these to ensure we sit in the resource length limits as there was a load of issues on the 3.0.0 release around these limits being hit leading to these tests being disabled.

@softwarefactory-project-zuul
Copy link
Contributor

This change depends on a change that failed to merge.

@marknet15
Copy link
Contributor Author

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@marknet15
Copy link
Contributor Author

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@marknet15
Copy link
Contributor Author

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@marknet15
Copy link
Contributor Author

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@alinabuzachis
Copy link
Contributor

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@ansibullbot
Copy link

cc @jillr @markuman @s-hertel @tremble
click here for bot help

@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 Jan 28, 2022
@ansibullbot ansibullbot removed the WIP Work in progress label Feb 3, 2022
@markuman markuman added backport-2 PR should be backported to the stable-2 branch backport-3 PR should be backported to the stable-3 branch labels Feb 3, 2022
alinabuzachis
alinabuzachis reviewed Feb 4, 2022
View reviewed changes
Copy link
Contributor

@alinabuzachis alinabuzachis left a comment

Choose a reason for hiding this comment

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

May I ask you to update the RETURN statement please?
I quickly checked and it seems: "associated_roles": [], is not documented, while it seems kms_key_idis not returned even if documented in the RETURN statement.
Could you please double check?

tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_complex.yml Outdated Show resolved Hide resolved
@marknet15
Copy link
Contributor Author

marknet15 commented Feb 4, 2022 •
edited
Loading

May I ask you to update the RETURN statement please? I quickly checked and it seems: "associated_roles": [], is not documented, while it seems kms_key_idis not returned even if documented in the RETURN statement. Could you please double check?

@alinabuzachis so I checked and "kms_key_id": null can be seen in output logs although I don't think there's any examples of it being set in the integration tests currently. I have though added associated_roles

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (third-party-check pipeline).

@alinabuzachis alinabuzachis added the mergeit Merge the PR (SoftwareFactory) label Feb 4, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 45e79ed into ansible-collections:main Feb 4, 2022
@patchback
Copy link

patchback bot commented Feb 4, 2022 •
edited
Loading

Backport to stable-2: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 45e79ed on top of patchback/backports/stable-2/45e79ed2e8a60d87b20f77cfbef4ba8a27da1260/pr-878

Backporting merged PR #878 into main

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it: 
    $ git remote add upstream https://github.com/ansible-collections/community.aws.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code: 
    $ git fetch upstream
$ git checkout -b patchback/backports/stable-2/45e79ed2e8a60d87b20f77cfbef4ba8a27da1260/pr-878 upstream/stable-2
  4. Now, cherry-pick PR Fix IOPs io1 DB instance updates and integration tests also #878 contents into that branch: 
    $ git cherry-pick -x 45e79ed2e8a60d87b20f77cfbef4ba8a27da1260
    If it'll yell at you with something like fatal: Commit 45e79ed2e8a60d87b20f77cfbef4ba8a27da1260 is a merge but no -m option was given., add -m 1 as follows intead: 
    $ git cherry-pick -m1 -x 45e79ed2e8a60d87b20f77cfbef4ba8a27da1260
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR Fix IOPs io1 DB instance updates and integration tests also #878 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub: 
    $ git push origin patchback/backports/stable-2/45e79ed2e8a60d87b20f77cfbef4ba8a27da1260/pr-878
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@patchback
Copy link

patchback bot commented Feb 4, 2022 •
edited
Loading

Backport to stable-3: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-3/45e79ed2e8a60d87b20f77cfbef4ba8a27da1260/pr-878

Backported as #927

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Feb 4, 2022
@marknet15 @patchback
Fix IOPs io1 DB instance updates and integration tests also (#878)
1253afd 
Fix IOPs io1 DB instance updates and integration tests also

SUMMARY
Primary this PR is to fix updates when updating iops or allocated_storage on io1 DB instances when only one param is changing.
Secondarily this fixes up the tests again and is test against some improvements to the waiter configuration see linked PR.
IOPs error on update attempts if only one param is being updated:
  error:
    code: InvalidParameterCombination
    message: You must specify both the storage size and iops when modifying the storage size or iops on a DB instance that has iops.
    type: Sender
  msg: 'Unable to modify DB instance: An error occurred (InvalidParameterCombination) when calling the ModifyDBInstance operation: You must specify both the storage size and iops when modifying the storage size or iops on a DB instance that has iops.'

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
rds_instance
ADDITIONAL INFORMATION
These tests are very slow and still a little flakey but generally all pass as expected now locally.

Reviewed-by: Mark Woolley <[email protected]>
Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Alina Buzachis <None>
(cherry picked from commit 45e79ed)
@patchback patchback bot mentioned this pull request Feb 4, 2022
Merged
@marknet15 marknet15 deleted the rds-iops-fix branch February 4, 2022 16:15
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Feb 5, 2022
@patchback
Fix IOPs io1 DB instance updates and integration tests also (#878) (#927
30ce931 
)

[PR #878/45e79ed2 backport][stable-3] Fix IOPs io1 DB instance updates and integration tests also

This is a backport of PR #878 as merged into main (45e79ed).
SUMMARY
Primary this PR is to fix updates when updating iops or allocated_storage on io1 DB instances when only one param is changing.
Secondarily this fixes up the tests again and is test against some improvements to the waiter configuration see linked PR.
IOPs error on update attempts if only one param is being updated:
  error:
    code: InvalidParameterCombination
    message: You must specify both the storage size and iops when modifying the storage size or iops on a DB instance that has iops.
    type: Sender
  msg: 'Unable to modify DB instance: An error occurred (InvalidParameterCombination) when calling the ModifyDBInstance operation: You must specify both the storage size and iops when modifying the storage size or iops on a DB instance that has iops.'

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
rds_instance
ADDITIONAL INFORMATION
These tests are very slow and still a little flakey but generally all pass as expected now locally.
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
@tremble
ec2_group - Add egress_rules and purge_egress_rules aliases (ansible-…
922800d 
…collections#878)

ec2_group - Add egress_rules and purge_egress_rules aliases

SUMMARY
Fix ansible-collections#440 - exception running with --diff and --check when creating a new SG by adding a rule referencing a missing SG.
Add egress_rules and purge_egress_rules aliases - while cleaning up the tests I kept writing "egress_rules" rather than "rules_egress".
Cleanup and re-enable ec2_group tests

Remove "Classic Networking" tests (we don't have this in CI and it'll be gone in August)
Fix Group/VPC deletion in tests (cross-referenced rules weren't being deleted, wipe the rules first)

ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ec2_group
ADDITIONAL INFORMATION
fixes: ansible-collections#440

Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
@marknet15 @goneri
Fix IOPs io1 DB instance updates and integration tests also (ansible-…
2aa3c30 
…collections#878)

Fix IOPs io1 DB instance updates and integration tests also

SUMMARY
Primary this PR is to fix updates when updating iops or allocated_storage on io1 DB instances when only one param is changing.
Secondarily this fixes up the tests again and is test against some improvements to the waiter configuration see linked PR.
IOPs error on update attempts if only one param is being updated:
  error:
    code: InvalidParameterCombination
    message: You must specify both the storage size and iops when modifying the storage size or iops on a DB instance that has iops.
    type: Sender
  msg: 'Unable to modify DB instance: An error occurred (InvalidParameterCombination) when calling the ModifyDBInstance operation: You must specify both the storage size and iops when modifying the storage size or iops on a DB instance that has iops.'

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
rds_instance
ADDITIONAL INFORMATION
These tests are very slow and still a little flakey but generally all pass as expected now locally.

Reviewed-by: Mark Woolley <[email protected]>
Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Alina Buzachis <None>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@45e79ed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alinabuzachis alinabuzachis alinabuzachis approved these changes

@markuman markuman markuman approved these changes

Assignees
No one assigned
Labels
backport-2 PR should be backported to the stable-2 branch backport-3 PR should be backported to the stable-3 branch 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@marknet15 @alinabuzachis @ansibullbot @markuman