-
Notifications
You must be signed in to change notification settings - Fork 813
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
Change schema of hub.extraEnv to allow use of valueFrom
#597
Conversation
Note: if we need to maintain backwards-compatibility, I'd happily create a new config setting that can co-exist with the existing |
08aeb4b
to
ea89deb
Compare
Thanks for the patch, @mattjbray! I would like us to be backwards compatible - we've done this for |
ea89deb
to
7a254a2
Compare
@yuvipanda Good idea - I've updated the PR. |
Nice! Does it make sense to do the same for singleuser.extraEnv? Or does the fact that that gets passed through jupyterhub config make that not work? |
Thanks for the PR, @mattjbray! Congratulations on getting your first PR here merged! \o/ Hope to see more :) @minrk yeah, we need to fix that up on the kubespawner side I believe. |
@mattjbray can you also tell us a little more about the tracing you are doing? I'm very curious! |
@yuvipanda thanks! Sure, at the moment we're using DataDog, a proprietary service. This DataDog helm chart adds a DataDog provide automatic instrumentation for Python applications (e.g. the FROM jupyterhub/k8s-hub:4b122ad
USER root
# Install ddtrace and create a script that wraps jupyterhub with ddtrace-run.
RUN pip3 install 'ddtrace==0.11.0'
RUN mv /usr/local/bin/jupyterhub /usr/local/bin/jupyterhub-orig
COPY scripts/jupyterhub /usr/local/bin/jupyterhub
USER jovyan Where #!/bin/bash
# Wrap with datadog tracer
ddtrace-run /usr/local/bin/jupyterhub-orig "$@" Then in our jupyterhub:
hub:
extraEnv:
- name: "DATADOG_TRACE_AGENT_HOSTNAME"
valueFrom:
fieldRef:
fieldPath: status.hostIP
- name: "DATADOG_TRACE_AGENT_PORT"
value: "8126"
- name: "DATADOG_PATCH_MODULES"
value: "tornado:true,sqlite:true"
datadog:
datadog:
apmEnabled: true
daemonset:
useHostPort: true |
I thought I'd mention a use case for keeping the backwards compatibility in future too. If hub:
extraEnv:
OAUTH2_AUTHORIZE_URL: "https://login.elixir-czech.org/oidc/authorize"
OAUTH2_TOKEN_URL: "https://login.elixir-czech.org/oidc/token"
OAUTH_CALLBACK_URL: https://example.org/hub/oauth_callback
OAUTH_CLIENT_ID: xxx and set the private variables separately during deployment: |
Ah! @manics this was useful knowledge for me! |
- hub.extraEnv's default value now {} - see [this comment](jupyterhub#597 (comment)) by @manics. - prePuller.extraImages was referenced in the image-puller daemonsets, and now has an empty placeholder.
This was discussed back in #228, but was not done because of difficulties with
toYaml
. It looks likeindent
hadn't been discovered then.Motivation: I'm adding tracing to the
hub
component, and I need to usevalueFrom
to access the downward API.