feat(gateway): add --resolve-gateway-load-balancer-hostname flag#6286
feat(gateway): add --resolve-gateway-load-balancer-hostname flag#6286Apoorva64 wants to merge 10 commits into
Conversation
|
Welcome @Apoorva64! |
|
Hi @Apoorva64. 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 Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
My main concern is that we're slowly accumulating per-source flags. We already have --resolve-service-load-balancer-hostname, and adding --resolve-gateway-load-balancer-hostname starts a pattern that's hard to stop -> next comes --resolve-ingress-, --resolve-istio-, and so on. Once those flags are out in the wild we're stuck with them. I think we can do better here. Two other options we could/should consider:
Either way, the underlying feature make sense to land in external-dns. |
|
I think that option A would be better as when we are using external DNS, the From what i see this feature is thus tied to a single deployment and should be enabled with a global flag. |
|
I've deleted my initial review comment. It was too big so I simplified, but without options A-B now. Option A — Generalize the existing --resolve-service-load-balancer-hostname into a single --resolve-load-balancer-hostname flag that applies uniformly across all sources producing hostname-type targets. Option B — Introduce a per-resource annotation (e.g. external-dns.alpha.kubernetes.io/resolve-target: "true") as a source-agnostic opt-in, and open a design discussion first. |
|
|
||
| If the `--resolve-gateway-load-balancer-hostname` flag is specified, any address with type | ||
| `Hostname` is queried through DNS and any resulting IP addresses are added instead of the hostname. | ||
| A DNS query failure results in zero targets being added for that address. |
There was a problem hiding this comment.
This will create endpoint without addresses? It will fail the DNS apply
There was a problem hiding this comment.
I fixed the docs. In the case of DNS failure for an Endpoint, External Dns will log the error and skip the endpoint
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
0b44bf6 to
1c99b54
Compare
We should probably add this for other sources but i think it would be another PR? |
|
Maybe annotation is worth to implement first, as it's provides a better opt-in functionality? Worth to check contribution docs https://github.com/kubernetes-sigs/external-dns/blob/master/docs/contributing/source-wrappers.md for some design ideas. This may take a while, as pros/cons not yet clear of each approach. |
i've drafted another approch using the wrappers you mentioned here #6289 it still uses a global var to activate and desactivate the feature. |
|
Try to configure locall cluster, this should simplify things
On top of that - wort to ask Chatgpt, Claude and DeepWiki https://deepwiki.com/kubernetes-sigs/external-dns |
|
PR needs rebase. DetailsInstructions 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. |
/close |
|
@ivankatliarchuk: Closed this PR. DetailsIn 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. |
What does this PR do?
Adds a new
--resolve-gateway-load-balancer-hostnameboolean flag (default:false) to all Gateway API route sources (HTTPRoute, GRPCRoute, TLSRoute, TCPRoute, UDPRoute).When enabled, any
status.addressesentry on a Gateway withtype: Hostnameis resolved via DNS (net.LookupIP) and the resulting IP addresses are used as endpoint targets instead of the hostname. This producesA/AAAArecords rather thanCNAMErecords.If DNS resolution fails for a hostname, zero targets are added for that address (same behavior as
--resolve-service-load-balancer-hostname).Motivation
Feature parity with
--resolve-service-load-balancer-hostname, which provides the same behavior forServiceresources of typeLoadBalancer. Some DNS providers or configurations do not supportCNAMErecords (e.g., internal Dns), requiring IP-based records.More