zeppelin
zeppelin copied to clipboard
apache
[ZEPPELIN-5475] fix zep k8s service hostname
What is this PR for?
According doc at https://kubernetes.io/docs/concepts/services-networking/service/ , service lookup must end by svc.cluster.local. This PR fix it.
What type of PR is it?
Bug fix
Todos
- [ ] - Task
What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-5475
Questions:
- Does the licenses files need update? no
- Is there breaking changes for older versions? no
- Does this needs documentation? no
Thanks @ebuildy for the contribution, I don't find that service lookup must end by svc.cluster.local in the doc, could you point me the exact place ?
Hello @zjffdu , I found DNS spec at https://github.com/kubernetes/dns/blob/master/docs/specification.md , also GKE doc is more explicit https://cloud.google.com/kubernetes-engine/docs/concepts/service-discovery?hl=en , or in the classic doc at https://kubernetes.io/docs/concepts/services-networking/dns-pod-service/.
The .cluster.local is the "cluster domain", in 99% of k8s cluster I have worked on, this is always the same value. But this can change, in a multiple clusters, this become .clusterset.local.
How do you test zeppelin on k8s?
@ebuildy Thanks for the clarification, this sounds like convention, right ? Anyway, this PR LGTM
"Cluster.local" as you mentioned is not always correct. I would prefer either a domain value that can be configured or add documentation that you need to configure your cluster DNS service with a search domain as described in the DNS K8s documentation.
Absolutely right @Reamer , in practice, I have never seen a cluster with a different search domain.
Also, we could use only the zeppelin domain name, without the namespace (if interpreter pod is in the same namespace of course)?
Do you want me to create a config entry ? Something like k8s_cluster_search_domain
Do you want me to create a config entry ? Something like k8s_cluster_search_domain
Yes, please. This is then a possible breaking change (for 1%). Please update your PR description. You also need to update the documentation. The default for the configuration value should of course be "cluster.local".