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

support extraFiles for binderhub pod #1472

Merged
merged 4 commits into from
Apr 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .github/workflows/diff.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,20 @@ jobs:

# NOTE: We change the directory so binderhub the chart name won't be
# misunderstood as the local folder name.
# validation is disabled, because the config is for a different version!
cd testing

old_config="../testing/k8s-binder-k8s-hub/binderhub-chart-config-old.yaml"
if [ -f "$old_config" ]; then
echo "using old config"
else
old_config="../testing/k8s-binder-k8s-hub/binderhub-chart-config.yaml"
fi

helm install binderhub-test binderhub \
--values ./k8s-binder-k8s-hub/binderhub-chart-config.yaml \
--values "$old_config" \
--repo https://jupyterhub.github.io/helm-chart/ \
--disable-openapi-validation \
--version=$UPGRADE_FROM_VERSION

- name: "Helm diff latest released dev chart with local chart"
Expand Down
13 changes: 11 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,19 @@ jobs:
#
# https://github.com/helm/helm/issues/9244
cd ci

old_config="../testing/k8s-binder-k8s-hub/binderhub-chart-config-old.yaml"
if [ -f "$old_config" ]; then
echo "using old config"
else
old_config="../testing/k8s-binder-k8s-hub/binderhub-chart-config.yaml"
fi

helm install binderhub-test \
--repo https://jupyterhub.github.io/helm-chart/ binderhub \
--disable-openapi-validation \
--version=$UPGRADE_FROM_VERSION \
--values ../testing/k8s-binder-k8s-hub/binderhub-chart-config.yaml \
--values "$old_config" \
--set config.BinderHub.hub_url=http://localhost:30902 \
--set config.BinderHub.hub_url_local=http://proxy-public \
--set config.GitHubRepoProvider.access_token=$GITHUB_ACCESS_TOKEN \
Expand Down Expand Up @@ -239,7 +248,7 @@ jobs:
if: matrix.test == 'helm'
run: |
export BINDER_URL=http://localhost:30901
pytest -m "remote" -v --maxfail=10 --cov binderhub --durations=10 --color=yes
pytest --helm -m "remote" -v --maxfail=10 --cov binderhub --durations=10 --color=yes

# GitHub Action reference: https://github.com/jupyterhub/action-k8s-namespace-report
- name: Kubernetes namespace report
Expand Down
14 changes: 11 additions & 3 deletions binderhub/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,23 @@ def pytest_configure(config):
config.addinivalue_line(
"markers", "auth: mark test to run only on auth environments"
)
config.addinivalue_line(
"markers", "remote: mark test to run only on remote environments"
)
consideRatio marked this conversation as resolved.
Show resolved Hide resolved
config.addinivalue_line(
"markers", "github_api: mark test to run only with GitHub API credentials"
)
config.addinivalue_line(
"markers", "remote: mark test for when BinderHub is already running somewhere."
)
config.addinivalue_line(
"markers",
"helm: mark test to only run when BinderHub is launched with our k8s-binderhub test config.",
)


def pytest_runtest_setup(item):
is_helm_test = any(mark for mark in item.iter_markers(name="helm"))
if not item.config.getoption("--helm"):
if is_helm_test:
pytest.skip("Skipping test marked as 'helm'")


def pytest_terminal_summary(terminalreporter, exitstatus):
Expand Down
9 changes: 9 additions & 0 deletions binderhub/tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,15 @@ async def test_main_page(app):
assert r.status_code == 200, f"{r.status_code} {url}"


@pytest.mark.remote
@pytest.mark.helm
async def test_custom_template(app):
"""Check that our custom template config is applied via the helm chart"""
r = await async_requests.get(app.url)
assert r.status_code == 200
assert "test-template" in r.text


@pytest.mark.remote
async def test_about_handler(app):
# Check that the about page loads
Expand Down
14 changes: 14 additions & 0 deletions conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
"""top-level pytest config

options can only be defined here,
not in binderhub/tests/conftest.py
"""


def pytest_addoption(parser):
parser.addoption(
"--helm",
action="store_true",
default=False,
help="Run tests marked with pytest.mark.helm",
)
3 changes: 3 additions & 0 deletions helm-chart/binderhub/files/binderhub_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ def get_value(key, default=None):
return value


# load custom templates, by default
c.BinderHub.template_path = "/etc/binderhub/templates"

# load config from values.yaml
for section, sub_cfg in get_value("config", {}).items():
c[section].update(sub_cfg)
Expand Down
34 changes: 33 additions & 1 deletion helm-chart/binderhub/schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ required:
- ingress
- initContainers
- lifecycle
- extraFiles
- extraVolumes
- extraVolumeMounts
- extraEnv
Expand Down Expand Up @@ -260,6 +261,37 @@ properties:
manifest as either the binder pod going into `Error` or `CrashLoopBackoff` states, or in
some special cases, the binder pod running but... just doing very random things. Be careful!

extraFiles:
type: object
additionalProperties: false
description: |
A dictionary with extra files to be injected into the binder pod's container
on startup. This can for example be used to inject: configuration
files, custom user interface templates, images, and more.

See zero-to-jupyterhub's extraFiles documentation for reference.
patternProperties:
".*":
type: object
additionalProperties: false
required: [mountPath]
oneOf:
- required: [data]
- required: [stringData]
- required: [binaryData]
properties:
mountPath:
type: string
data:
type: object
additionalProperties: true
stringData:
type: string
binaryData:
type: string
mode:
type: number

jupyterhub:
type: object
additionalProperties: true
Expand Down Expand Up @@ -479,7 +511,7 @@ properties:
description: |
The path type to use. The default value is 'Prefix'. Only applies on Kubernetes v1.18+.

See [the Kubernetes documentation](https://kubernetes.io/docs/concepts/services-networking/ingress/#path-types)
See [the Kubernetes documentation](https://kubernetes.io/docs/concepts/services-networking/ingress/#path-types)
for more details about path types.
tls:
type: array
Expand Down
22 changes: 22 additions & 0 deletions helm-chart/binderhub/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@ spec:
- name: config
secret:
secretName: binder-secret
{{- if .Values.extraFiles }}
- name: files
secret:
secretName: binder-secret
items:
{{- range $file_key, $file_details := .Values.extraFiles }}
- key: {{ $file_key | quote }}
path: {{ $file_key | quote }}
{{- with $file_details.mode }}
mode: {{ . }}
{{- end }}
{{- end }}
{{- end }}
{{- if .Values.config.BinderHub.use_registry }}
- name: docker-secret
secret:
Expand All @@ -54,6 +67,7 @@ spec:
hostPath:
path: /var/run/docker.sock
{{- end }}

{{- with .Values.extraVolumes }}
{{- . | toYaml | nindent 6 }}
{{- end }}
Expand All @@ -71,6 +85,14 @@ spec:
- mountPath: /etc/binderhub/config/
name: config
readOnly: true
{{- range $file_key, $file_details := .Values.extraFiles }}
- mountPath: {{ $file_details.mountPath }}
subPath: {{ $file_key | quote }}
{{- with $file_details.mode }}
mode: {{ . }}
{{- end }}
name: files
{{- end }}
{{- if .Values.config.BinderHub.use_registry }}
- mountPath: /root/.docker
name: docker-secret
Expand Down
9 changes: 9 additions & 0 deletions helm-chart/binderhub/templates/secret.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,15 @@ stringData:
{{- /* Glob files to allow them to be mounted by the binderhub pod */}}
{{- /* key=filename: value=content */}}
{{- (.Files.Glob "files/*").AsConfig | nindent 2 }}

{{- with include "jupyterhub.extraFiles.stringData" .Values.extraFiles }}
{{- . | nindent 2 }}
{{- end }}

{{- with include "jupyterhub.extraFiles.data" .Values.extraFiles }}
data:
{{- . | nindent 2 }}
{{- end }}
---
{{- if or .Values.config.BinderHub.use_registry .Values.config.BinderHub.buildDockerConfig }}
kind: Secret
Expand Down
2 changes: 2 additions & 0 deletions helm-chart/binderhub/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ config:

extraConfig: {}

extraFiles: {}

# Two bits of config need to be set to fully enable cors.
# config.BinderHub.cors_allow_origin controls the allowed origins for the
# binderhub api, and jupyterhub.hub.config.BinderSpawner.cors_allow_origin
Expand Down
55 changes: 55 additions & 0 deletions testing/k8s-binder-k8s-hub/binderhub-chart-config-old.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# (DELETE ME)
# this file should be removed immediately after merging PR #1472

service:
type: NodePort
nodePort: 30901

config:
BinderHub:
# Use the internal host name for Pod to Pod communication
# We can't use `hub_url` here because that is set to localhost which
# works on the host but not from within a Pod
hub_url_local: http://proxy-public
use_registry: false
log_level: 10
cors_allow_origin: "*"

ingress:
# Enabled to test the creation/update of the k8s Ingress resource, but not
# used actively in our CI system.
enabled: true

dind:
# Not enabled to test the creation/update of the image-cleaner DaemonSet
# resource because it also requires us to setup a container registry to test
# against which we haven't. We currently only test this via
# lint-and-validate-values.yaml that makes sure our rendered templates are
# valid against a k8s api-server.
enabled: false

# NOTE: This is a mirror of the jupyterhub section in
# jupyterhub-chart-config.yaml in testing/local-binder-k8s-hub, keep these
# two files synced please.
jupyterhub:
debug:
enabled: true

hub:
config:
BinderSpawner:
cors_allow_origin: "*"
db:
type: "sqlite-memory"

proxy:
service:
type: NodePort
nodePorts:
http: 30902

singleuser:
storage:
type: none
memory:
guarantee: null
14 changes: 14 additions & 0 deletions testing/k8s-binder-k8s-hub/binderhub-chart-config.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# This config is used when both BinderHub and the JupyterHub it uses are
# deployed to a kubernetes cluster.
# note: when changing the config schema,
# the old version of this file may need to be copied to ./binderhub-chart-config-old.yaml
# before updating, and then deleted in a subsequent PR.

service:
type: NodePort
nodePort: 30901
Expand All @@ -14,6 +18,16 @@ config:
log_level: 10
cors_allow_origin: "*"

extraFiles:
page.html:
mountPath: /etc/binderhub/templates/page.html
stringData: |
{% extends "templates/page.html" %}
{% block footer %}
{{ super() }}
<span>test-template</span>
{% endblock %}

ingress:
# Enabled to test the creation/update of the k8s Ingress resource, but not
# used actively in our CI system.
Expand Down