support extraFiles for binderhub pod

[minrk]

imports extraFiles helpers, spec from JupyterHub, applied to the binderhub pod

Possibly more general solution to #1471

[minrk]

Hm, we have a problem in CI because it's now impossible to update the helm schema and pass tests, because the same config is passed both to the current and previous version of binderhub.

If the schema weren't so strict (i.e. rejecting addtionalProperties), this would be less of an issue.

[consideRatio]

This seems like [hub|singleuser].extraFiles but specialized for templates. Should we go for the even more general approach of extraFiles instead of this?

I'm probably not willing to invest my own time into this effort at the moment so please go ahead with this template specific approach if you think it makes sense. I know for z2jh it was just one use case to mount templates while mounting configuration files etc would also be relevant, due to that we developed this more general support both for the hub pod and user pods. Overall, this feature seems to have been a great success.

• Z2JH configuration reference
• Z2JH code implementation
  • extraFiles helpers defined. These can perhaps also be reused directly from this helm chart as it depends on the jupyterhub Helm chart.
  • extraFiles helpers referenced from a k8s Secret

[minrk]

This seems like [hub|singleuser].extraFiles but specialized for templates

It is! I actually used your extraFiles implementation as reference. JupyterHub is much more infinitely extensible, so there are more general files that make sense to drop-in, especially files we as chart maintainers don't need to know about. I'm not sure I see a similar arbitrary files use case in BinderHub, so I thought this would be simpler, both to implement (the whole thing is smaller than just the helpers for extraFiles) and use (deployments don't need to know where we put templates). On the other hand, I can imagine expert JupyterHub deployers (and maintainers) liking the familiarity of extraFiles consistently applied everywhere.

[minrk]

This now imports extraFiles exactly as-is (thanks for the links @consideRatio), so the feature is consistent across jupyterhub/binderhub.

[minrk]

FWIW, I couldn't figure out how to add a simple test that would only run on the full k8s case to verify that the custom template has its effect. I did verify it with a manual deploy (haven't yet with extraFiles, will do that tomorrow).

[consideRatio]

FWIW, I couldn't figure out how to add a simple test that would only run on the full k8s case to verify that the custom template has its effect. I did verify it with a manual deploy (haven't yet with extraFiles, will do that tomorrow).

I've also failed to make a simple test, but here is an overview of what I did in z2jh in case you wish to implement a test like that.

• extraFiles chart config passed during tests
• extra_files_test_command - runs inside a container to detect mounted extraFiles string literal looking for mounted test data.
• pytest test running the test command above in a pod

Hm, we have a problem in CI because it's now impossible to update the helm schema and pass tests, because the same config is passed both to the current and previous version of binderhub.

I think this is inevitable to avoid issues during upgrade tests without having separate configurations. This is how I've resolved it in z2jh:

1. dev-config.yaml is meant for config that works in both latest stable release and current release.
2. dev-config-local-chart-extra-config.yaml is meant to adjust the dev-config.yaml with additions that is only valid in the latest version.

The GitHub Workflow testing the charts always run a step referencing dev-config.yaml and dev-config-local-chart-extra-config.yaml but sometimes also runs a step installing the stable release before this using only dev-config.yaml.

If the schema weren't so strict (i.e. rejecting addtionalProperties), this would be less of an issue.

Its strictness can be troublesome here, but is one of the key values of the schema I think. Rejecting unrecognized property names has helped a lot of users catch invalid configuration I believe, and also reduced the maintenance by fewer bugs being reported by invalid configuration caused by typos.

[minrk]

I went with a simple test under pytest.mark.remote. It's not 100% correct semantically (remote doesn't mean it's our helm test config), but it is correct practically (those are the tests we run against this config).

[minrk]

Looks like I wasn't quite right about the condition, trying a fix

[minrk]

New test (new helm mark) now runs only when --helm is passed, verifying that extraFiles is working as intended.