source/pod: skip internal-hostname pod when PodIP is empty#6382
Conversation
addInternalHostnameAnnotationEndpoints called endpoint.SuitableType(pod.Status.PodIP)
and addToEndpointMap without first checking whether PodIP was empty.
Pending / Scheduled pods that carry external-dns.alpha.kubernetes.io/internal-hostname
ended up producing a CNAME record whose target was the empty string,
because SuitableType("") falls through to CNAME. Once the pod got a
PodIP and external-dns tried to write an A record at the same name,
the provider rejected the update (A and CNAME cannot coexist), leaving
the zone permanently stuck on the empty-target CNAME.
Skip the pod entirely when PodIP is empty. The pod will be reconciled
again once the kubelet assigns an address.
Fixes kubernetes-sigs#6375.
Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>
|
[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 |
|
Welcome @SAY-5! |
|
Hi @SAY-5. 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. |
|
Invalid commit message issues detected Invalid commit messagesKeywords which can automatically close issues and hashtag(#) mentions are not allowed.
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. I understand the commands that are listed here. |
| // Pods in Pending / Scheduled state without a PodIP must | ||
| // not emit an endpoint: endpoint.SuitableType("") returns | ||
| // CNAME, which would create a provider record whose target | ||
| // is the empty string. Once the pod eventually gets an IP | ||
| // external-dns then tries to create an A record at the | ||
| // same name and the provider rejects it because A and | ||
| // CNAME cannot coexist (#6375, #6199 on Azure / RFC2136). |
There was a problem hiding this comment.
I don’t think this comment is necessary, as it’s already clear why we filter out pods without an IP. Endpoint shouldn't have an empty address.
vflaux
left a comment
There was a problem hiding this comment.
pod.Status.PodIP is also used in addKopsDNSControllerEndpoints() and addPodSourceDomainEndpoints(). I think we need to filter there too.
Could you also add tests for those cases?
vflaux
left a comment
There was a problem hiding this comment.
I'm surprised that an endpoint with an empty target is accepted by your provider, I would expect it to fail.
Description
addInternalHostnameAnnotationEndpointscalledendpoint.SuitableType(pod.Status.PodIP)andaddToEndpointMapwithout first checking whether PodIP was empty. Pending / Scheduled pods carryingexternal-dns.alpha.kubernetes.io/internal-hostnameemit a CNAME record whose target is the empty string (becauseSuitableType("")falls through toCNAME). Once the pod gets a PodIP and external-dns tries to write an A record at the same name, the provider rejects the update (A and CNAME cannot coexist), leaving the zone permanently stuck on the empty-target CNAME, the exact scenario the reporter hit on Azure Private DNS.Fix
Skip the pod entirely when
PodIPis empty. The pod will be reconciled again once the kubelet assigns an address.Fixes
Fixes #6375
Checklist
go test ./source/...).go build ./source/...clean.gofmtclean.Signed-off-by: SAY-5 SAY-5@users.noreply.github.com