Merge pull request #1923 from consideRatio/pr/fullname-override

Support fullnameOverride / nameOverride and reference resources by named templates

.github/workflows/test-chart.yaml

@@ -148,19 +148,45 @@ jobs:
         run: |
           . ./ci/common
           UPGRADE_FROM_VERSION=$(curl -sS https://jupyterhub.github.io/helm-chart/info.json | jq -er '.jupyterhub.${{ matrix.upgrade-from }}')
+          echo "UPGRADE_FROM_VERSION=$UPGRADE_FROM_VERSION" >> $GITHUB_ENV
 
           echo ""
           echo "Installing already released jupyterhub version $UPGRADE_FROM_VERSION"
-          helm install jupyterhub --repo https://jupyterhub.github.io/helm-chart/ jupyterhub --values dev-config.yaml --version=$UPGRADE_FROM_VERSION
+
+          # FIXME: We change the directory so jupyterhub the chart name won't be
+          #        misunderstood as the local folder name.
+          #
+          #        https://github.com/helm/helm/issues/9244
+          cd ci
+          helm install jupyterhub --repo https://jupyterhub.github.io/helm-chart/ jupyterhub --values ../dev-config.yaml --version=$UPGRADE_FROM_VERSION
 
           echo ""
           echo "Installing Helm diff plugin while k8s resources are initializing"
           helm plugin install https://github.com/databus23/helm-diff
 
-      - name: "Helm diff ${{ matrix.upgrade-from }} chart with current chart"
+      # ref: https://github.com/jacobtomlinson/gha-read-helm-chart
+      - name: Load local Chart.yaml
+        id: local-chart
+        uses: jacobtomlinson/gha-read-helm-chart@0.1.3
+        with:
+          path: jupyterhub
+
+      - name: "Helm diff ${{ matrix.upgrade-from }} chart with local chart"
         if: matrix.test == 'upgrade'
         run: |
-          helm diff upgrade --install jupyterhub ./jupyterhub --values dev-config.yaml
+          export STRING_REPLACER_A=${{ steps.local-chart.outputs.version }}
+          export STRING_REPLACER_B=$UPGRADE_FROM_VERSION
+
+          echo "NOTE: For the helm diff only, we have replaced the new chart"
+          echo "      version with the old chart version to reduce clutter."
+          echo
+          echo "      Old version: $UPGRADE_FROM_VERSION"
+          echo "      New version: ${{ steps.local-chart.outputs.version }} (replaced)"
+          echo
+
+          helm diff upgrade --install jupyterhub ./jupyterhub --values dev-config.yaml \
+              --context=3 \
+              --post-renderer=ci/string-replacer.sh
 
       - name: "Await ${{ matrix.upgrade-from }} chart"
         if: matrix.test == 'upgrade'
@@ -170,7 +196,7 @@ jobs:
           await_autohttps_tls_cert_acquisition
           await_autohttps_tls_cert_save
 
-      - name: "Install or upgrade to current chart"
+      - name: "Install or upgrade to local chart"
         run: |
           . ./ci/common
           helm upgrade --install jupyterhub ./jupyterhub --values dev-config.yaml

ci/string-replacer.sh

@@ -0,0 +1,12 @@
+#!/bin/bash
+#
+# In .github/workflows/test-chart.yaml, we test upgrading one chart version to
+# another. After having installed the first version we run "helm diff" with the
+# new version.
+#
+# This script created to be referenced by helm's --post-renderer flag to replace
+# strings in the rendered templates into something that doesn't change.
+#
+
+set -eu
+sed -e "s|$STRING_REPLACER_A|$STRING_REPLACER_B|" < /dev/stdin

jupyterhub/files/hub/jupyterhub_config.py

@@ -12,7 +12,7 @@
 configuration_directory = os.path.dirname(os.path.realpath(__file__))
 sys.path.insert(0, configuration_directory)
 
-from z2jh import get_config, set_config_if_not_none
+from z2jh import get_config, set_config_if_not_none, get_name, get_name_env
 
 
 def camelCaseify(s):
@@ -35,7 +35,7 @@ def camelCaseify(s):
 # Connect to a proxy running in a different pod. Note that *_SERVICE_*
 # environment variables are set by Kubernetes for Services
 c.ConfigurableHTTPProxy.api_url = (
-    f"http://proxy-api:{os.environ['PROXY_API_SERVICE_PORT']}"
+    f'http://{get_name("proxy-api")}:{get_name_env("proxy-api", "_SERVICE_PORT")}'
 )
 c.ConfigurableHTTPProxy.should_start = False
 
@@ -92,7 +92,9 @@ def camelCaseify(s):
 # hub_connect_url is the URL for connecting to the hub for use by external
 # JupyterHub services such as the proxy. Note that *_SERVICE_* environment
 # variables are set by Kubernetes for Services.
-c.JupyterHub.hub_connect_url = f"http://hub:{os.environ['HUB_SERVICE_PORT']}"
+c.JupyterHub.hub_connect_url = (
+    f'http://{get_name("hub")}:{get_name_env("hub", "_SERVICE_PORT")}'
+)
 
 # implement common labels
 # this duplicates the jupyterhub.commonLabels helper
@@ -174,7 +176,7 @@ def camelCaseify(s):
 if get_config("imagePullSecret.automaticReferenceInjection") and (
     get_config("imagePullSecret.create") or get_config("imagePullSecret.enabled")
 ):
-    image_pull_secrets.append("image-pull-secret")
+    image_pull_secrets.append(get_name("image-pull-secret"))
 if get_config("imagePullSecrets"):
     image_pull_secrets.extend(get_config("imagePullSecrets"))
 if get_config("singleuser.image.pullSecrets"):
@@ -184,11 +186,9 @@ def camelCaseify(s):
 
 # scheduling:
 if get_config("scheduling.userScheduler.enabled"):
-    c.KubeSpawner.scheduler_name = os.environ["HELM_RELEASE_NAME"] + "-user-scheduler"
+    c.KubeSpawner.scheduler_name = get_name("user-scheduler")
 if get_config("scheduling.podPriority.enabled"):
-    c.KubeSpawner.priority_class_name = (
-        os.environ["HELM_RELEASE_NAME"] + "-default-priority"
-    )
+    c.KubeSpawner.priority_class_name = get_name("priority")
 
 # add node-purpose affinity
 match_node_purpose = get_config("scheduling.userPods.nodeAffinity.matchNodePurpose")

jupyterhub/files/hub/z2jh.py

@@ -9,25 +9,46 @@
 
 import yaml
 
-
 # memoize so we only load config once
 @lru_cache()
 def _load_config():
-    """Load configuration from disk
+    """Load the Helm chart configuration used to render the Helm templates of
+    the chart from a mounted k8s Secret."""
 
-    Memoized to only load once
-    """
-    cfg = {}
-    for source in ("config", "secret"):
-        path = f"/etc/jupyterhub/{source}/values.yaml"
-        if os.path.exists(path):
-            print(f"Loading {path}")
-            with open(path) as f:
-                values = yaml.safe_load(f)
-            cfg = _merge_dictionaries(cfg, values)
-        else:
-            print(f"No config at {path}")
-    return cfg
+    path = f"/etc/jupyterhub/secret/values.yaml"
+    if os.path.exists(path):
+        print(f"Loading {path}")
+        with open(path) as f:
+            return yaml.safe_load(f)
+    else:
+        raise Exception(f"{path} not found!")
+
+
+@lru_cache()
+def _get_config_value(key):
+    """Load value from the k8s ConfigMap given a key."""
+
+    path = f"/etc/jupyterhub/config/{key}"
+    if os.path.exists(path):
+        with open(path) as f:
+            return f.read()
+    else:
+        raise Exception(f"{path} not found!")
+
+
+def get_name(name):
+    """Returns the fullname of a resource given its short name"""
+    return _get_config_value(name)
+
+
+def get_name_env(name, suffix=""):
+    """Returns the fullname of a resource given its short name along with a
+    suffix, converted to uppercase with dashes replaced with underscores. This
+    is useful to reference named services associated environment variables, such
+    as PROXY_PUBLIC_SERVICE_PORT."""
+    env_key = _get_config_value(name) + suffix
+    env_key = env_key.upper().replace("-", "_")
+    return os.environ[env_key]
 
 
 def _merge_dictionaries(a, b):

jupyterhub/schema.yaml

@@ -1,6 +1,55 @@
 title: Config
 type: object
 properties:
+  fullnameOverride:
+    type: string
+    description: |
+      fullnameOverride and nameOverride allow you to adjust how the resources
+      part of the Helm chart are named.
+
+      Name format               | Resource types | fullnameOverride | nameOverride | Note
+      -                         | -              | -                | -            | -
+      component                 | namespaced     | `""`             | *            | Default
+      release-component         | cluster wide   | `""`             | *            | Default
+      fullname-component        | *              | str              | *            | -
+      release-component         | *              | null             | `""`         | -
+      release-(name-)component  | *              | null             | str          | omitted if contained in release
+      release-(chart-)component | *              | null             | null         | omitted if contained in release
+
+      ```{admonition} Warning!
+      :class: warning
+      Changing fullnameOverride or nameOverride after the initial installation
+      of the chart isn't supported. Changing their values likely leads to a
+      reset of non-external JupyterHub databases, abandonment of users' storage,
+      and severed couplings to currently running user pods.
+      ```
+
+      If you are a developer of a chart depending on this chart, you should
+      avoid hardcoding names. If you want to reference the name of a resource in
+      this chart from a parent helm chart's template, you can make use of the
+      global named templates instead.
+      
+      ```yaml
+      # some pod definition of a parent chart helm template
+      schedulerName: {{ include "jupyterhub.user-scheduler.fullname" . }}
+      ```
+
+      To access them from a container, you can also rely on the hub ConfigMap
+      that contains entries of all the resource names.
+
+      ```yaml
+      # some container definition in a parent chart helm template
+      env:
+        - name: SCHEDULER_NAME
+          valueFrom:
+            configMapKeyRef:
+              name: {{ include "jupyterhub.user-scheduler.fullname" . }}
+              key: user-scheduler
+      ```
+  nameOverride:
+    type: string
+    description: |
+      See the documentation under [`fullnameOverride`](schema_fullnameOverride).
   imagePullSecret:
     type: object
     description: |
@@ -663,21 +712,22 @@ properties:
         type:
           - string
         description: |
-          Name of the existing secret in the kubernetes cluster, typically the `hub-secret`.
+          Name of an existing k8s Secret to use instead of the chart managed k8s
+          Secret.
 
-          This secret should represent the structure as otherwise generated by this chart:
-            ```yaml
-            apiVersion: v1
-            data:
-              proxy.token: < FILL IN >
-              values.yaml: < FILL IN >
-            kind: Secret
-            metadata:
-              name: hub-secret
-            ```
+          This k8s Secret must represent the structure generated by this chart
+          and by using this option, you are in change of ensuring the secret
+          structure is reflected when upgrading to new versions of the chart.
 
-          NOTE: if you choose to manage the secret yourself, you are in charge of ensuring the
-          secret having the proper contents.
+          ```yaml
+          apiVersion: v1
+          data:
+            proxy.token: < FILL IN >
+            values.yaml: < FILL IN >
+          kind: Secret
+          metadata:
+            name: my-self-managed-secret
+          ```
       nodeSelector: &nodeSelector-spec
         type:
           - object