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

Allow setting extraContainers, extraVolumes, etc as dicts #468

Open
yuvipanda opened this issue Dec 14, 2020 · 2 comments
Open

Allow setting extraContainers, extraVolumes, etc as dicts #468

yuvipanda opened this issue Dec 14, 2020 · 2 comments

Comments

@yuvipanda
Copy link
Collaborator

Proposed change

Right now, extraContainers, extraVolumes, extraVolumeMounts, etc are lists. This matches what the k8s API needs. However, this makes overriding them in helm config very difficult, as there's no way to spend to a list or modify individual elements.

For example, if you have this config in your generic-to-all-hubs helm config:

hub:
  initContainers:
      - name: templates-clone
        image: alpine/git

You can't actually add another container to this in an override without copying the original. You'll have to do something like:

hub:
  initContainers:
      - name: templates-clone
        image: alpine/git
      - name: something-else
        image: alpine

This can get out of sync quite quickly.

In addition to treating extraContainers, etc as lists, we can also treat them as dicts, with name being the key. This lets helm overrides work as they should. We have done this in the past to extraConfig and friends in zero to jupyterhub, so doing this here would be valuble.

Alternative options

Do this in zero to jupyterhub, but that's probably the wrong place for this.

Who would use this feature?

Anyone doing a deployment of many hubs that are similar to each other.

yuvipanda added a commit to yuvipanda/datahub-old-fork that referenced this issue Dec 14, 2020
We override `hub.extraContainers` in `secrets/staging.yaml`
and `secrets/prod.yaml` to include the fetch-course-emails
container along the hub. But this overrides what's in
`config/common.yaml` destructively, since it just replaces
the list rather than add to it.

I've filed jupyterhub/kubespawner#468
to fix this. Until then, I've manually moved the extraContainers
config to the secret files.
@manics
Copy link
Member

manics commented Dec 16, 2020

Do you think we should modify just these three properties, or should we do this for all List properties that aren't strings in one go?

@yuvipanda
Copy link
Collaborator Author

@manics ideally for all List properties, but we need to identify what the key should be for each of them. Makes it easy for containers, volumes, etc. The only other thing I can think of is env vars, but those are already settable as lists or dicts.

yuvipanda added a commit to yuvipanda/datahub-old-fork that referenced this issue Jan 4, 2021
I wish this could be done centrally!
jupyterhub/kubespawner#468 should
help.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants