-
Notifications
You must be signed in to change notification settings - Fork 397
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 eip association when both instance id and private ip address are passed #328
Fix eip association when both instance id and private ip address are passed #328
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, the change itself looks good, thanks for updating the test cases too.
Please add a changelog fragment: https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to
Hmm it looks like the changes are failing: https://app.shippable.com/github/ansible-collections/community.aws/runs/1139/24/tests
|
It seems to me those failures are unrelated. Line 62 is very early in the file where everything is setup for the actual tests. If you haven't seen this before, maybe there has been other changes to AWS APIs. Maybe the I'll look into the changelog fragment. 👍 |
Hi @Phoosha yeah, I'm seeing lots of CI failures related to the Instance APIs, I suspect this is an issue on the AWS side which got combined with a large number of tests going through at the same time. |
changelogs/fragments/328-fix-ec2_eip-instance-id-private-ip-address.yml
Outdated
Show resolved
Hide resolved
The CI issue appears to be related to how ec2_instance finds instances by name, I'm attempting to get it working again. Once we can run this test suite successfully we should be good to merge this change. |
Thank you for your contribution, unfortunately the CI issues we saw from ec2_instance meant we didn't get this merged before 1.3.0, but it shouldn't be too long before the next release. |
Thanks for keeping me in the loop, Mark. I appreciate it 👍 |
…passed (ansible-collections#328) * Fix ec2_eip with both instance_id and private_ip_address * Add changelog fragment for the ec2_eip fix
…passed (ansible-collections#328) * Fix ec2_eip with both instance_id and private_ip_address * Add changelog fragment for the ec2_eip fix
…passed (ansible-collections#328) * Fix ec2_eip with both instance_id and private_ip_address * Add changelog fragment for the ec2_eip fix
…ctions#328) aws_ec2 inventory: add includes_filters and excludes_filters Reviewed-by: https://github.com/apps/ansible-zuul
…_contrib_script_compatible_ec2_tag_keys (ansible-collections#858) Add version_added: 1.5.0 for exclude_filters, include_filters and use_contrib_script_compatible_ec2_tag_keys SUMMARY Add version_added: 1.5.0 for exclude_filters, include_filters and use_contrib_script_compatible_ec2_tag_keys include_filters and exclude_filters have been added ansible-collections#328 and released with amazon.aws 1.5.0 use_contrib_script_compatible_ec2_tag_keys has been added ansible-collections#331 and released with amazon.aws 1.5.0 Let's update the aws_ec2 inventory plugin documentation with this information. This should be a step towards closing this one ansible-collections#676 and ansible-collections#675 aws_ec2 documentation will be enriched with exhaustive examples in an upcoming PR. ISSUE TYPE Docs Pull Request COMPONENT NAME aws_ec2 ADDITIONAL INFORMATION Reviewed-by: Mark Chappell <None>
…passed (ansible-collections#328) * Fix ec2_eip with both instance_id and private_ip_address * Add changelog fragment for the ec2_eip fix This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections@4d1aa98
I assume there has recently been a change to the AWS API introducing case sensitivity to the
PrivateIpAddress
parameter ofAssociateAddress
. Theec2_eip
module uses this parameter in two branches (instance id vs interface id) and only in the instance branch it usesPrivateIPAddress
(note how it'sP
and notp
).This PR also adds a variant of an existing test case to cover the branch that triggered this failure.
Example
This has worked for me for a long time. But when running it today I got this error: