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

set spec hostname to get headless service to work #626

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kagesenshi
Copy link

@welcome
Copy link

welcome bot commented Aug 10, 2022

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@consideRatio
Copy link
Member

Thanks @kagesenshi!

    pod.spec.hostname = name
    return pod

I'm not yet confident enough to merge this, thinking about questions like this:

  • Does hostname part of the k8s Pod specification has restrictions on what we can specify, for example only a certain set of characters? If so, is what we assign to it always an acceptable string as well?
  • Does setting hostname risk influencing things outside the pod, or is it just influencing the internals of containers running in the pod?
  • Do we perceive a risk that other software stops working when we now start declaring a hostname?

Unless we are confident this is a good default, I suggest we instead recommend that users running into this use extra_pod_config to configure hostname.

@kagesenshi
Copy link
Author

kagesenshi commented Sep 8, 2022

unfortunately i don't think i have an answer for that. How I found out the fix is by comparing the containers that was created in k8s through k8s yml component yml file and finding the key that was missing, the most obvious missing key was this.

hostname afaik is limited to valid DNS domain characters and subject to the usual DNS character limits.

iirc when trying to solve this, extra_pod_config was unable to set this property there are no way to know what is the generated container name, closest method that can override this attribute is through modify_pod_hook.

@dolfinus
Copy link
Contributor

You can use extra_pod_config with name: "jupyter-{username}--{servername}" where "jupyter-{username}--{servername}" is a value of c.KubeSpawner.pod_name_template. Kubespawner will resolve substitutions before updating the spec

@kagesenshi
Copy link
Author

@dolfinus

the issue here is not about setting pod_name_template, but rather this pod name is not being applied as container hostname itself, causing internal DNS resolution of services to not work correctly.

@dolfinus
Copy link
Contributor

Sorry, I mean that you can use extra_pod_config to set hostname: "jupyter-{username}--{servername}" in pod's spec, without adding potentially breaking changes to the KubeSpawner

@manics
Copy link
Member

manics commented Jun 29, 2023

It sounds like extra_pod_config or modify_pod_hook can handle this, so should we close this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants