Draft work for using jsonnet to generate profile lists, taking advantage of the resource allocation script#8549
Draft work for using jsonnet to generate profile lists, taking advantage of the resource allocation script#8549GeorgianaElena wants to merge 4 commits into
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
|
This should be independent of jupyterhub/kubespawner#919! I'm excited this is being worked on. |
| - enc-common.secret.values.yaml | ||
| - staging.values.yaml | ||
| - enc-staging.secret.values.yaml | ||
| profiles_source_files: |
There was a problem hiding this comment.
Can we instead just rely on the fact that you can put regular jsonnet files under helm_chart_values_files already?
|
|
||
| local requestsChoices = profileList.makeChoiceDefault(r5xlarge_5 + r54xlarge_2, 'mem_2_gb'), | ||
|
|
||
| local defaultProfileImageChoices = { |
There was a problem hiding this comment.
I would also love for us to keep image lists in yaml, and only be using jsonnet for the resource allocation. I suppose that requires jupyterhub/kubespawner#919?
There was a problem hiding this comment.
@yuvipanda what's your incentive for preferring YAML? I agree that YAML is more readable than JSON, but the whitespace sensitivity is also a bit of a footgun. Jsonnet seems like a nice trade-off between the two, because it's more like JSON5.
My view is that trying to add dict-support to upstream charts is an uphill battle to work around the limitations of the YAML DSL. I'm in favour of acknowledging this by making Jsonnet more first class in our deployment workflow.
That's a bit more work, but fairly easy to move over to systematically. I'll expand in my top-level review comment.
6498e7e to
41988cb
Compare
|
@GeorgianaElena thank you for spiking this! My gut feeling is that we need the flexibility of Jsonnet — expressing things like "adapt these kinds of profiles to workshop packing vs regular node packing" etc. is much much easier (and more readable) in Jsonnet than YAML. I acknowledge that our third-party contributors are more familiar with YAML, but we don't have to give that up (see more later). I think I'd be interested in really pushing this further, and not using Helm as the config merging tool — what if instead of hubs:
- name: staging
display_name: Reflective Cloud - staging
domain: staging.reflective.2i2c.cloud
helm_chart: basehub
helm_chart_values_files:
- common.values.yaml
- staging.values.yaml
- enc-staging.secret.values.yaml
- name: prod
display_name: Reflective Cloud - prod
domain: reflective.2i2c.cloud
helm_chart: basehub
helm_chart_values_files:
- common.values.yaml
- prod.values.yaml
- enc-prod.secret.values.yamlwe just had hubs:
- name: staging
display_name: Reflective Cloud - staging
domain: staging.reflective.2i2c.cloud
helm_chart: basehub
values: staging.jsonnet
secret-values:
- enc-staging.secret.values.yaml
- name: prod
display_name: Reflective Cloud - prod
domain: reflective.2i2c.cloud
helm_chart: basehub
values: prod.jsonnet
secret-values:
- enc-prod.secret.values.yamli.e. we get rid of multiple values files and just use Jsonnet imports. I suspect that we'll want to make it easier for users to contribute their own profiles. We could either add documentation for how to do that with Jsonnet, or just write local images = std.parseYaml(importstr 'images.yaml'); |
Ref #7411
Feeback needed
Before building more logic around this, I want to get your input first on this. So, at the moment this PR presents the desired state of where a hub's profileList config will come from.
The general structure
basehubhelm chart we'll haveprofilejsonfiles generated with the resource allocation script, that the hubs will usecluster.yamlwill have a new field that we'll tell us where to get the profileList config fromQuestions