ec2_eip: add support of updating reverse dns record for eip #2292
Conversation
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
|
Build failed.
|
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 23s |
There was a problem hiding this comment.
I think what we expect here is just the possibility to modify an EIP attribute that is either newly created or already existing.
- name: Create new EIP and update DNS Record
ec2_eip:
state: present
domain_name: example.netor
- name: Update PTR Record of existing EIP
ec2_eip:
public_ip: 1.2.3.4
domain_name: example.net
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 5m 00s |
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 5m 06s |
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 40s |
|
@abikouo added tests, waiting on terminator for permissions PR to merge |
mandar242
force-pushed
the
aaws-1296
branch
from
d4b9146
to
1f4be3a
Compare
|
Build failed. ✔️ ansible-galaxy-importer SUCCESS in 4m 13s |
| vars: | ||
| has_no_new_eip: true | ||
|
|
||
| - name: Allocate a new EIP and modify it's reverse DNS record |
There was a problem hiding this comment.
Can you also please add an idempotency check?
There was a problem hiding this comment.
moved to slack channel for discussion
There was a problem hiding this comment.
@alinabuzachis I have updated the tested to include more testing. The tests pass locally
"ec2:DescribeVpcs",
"ec2:DeleteVpc"
],
"vpc": {}
}
PLAY RECAP *********************************************************************
testhost : ok=45 changed=17 unreachable=0 failed=0 skipped=16 rescued=0 ignored=0
AWS ACTIONS: ['ec2:AllocateAddress', 'ec2:AttachInternetGateway', 'ec2:AuthorizeSecurityGroupIngress', 'ec2:CreateInternetGateway', 'ec2:CreateNetworkInterface', 'ec2:CreateSecurityGroup', 'ec2:CreateSubnet', 'ec2:CreateVpc', 'ec2:DeleteInternetGateway', 'ec2:DeleteNetworkInterface', 'ec2:DeleteSecurityGroup', 'ec2:DeleteSubnet', 'ec2:DeleteVpc', 'ec2:DescribeAddresses', 'ec2:DescribeAddressesAttribute', 'ec2:DescribeAvailabilityZones', 'ec2:DescribeInternetGateways', 'ec2:DescribeNetworkInterfaces', 'ec2:DescribeSecurityGroups', 'ec2:DescribeSubnets', 'ec2:DescribeTags', 'ec2:DescribeVpcAttribute', 'ec2:DescribeVpcs', 'ec2:DetachInternetGateway', 'ec2:ModifyAddressAttribute', 'ec2:ModifyVpcAttribute', 'ec2:ReleaseAddress', 'ec2:ResetAddressAttribute', 'route53:ChangeResourceRecordSets', 'route53:GetChange', 'route53:ListHostedZones', 'route53:ListResourceRecordSets', 'sts:GetCallerIdentity']
Command exited with status 0 after 435.9944279193878 seconds.
But, for now, I have disabled the tests for reverse DNS record feature as it requires use of registered domain and a corresponding hosted zone to successfully add reverse DNS record to an EIP.
Slack thread started for discussion on how to handle this.
mandar242
force-pushed
the
aaws-1296
branch
from
715e4dc
to
3c0d6c0
Compare
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 37s |
| tags: "{{ eip_test_tags }}" | ||
| register: eip | ||
|
|
||
| - name: Add EIP IP address an A record |
There was a problem hiding this comment.
Do we really need this step to test the newly added parameter? While it is beneficial to have an additional test for full functionality, I believe that if ec2_eip supports adding or modifying the EIP with a domain name and runs correctly in our CI account, we might not need to do the Route53 testing. What do you think? cc @alinabuzachis @abikouo
There was a problem hiding this comment.
from what I understand if A record is not present in the hosted zone of domain being added to EIP reverse DNS recrod,
- The reverse DNS record does not get applied and fails
- There will be no way to test idempotency as there will be no successful EIP with domain applied
that's why initially I had skipped adding idempotency test @GomathiselviS @alinabuzachis
| - eip.public_ip is defined and ( eip.public_ip | ansible.utils.ipaddr ) | ||
| - eip.allocation_id is defined and eip.allocation_id.startswith("eipalloc-") | ||
|
|
||
| - name: Wait for reverse DNS record update to complete |
There was a problem hiding this comment.
yes, update takes more than a minute to complete, if not completed and we move to next task then the next task fails saying another request already pending on EIP
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 46s |
plugins/modules/ec2_eip.py
Outdated
| description: The domain name to attach to the IP address. | ||
| required: false | ||
| type: str | ||
| version_added: 9.0.0 |
There was a problem hiding this comment.
Why not including this feature into 8.3.0?
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 5m 16s |
|
Build succeeded (gate pipeline). ✔️ ansible-galaxy-importer SUCCESS in 6m 08s |
softwarefactory-project-zuul
bot
merged commit efcaf86
into
ansible-collections:main
…collections#2292) SUMMARY Add support of updating reverse dns record for eip Fixes ansible-collections#1296 ISSUE TYPE Feature Pull Request COMPONENT NAME ec2_eip ADDITIONAL INFORMATION https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/ec2/client/modify_address_attribute.html Reviewed-by: Bikouo Aubin Reviewed-by: Mandar Kulkarni <[email protected]> Reviewed-by: Alina Buzachis Reviewed-by: GomathiselviS
SUMMARY
Add support of updating reverse dns record for eip
Fixes #1296
ISSUE TYPE
COMPONENT NAME
ec2_eip
ADDITIONAL INFORMATION
https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/ec2/client/modify_address_attribute.html