Handle same hostname for multiple pods in a headless service

[guillaumerose]

This code improves the handling of headless services pointing at pods sharing
the same hostname.

• For each hostname, A records should point at the list of pods IPs.
  Currently, only the last pod gets its A record.
• For the service, a A record should be created for each pod.

Fixes #116

cc @dgageot

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

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/test-infra repository. I understand the commands that are listed here.

[thockin]

I know the spec says it should return multiple A records in this case, but I just want to think it through. Is this the correct behavior? It's less surprising than dropping all-but-one, though I could argue that using the same hostname twice is a bug on the user's end.

So is the spec wrong or is the code wrong? If we implement this - what else might break? Are there apps that actually expect to use DNS as "phantom elimination"? I feel the current behavior is under-specified to rely on that, but the implementation is reasonably deterministic.

@bowei @johnbelamaric @smarterclayton @erictune @kubernetes/sig-apps-misc

[erictune]

@kow3ns

[erictune]

Using the same hostname is not a bug.

Spreading may be soft rather than hard.

Someone may want to bring up a size-3 etcd in their 2-node cluster as part of a test. They don't need HA, they just want to reuse the config. That should work as similarly as possible to the "normal" case where the pods are properly spread.

[thockin]

@erictune I am not clear on your reply. Let me provide an example and you tell me which behavior is considered correct.

T0: User somehow creates a set of pods that have hostnames "app-1", "app-2", "app-3". DNS lookups of "service" return 3 distinct A records. DNS lookups of "app-[123].service" each return one distinct A record.

T1: Some management event happens and, through cut-paste or similar, the app-3 pod is replaced with a new pod that has hostname "app-2". DNS lookups of "service" return 3 distinct A records. Question: should a DNS lookup of "app-2" return a single A record (one of the 2 pods, chosen somehow) or should it return 2 A records?

Today it returns 1 record.

[thockin]

Another note: if we decide the spec is wrong, and that we should only serve one A record, we have a bug. When you hit this overlap, the IP of the last pod created "wins". If that pod goes away, we do not update the PTR to the other pod with the same hostname.

[erictune]

Please disregard previous comment. I was not aware of how the hostname override behavior affected DNS. I don't know of any use cases that break either way. Ken may know some.

…

On Fri, Jul 7, 2017 at 10:31 AM, Tim Hockin ***@***.***> wrote: Another note: if we decide the spec is wrong, and that we should only serve one A record, we have a bug. When you hit this overlap, the IP of the last pod created "wins". If that pod goes away, we do not update the PTR to the other pod with the same hostname. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#123 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHuudieh9DlWhKR8hXezcR2XsdssdxC0ks5sLmtjgaJpZM4OQ5rn> .

[dgageot]

Thanks @thockin for the feedback.

When we prepared this PR, we tried real hard to not break existing things and we considered this whole "phantom elimination" couldn't be considered a feature given what's in the spec. But I understand that nonetheless, it might be a breaking change.

What do you think we can do to mitigate that risk?

[johnbelamaric]

@chrisohaver I believe in CoreDNS we include all IPs. This is consistent with ordinary DNS behavior - it is legal in DNS to have multiple A records with the same name and different IPs.

[guillaumerose]

We fixed the CLA check. Any chance to get this merged ? Should we do something ?
Thanks !

[dgageot]

ping @thockin

[guillaumerose]

Any update on this ?

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale