Skip to content
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

Move discovery to DNS names #3712

Closed
wants to merge 3 commits into from
Closed

Conversation

pebrc
Copy link
Collaborator

@pebrc pebrc commented Sep 4, 2020

Fixes #2830
Related #3654

This PR changes Elasticsearch discovery to use DNS hostnames instead of IP addresses. To replicate the discussion on #3654 : we think the potential negative impact of DNS caching is manageable and outweighs the advantage of simplifying discovery.

We could in theory remove the IP address from the ECK-generated transport certificates now and save a few certificate regenerations when a node is scheduled on another Pod. I left it in place because I believe the frequency of these regenerations if fairly low but open to reconsider.

@pebrc pebrc added >enhancement Enhancement of existing functionality v1.3.0 labels Sep 4, 2020
Copy link
Contributor

@sebgl sebgl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Side note: using DNS names only in the certificates (#2830) would help fixing #2832 and #2833.

//PodDNSName returns the DNS resolvable name of the given pod resolvable within the namespace.
func PodDNSName(pod corev1.Pod) string {
ssetName := pod.Labels[label.StatefulSetNameLabelName]
return fmt.Sprintf("%s.%s", pod.Name, HeadlessServiceName(ssetName))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should consider how a user may override the Pod hostname and subdomain fields: https://kubernetes.io/docs/concepts/services-networking/dns-pod-service/#pod-s-hostname-and-subdomain-fields
But I think that's really a corner case that is probably safe to ignore for now. It probably has other impacts.

@pebrc
Copy link
Collaborator Author

pebrc commented Sep 10, 2020

Closing in the light of #3723 (comment)

@pebrc pebrc closed this Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement Enhancement of existing functionality v1.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Pod DNS names instead of IP addresses for discovery and TLS certificates
3 participants