-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
add dualstack support for AWS provider with ALB ingress controllers #1079
Conversation
Welcome @twilfong! |
/assign @linki |
… in actual vs expected. (This shouldn't really be part of the PR, but it is intermitently breaking builds.)
Another question: Is this cleaned up correctly? When you delete the ingress again, does it clean up both records? |
Yes it does delete both records. Any change (Create, Upsert, or Delete) that goes through the AWS provider will now check for the presence of a dualstack label (with value = "true") in the registry entry for the given endoint (e.g. the TXT record if using TXT registry) if the endpoint returns true for isAWSLoadBalancer(). In the case where these two things are true, the single change of type A is turned into two changes, both A and AAAA. This works for creates, deletes, or upserts. I modified the TestAWSCreateRecordsWithALIAS case in aws_test.go to include a dualstack loadbalancer. But, we might want to add cases that test delete and update. I did run manual tests to ensure that this works, (deletes and updates,) and the codepath definitely is the same for any kind change. I'm not sure how much extra value would be added by specific test cases for dualstack endpoint upsert/delete, but they would be simple enough to add to aws_test.go if we want them. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: njuettner The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Does this work for dualstack NLB on K8s services as well? For example if you use a traefik or nginx or similar load balancer, but with NLB, external-dns enters that as the A record. |
This PR adds dualstack support for ALB ingress, and fixes issue #520.