Page MenuHomeElementl

[helm] Allow creation of workspace from dagit values
ClosedPublic

Authored by rexledesma on Mar 31 2021, 11:51 PM.

Details

Summary

Depends on D7252. Resolves https://github.com/dagster-io/dagster/issues/3946.

If the user wants to manage their user deployments in a separate helm release, they
still have to specify the user deployments in the umbrella dagster chart to generate
the workspace.yaml.

Although the workspace only needs the name/port of the deployment server, they'll still
have to specify the image and dagsterApiGrpcArgs for their user deployment since the
chart relies on the subchart schema for validation.

To improve ergonomics, we expose another field under dagit for them to specify the workspace.yaml
values without the unnecessary fields.

Test Plan

pytest

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 1 2021, 12:10 AM
Harbormaster failed remote builds in B28323: Diff 34747!
johann requested changes to this revision.Apr 1 2021, 3:04 PM

I think the field would be more natural under userDeployments than dagit

helm/dagster/schema/schema/charts/dagster/subschema/dagit.py
9

should this be host?

E.g. if the servers are in another namespace, the name will be foo but host will be foo.namespace

helm/dagster/values.yaml
42

"Since the servers will be deployed in another chart, this chart just needs the addresses of the servers"

Also should show an example server schema

This revision now requires changes to proceed.Apr 1 2021, 3:04 PM

@johann I'm cautious around putting this under dagster-user-deployments since that would generate a manifest used by the parent chart, using values from the subchart. That's a bit confusing. We already do that with dagster-user-deployments.enabled, which is not good.

Good point about the subchart. I don't love putting it under dagit- makes sense to us, but I don't think they seem related for users. Breaking change, but what if the subchart was under an additional yaml key? So that additional values related to user deployments could be under the userDeployments key?

helm/dagster/values.yaml
42

Comment could still be clearer- I think something like the above would help clarify what's going on

clarifying comment on why dagit needs a workspace

I think from the introductory line in https://docs.dagster.io/concepts/repositories-workspaces/workspaces#workspaces, it should make sense that workspace lives under dagit.

This revision is now accepted and ready to land.Apr 1 2021, 8:23 PM