From 06e32740d7635a942384eb10805c880875f1ec21 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sun, 29 Oct 2023 14:34:30 +0100 Subject: [PATCH 1/3] basehub: make use of configurator opt-out This commit represents the simplest merge of a merge conflict. --- helm-charts/basehub/values.schema.yaml | 9 +++++ helm-charts/basehub/values.yaml | 46 +++++++++++++++++--------- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/helm-charts/basehub/values.schema.yaml b/helm-charts/basehub/values.schema.yaml index d7375f6bc0..cbf6f3ca1b 100644 --- a/helm-charts/basehub/values.schema.yaml +++ b/helm-charts/basehub/values.schema.yaml @@ -282,6 +282,7 @@ properties: - cloudResources - 2i2c - auth + - jupyterhubConfigurator properties: auth: type: object @@ -492,3 +493,11 @@ properties: type: array items: type: string + jupyterhubConfigurator: + type: object + additionalProperties: false + required: + - enabled + properties: + enabled: + type: boolean diff --git a/helm-charts/basehub/values.yaml b/helm-charts/basehub/values.yaml index 231593447e..c48ea1aace 100644 --- a/helm-charts/basehub/values.yaml +++ b/helm-charts/basehub/values.yaml @@ -109,6 +109,8 @@ jupyterhub: gitRepoUrl: "https://github.com/2i2c-org/default-hub-homepage" # TODO: make main the default branch in the repo above gitRepoBranch: "master" + jupyterhubConfigurator: + enabled: true ingress: enabled: true ingressClassName: nginx @@ -554,15 +556,6 @@ jupyterhub: name: custom-templates subPath: repo/extra-assets services: - configurator: - url: http://configurator:10101 - # Don't require users to explicitly 'confirm' before sending them to configurator - oauth_no_confirm: true - command: - - python3 - - -m - - jupyterhub_configurator.app - - --Configurator.config_file=/usr/local/etc/jupyterhub-configurator/jupyterhub_configurator_config.py # hub-health service helps us run health checks from the deployer script. # The JupyterHub Helm chart will automatically generate an API token for # services and expose it in a k8s Secret named `hub`. When we run health @@ -605,7 +598,7 @@ jupyterhub: # jupyterhub-configurator isn't needed, as the request is directly to # 127.0.0.1:10101. # - # ref: The extraConfig.02-custom-admin section below + # ref: The extraConfig.02-basehub-spawner section below # ref: https://github.com/yuvipanda/jupyterhub-configurator/blob/996405d2a7017153d5abe592b8028fed7a1801bb/jupyterhub_configurator/mixins.py#L7C5-L11 # - from: @@ -638,9 +631,13 @@ jupyterhub: c.JupyterHub.template_vars.update({ 'custom': get_config('custom.homepage.templateVars') }) - 02-custom-admin: | - # adjusts the spawner to add config specific to admin users - + 02-basehub-spawner: | + # Updates JupyterHub.spawner_class to handle features introduced by the + # basehub chart, specifically those configured via: + # + # jupyterhub.custom.singleuserAdmin + # jupyterhub.custom.jupyterhubConfigurator + # from z2jh import get_config from kubespawner.utils import get_k8s_model from kubespawner import KubeSpawner @@ -673,7 +670,26 @@ jupyterhub: return pod c.KubeSpawner.modify_pod_hook = modify_pod_hook - class CustomSpawner(ConfiguratorSpawnerMixin, KubeSpawner): + spawner_base_classes = [KubeSpawner] + + # Setup jupyterhub-configurator only if needed + if get_config("custom.jupyterhubConfigurator.enabled"): + spawner_base_classes = [ConfiguratorSpawnerMixin, KubeSpawner] + + jhc_service = { + "name": "configurator", + "url": "http://configurator:10101", + "oauth_no_confirm": True, + "command": [ + "python3", + "-m", + "jupyterhub_configurator.app", + "--Configurator.config_file=/usr/local/etc/jupyterhub-configurator/jupyterhub_configurator_config.py", + ], + } + c.JupyterHub.services.append(jhc_service) + + class BaseHubSpawner(*spawner_base_classes): def start(self, *args, **kwargs): custom_admin = get_config('custom.singleuserAdmin', {}) if not (self.user.admin and custom_admin): @@ -689,7 +705,7 @@ jupyterhub: return super().start(*args, **kwargs) - c.JupyterHub.spawner_class = CustomSpawner + c.JupyterHub.spawner_class = BaseHubSpawner 03-cloud-storage-bucket: | from z2jh import get_config cloud_resources = get_config('custom.cloudResources') From 2f001e52e3319cbcfd6d06a2f663f9ee6721e028 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Fri, 3 Nov 2023 15:11:37 +0100 Subject: [PATCH 2/3] Add comments and smaller refactors --- helm-charts/basehub/values.yaml | 39 ++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/helm-charts/basehub/values.yaml b/helm-charts/basehub/values.yaml index c48ea1aace..762f0127d6 100644 --- a/helm-charts/basehub/values.yaml +++ b/helm-charts/basehub/values.yaml @@ -632,26 +632,29 @@ jupyterhub: 'custom': get_config('custom.homepage.templateVars') }) 02-basehub-spawner: | - # Updates JupyterHub.spawner_class to handle features introduced by the - # basehub chart, specifically those configured via: + # Updates JupyterHub.spawner_class and KubeSpawner.modify_pod_hook to + # handle features introduced by the basehub chart, specifically those + # configured via: # # jupyterhub.custom.singleuserAdmin # jupyterhub.custom.jupyterhubConfigurator # - from z2jh import get_config - from kubespawner.utils import get_k8s_model - from kubespawner import KubeSpawner from jupyterhub_configurator.mixins import ConfiguratorSpawnerMixin from kubernetes_asyncio.client.models import V1VolumeMount + from kubespawner import KubeSpawner + from kubespawner.utils import get_k8s_model + from z2jh import get_config def modify_pod_hook(spawner, pod): """ - Modify the pod manifest for admins. + Modify admin user's pod manifests based on *list* config under + `jupyterhub.custom.singleuserAdmin`. - This is used to modify attributes of the pod that are *lists*, - as those are not merged but replaced by KubeSpawner. Everything - else is managed via the CustomSpawner's start method below. + This hook is required to ensures that list config under + `jupyterhub.custom.singleuserAdmin` are appended and not just + overridden when a profile_list entry has a kubespawner_override + modifying the same config. """ custom_admin = get_config('custom.singleuserAdmin', {}) if not (spawner.user.admin and custom_admin): @@ -659,8 +662,8 @@ jupyterhub: for c in pod.spec.containers: if c.name == "notebook": - notebook_container = c - break + notebook_container = c + break else: raise Exception("No container named 'notebook' found in pod definition") @@ -670,9 +673,9 @@ jupyterhub: return pod c.KubeSpawner.modify_pod_hook = modify_pod_hook - spawner_base_classes = [KubeSpawner] - # Setup jupyterhub-configurator only if needed + # Setup jupyterhub-configurator only if its enabled + spawner_base_classes = [KubeSpawner] if get_config("custom.jupyterhubConfigurator.enabled"): spawner_base_classes = [ConfiguratorSpawnerMixin, KubeSpawner] @@ -689,8 +692,16 @@ jupyterhub: } c.JupyterHub.services.append(jhc_service) + class BaseHubSpawner(*spawner_base_classes): def start(self, *args, **kwargs): + """ + Modify admin users' spawners' non-list config based on + `jupyterhub.custom.singleuserAdmin`. + + The list config is handled separately in by the + `modify_pod_hook`. + """ custom_admin = get_config('custom.singleuserAdmin', {}) if not (self.user.admin and custom_admin): return super().start(*args, **kwargs) @@ -703,8 +714,6 @@ jupyterhub: self.service_account = admin_service_account return super().start(*args, **kwargs) - - c.JupyterHub.spawner_class = BaseHubSpawner 03-cloud-storage-bucket: | from z2jh import get_config From dc6ffc17710337080b7bb89bac9a9aad44c062b0 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Fri, 3 Nov 2023 15:12:27 +0100 Subject: [PATCH 3/3] Relocate modify_pod_hook to bottom --- helm-charts/basehub/values.yaml | 56 ++++++++++++++++----------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/helm-charts/basehub/values.yaml b/helm-charts/basehub/values.yaml index 762f0127d6..c83cefff18 100644 --- a/helm-charts/basehub/values.yaml +++ b/helm-charts/basehub/values.yaml @@ -646,34 +646,6 @@ jupyterhub: from z2jh import get_config - def modify_pod_hook(spawner, pod): - """ - Modify admin user's pod manifests based on *list* config under - `jupyterhub.custom.singleuserAdmin`. - - This hook is required to ensures that list config under - `jupyterhub.custom.singleuserAdmin` are appended and not just - overridden when a profile_list entry has a kubespawner_override - modifying the same config. - """ - custom_admin = get_config('custom.singleuserAdmin', {}) - if not (spawner.user.admin and custom_admin): - return pod - - for c in pod.spec.containers: - if c.name == "notebook": - notebook_container = c - break - else: - raise Exception("No container named 'notebook' found in pod definition") - - admin_volume_mounts = custom_admin.get('extraVolumeMounts', []) - notebook_container.volume_mounts += [get_k8s_model(V1VolumeMount, obj) for obj in (admin_volume_mounts)] - - return pod - c.KubeSpawner.modify_pod_hook = modify_pod_hook - - # Setup jupyterhub-configurator only if its enabled spawner_base_classes = [KubeSpawner] if get_config("custom.jupyterhubConfigurator.enabled"): @@ -715,6 +687,34 @@ jupyterhub: return super().start(*args, **kwargs) c.JupyterHub.spawner_class = BaseHubSpawner + + + def modify_pod_hook(spawner, pod): + """ + Modify admin user's pod manifests based on *list* config under + `jupyterhub.custom.singleuserAdmin`. + + This hook is required to ensures that list config under + `jupyterhub.custom.singleuserAdmin` are appended and not just + overridden when a profile_list entry has a kubespawner_override + modifying the same config. + """ + custom_admin = get_config('custom.singleuserAdmin', {}) + if not (spawner.user.admin and custom_admin): + return pod + + for c in pod.spec.containers: + if c.name == "notebook": + notebook_container = c + break + else: + raise Exception("No container named 'notebook' found in pod definition") + + admin_volume_mounts = custom_admin.get('extraVolumeMounts', []) + notebook_container.volume_mounts += [get_k8s_model(V1VolumeMount, obj) for obj in (admin_volume_mounts)] + + return pod + c.KubeSpawner.modify_pod_hook = modify_pod_hook 03-cloud-storage-bucket: | from z2jh import get_config cloud_resources = get_config('custom.cloudResources')