Add ResourceType on ec2 snapshot copy #1419
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ansibullbot
added
bug
tremble
requested changes
Aug 31, 2022
There was a problem hiding this comment.
Hi @bplaxco,
Thanks for taking the time to submit this PR. The change itself looks correct, however I'd ask for two minor additions:
- Please add a changelog fragment: https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to
- (optional) It would be really good if you could add an integration test. These are (in essence) just an an Ansible role which try to systematically exercise the module to test it behaves as expected, and live in
tests/integration/targets/. A good starting point might be https://github.com/ansible-collections/amazon.aws/tree/main/tests/integration/targets/ec2_snapshot from the amazon.aws collection
ansibullbot
added
needs_revision
|
Changelog fragment added. Looking into the tests. |
ansibullbot
added
community_review
integration
|
@tremble in the zuul ouput I get: "error": {
"code": "UnauthorizedOperation",
"message": "You are not authorized to perform this operation. Encoded authorization failure message: SZOho26QkeN31KcOxBTUnC_sBSeRa3G9qqAmqJOLtkkAiUvLcOIlJVojSNLkyZa7MXEHWbTOAtc5aemsWR_tKiqEZtwr5OlwXk4PbtKTtj0NdpDaBeDs1q-Mpt7zDktDjJmk41tSPe-Rjjqe1ftFJhuZ76rQKHOXEeLol1QsaW2kA9x74EEf6gbr13kMUi2CyF3D-XTFMLkpZj-XShxzE9QbF1xvmnvwxIbmwuVk0PWFUX8dKbQbeB4K_jSCBLIV5Ag3Rio0Foc4h6uYfBmTz_vOR7J2ukyrRdgn5dwIc_78jFpa2DSqPa93EbGXeXjW6fTtzFvsExeRynD7-QBDY4iZEYGE2inBGZHZO7elmhh4Ie9G-xV6g2hBZVlWShrDzxGoJjanOiqTxl3530TxVRO-YO3kIzDbG55DouvtyOGdJyQntxyeIB2eR0gZhI5maCAcsD2YODtRyhwIDIXhGiBKmB4GmZ90ZgfQ0zHkLf4aLr5xxZcpeqf3juVUpMUMdOIWQ_VIdldVmPvx13e4FrsVACt4509Uh1vt5R7N-vX6YSh5g9CDrJ8"
}The module args passed in to ec2_snapshot_copy seem legit from what I know about the module, would someone be able to take a look and confirm the service account used has the right permissions? Or would you rather me mark the tests as disabled or something like that? |
colbyshores
approved these changes
Sep 12, 2022
Fixes this issue that shows up when tags are specified: An error occurred waiting for the snapshot to become available.: An error occurred (InvalidParameterValue) when calling the CopySnapshot operation: Tag specification resource type must have a value ~ B'ezrat Hashem ~
The new tests seem to be failing due to a permissions issue with the service account used to run them. Since they were marked as optional in the MR I'm going to remove them but leave them in the history[1] so the work's not lost in case someone wants to use them. [1] The tests are still on aec7b2f
|
I went ahead and removed the tests to get the MR through since they were marked as optional, but since they seemed to be failing because of a permission issue with the service account running them, I kept them in the history[1] in case we want to pull them back in. Feel free to ping me on gchat at work if you'd like to sync up on getting the test running ^_^ [1] The tests are still on aec7b2f |
|
Hey @tremble just wanted to see if you need anything else from me on this ^_^ |
ringmany
approved these changes
Nov 25, 2022
tremble
approved these changes
Nov 25, 2022
|
@bplaxco sorry about the delay here. LGTM |
tremble
added
mergeit
Backport to stable-5: 💚 backport PR created✅ Backport PR branch: Backported as #1605 🤖 @patchback |
patchback bot
pushed a commit
that referenced
this pull request
Nov 25, 2022
Add ResourceType on ec2 snapshot copy
SUMMARY
Fixes this issue that shows up when tags are specified:
An error occurred waiting for the snapshot to become available.: An
error occurred (InvalidParameterValue) when calling the CopySnapshot
operation: Tag specification resource type must have a value
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
ec2_snapshot_copy
ADDITIONAL INFORMATION
# before
botocore.exceptions.ClientError: An error occurred (InvalidParameterValue) when calling the CopySnapshot operation: Tag specification resource type must have a value
fatal: [localhost]: FAILED! => {
"boto3_version": "1.24.57",
"botocore_version": "1.27.58",
"changed": false,
"error": {
"code": "InvalidParameterValue",
"message": "Tag specification resource type must have a value"
},
....
# after (no error message)
changed: [localhost] => {
"changed": true,
"invocation": {
"module_args": {
.....
Reviewed-by: Mark Chappell <None>
Reviewed-by: Colby Shores <[email protected]>
Reviewed-by: None <None>
(cherry picked from commit fad3589)
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
softwarefactory-project-zuul bot
pushed a commit
that referenced
this pull request
Nov 25, 2022
[PR #1419/fad35892 backport][stable-5] Add ResourceType on ec2 snapshot copy This is a backport of PR #1419 as merged into main (fad3589). SUMMARY Fixes this issue that shows up when tags are specified: An error occurred waiting for the snapshot to become available.: An error occurred (InvalidParameterValue) when calling the CopySnapshot operation: Tag specification resource type must have a value ISSUE TYPE Bugfix Pull Request COMPONENT NAME ec2_snapshot_copy ADDITIONAL INFORMATION # before botocore.exceptions.ClientError: An error occurred (InvalidParameterValue) when calling the CopySnapshot operation: Tag specification resource type must have a value fatal: [localhost]: FAILED! => { "boto3_version": "1.24.57", "botocore_version": "1.27.58", "changed": false, "error": { "code": "InvalidParameterValue", "message": "Tag specification resource type must have a value" }, .... # after (no error message) changed: [localhost] => { "changed": true, "invocation": { "module_args": { ..... Reviewed-by: Mark Chappell <None>
1 task
abikouo
pushed a commit
to abikouo/community.aws
that referenced
this pull request
Oct 24, 2023
…llections#1419) route53_info: fix "key error" for health_check operations SUMMARY Fixes ansible-collections#1396 This pull request Add new return key health_check_observations for health check operations, returned when I(query=health_check) and I(health_check_method=status) or I(health_check_method=failure_reason) Fixes "Key Error" when getting status or failure_reason of a health check. ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info ADDITIONAL INFORMATION Reviewed-by: Mark Chappell Reviewed-by: Mandar Kulkarni <[email protected]> Reviewed-by: Alina Buzachis
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Fixes this issue that shows up when tags are specified:
ISSUE TYPE
COMPONENT NAME
ec2_snapshot_copy
ADDITIONAL INFORMATION