Merge pull request #1472 from minrk/helm-templates

support extraFiles for binderhub pod

.github/workflows/diff.yml

@@ -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"

.github/workflows/test.yml

@@ -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 \
@@ -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

binderhub/tests/conftest.py

@@ -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"
-    )
     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):

binderhub/tests/test_main.py

@@ -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

conftest.py

@@ -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",
+    )

helm-chart/binderhub/files/binderhub_config.py

@@ -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)

helm-chart/binderhub/schema.yaml

@@ -32,6 +32,7 @@ required:
   - ingress
   - initContainers
   - lifecycle
+  - extraFiles
   - extraVolumes
   - extraVolumeMounts
   - extraEnv
@@ -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
@@ -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

helm-chart/binderhub/templates/deployment.yaml

@@ -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:
@@ -54,6 +67,7 @@ spec:
         hostPath:
           path: /var/run/docker.sock
       {{- end }}
+
       {{- with .Values.extraVolumes }}
       {{- . | toYaml | nindent 6 }}
       {{- end }}
@@ -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

helm-chart/binderhub/templates/secret.yaml

@@ -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

helm-chart/binderhub/values.yaml

@@ -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

testing/k8s-binder-k8s-hub/binderhub-chart-config-old.yaml

@@ -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

testing/k8s-binder-k8s-hub/binderhub-chart-config.yaml

@@ -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
@@ -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.