rfc2136: merge Endpoints with same name/type into single Endpoint #4613
Conversation
k8s-ci-robot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
cncf-cla: yes
|
Welcome @bogdando! |
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @bogdando. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
the
size/L
map multiple DNS records with the same name/type into the external-dns which models such records as a single Endpoint with multiple entries in the Targets attribute. Solution: Generalize the solution introduced to the DigitalOcean provider by kubernetes-sigs#1595 by refactoring the mergeEndpointsByNameType function into the endpoint module. Then use that function in the RFC2136 provider. Authored-by: Tom Dyas <[email protected]> Signed-off-by: Bogdan Dobrelya <[email protected]>
|
I checked that this works for rfc2136 provider with endpoints list of a ~700 records, but all targets have a length of 1. It works without problems, or regressions. I don't have ideas how to make an artificial scenario with target len > 1. I hope unit tests coverage is sufficient for such a case. @tdyas FYI |
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
| @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
|
|
|||
| - Added support for `extraContainers` argument. ([#4432](https://github.com/kubernetes-sigs/external-dns/pull/4432)) _@omerap12_ | |||
| - Added support for setting `excludeDomains` argument. ([#4380](https://github.com/kubernetes-sigs/external-dns/pull/4380)) _@bford-evs_ | |||
| - Fixed reconciling `A` records that have more than 1 target for `rfc2136` provider. ([4613#](https://github.com/kubernetes-sigs/external-dns/pull/4613)) _@tdyas_ | |||
There was a problem hiding this comment.
| - Fixed reconciling `A` records that have more than 1 target for `rfc2136` provider. ([4613#](https://github.com/kubernetes-sigs/external-dns/pull/4613)) _@tdyas_ |
This changelog is for the helm chart
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
@bogdando Would you please remove the line on the changelog ? Otherwise, the pr looks good. |
k8s-ci-robot
added
the
needs-rebase
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Since there is no answer since a few month, I close it. |
|
@mloiseleur: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Description
Rebased and reopened no longer maintained #1678
Related #1596
Map multiple DNS records with the same name/type into the external-dns which models such records as a single Endpoint with multiple entries in the Targets attribute.
Solution: Generalize the solution introduced to the DigitalOcean provider by #1595 by refactoring the mergeEndpointsByNameType function into the endpoint module. Then use that function in the RFC2136 provider.
Fixes #1596
Checklist