feat(helm): support custom configmap for workspace (#7882)

* feat(helm): support custom configmap for workspace * Add workspace render test * Review fixes Co-authored-by: peay <peay@protonmail.com>
dagster-io · May 16, 2022 · ea27cdc · ea27cdc
1 parent 91b6c0f
commit ea27cdc
Show file tree

Hide file tree

Showing 9 changed files with 117 additions and 10 deletions.
diff --git a/helm/dagster/schema/schema/charts/dagster/subschema/dagit.py b/helm/dagster/schema/schema/charts/dagster/subschema/dagit.py
@@ -15,6 +15,7 @@ class Server(BaseModel):
 class Workspace(BaseModel):
     enabled: bool
     servers: List[Server]
+    externalConfigmap: Optional[str]
 
 
 class Dagit(BaseModel):

diff --git a/helm/dagster/schema/schema_tests/test_dagit.py b/helm/dagster/schema/schema_tests/test_dagit.py
@@ -1,6 +1,6 @@
 import pytest
 from kubernetes.client import models
-from schema.charts.dagster.subschema.dagit import Dagit
+from schema.charts.dagster.subschema.dagit import Dagit, Workspace
 from schema.charts.dagster.values import DagsterHelmValues
 from schema.charts.utils import kubernetes
 from schema.utils.helm_template import HelmTemplate
@@ -221,3 +221,20 @@ def test_dagit_labels(deployment_template: HelmTemplate):
 
     assert set(deployment_labels.items()).issubset(dagit_deployment.metadata.labels.items())
     assert set(pod_labels.items()).issubset(dagit_deployment.spec.template.metadata.labels.items())
+
+
+def test_dagit_workspace_external_configmap(deployment_template: HelmTemplate):
+    helm_values = DagsterHelmValues.construct(
+        dagit=Dagit.construct(
+            workspace=Workspace(
+                enabled=True,
+                servers=[],
+                externalConfigmap="test-external-workspace",
+            )
+        ),
+    )
+
+    [dagit_deployment] = deployment_template.render(helm_values)
+    assert (
+        dagit_deployment.spec.template.spec.volumes[1].config_map.name == "test-external-workspace"
+    )
diff --git a/helm/dagster/schema/schema_tests/test_workspace.py b/helm/dagster/schema/schema_tests/test_workspace.py
@@ -179,3 +179,52 @@ def test_workspace_renders_empty(template: HelmTemplate):
     grpc_servers = workspace["load_from"]
 
     assert len(grpc_servers) == len(servers)
+
+
+def test_workspace_external_configmap_fail(template: HelmTemplate, capfd):
+    helm_values = DagsterHelmValues.construct(
+        dagit=Dagit.construct(
+            workspace=Workspace(
+                enabled=True,
+                servers=[
+                    Server(host="another-deployment-one", port=4000),
+                ],
+                externalConfigmap="test",
+            )
+        ),
+        dagsterUserDeployments=UserDeployments(
+            enabled=True,
+            enableSubchart=True,
+            deployments=[create_simple_user_deployment("deployment-one")],
+        ),
+    )
+
+    with pytest.raises(subprocess.CalledProcessError):
+        template.render(helm_values)
+
+    _, err = capfd.readouterr()
+    assert "workspace.servers and workspace.externalConfigmap cannot both be set." in err
+
+
+def test_workspace_external_configmap_not_present(template: HelmTemplate, capfd):
+    helm_values = DagsterHelmValues.construct(
+        dagit=Dagit.construct(
+            workspace=Workspace(
+                enabled=True,
+                servers=[],
+                externalConfigmap="test",
+            )
+        ),
+        dagsterUserDeployments=UserDeployments(
+            enabled=True,
+            enableSubchart=False,
+            deployments=[],
+        ),
+    )
+
+    with pytest.raises(subprocess.CalledProcessError):
+        template.render(helm_values)
+
+    _, err = capfd.readouterr()
+    # workspace.yaml isn't rendered if using an externalConfigmap
+    assert "Error: could not find template" in err
diff --git a/helm/dagster/templates/configmap-workspace.yaml b/helm/dagster/templates/configmap-workspace.yaml
@@ -5,18 +5,27 @@
 {{- end }}
 
 {{- if $userDeployments.enabled }}
+
+{{- $dagitWorkspace := .Values.dagit.workspace }}
+{{- $workspaceUseFixedServers := and $dagitWorkspace.enabled $dagitWorkspace.servers }}
+{{- $workspaceUseExternalConfigmap := and $dagitWorkspace.enabled $dagitWorkspace.externalConfigmap }}
+
+{{- if and $workspaceUseFixedServers $workspaceUseExternalConfigmap }}
+{{ fail "workspace.servers and workspace.externalConfigmap cannot both be set." }}
+{{- end }}
+
+{{- if not $workspaceUseExternalConfigmap }}
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  name: {{ template "dagster.fullname" . }}-workspace-yaml
+  name: {{ include "dagit.workspace.configmapName" . }}
   labels:
     app: {{ template "dagster.name" . }}
     chart: {{ template "dagster.chart" . }}
     release: {{ .Release.Name }}
     heritage: {{ .Release.Service }}
 data:
   workspace.yaml: |
-    {{- $dagitWorkspace := .Values.dagit.workspace }}
     {{- $deployments := ternary $dagitWorkspace.servers $userDeployments.deployments $dagitWorkspace.enabled }}
     {{- if $deployments }}
     load_from:
@@ -35,3 +44,5 @@ data:
     load_from: []
     {{- end }}
 {{- end }}
+
+{{- end }}
diff --git a/helm/dagster/templates/deployment-daemon.yaml b/helm/dagster/templates/deployment-daemon.yaml
@@ -98,9 +98,9 @@ spec:
               mountPath: "{{ $.Values.global.dagsterHome }}/dagster.yaml"
               subPath: dagster.yaml
             {{- if $userDeployments.enabled }}
+            # Do not use `subPath` to allow the configmap to update if modified
             - name: dagster-workspace-yaml
-              mountPath: "/dagster-workspace/workspace.yaml"
-              subPath: workspace.yaml
+              mountPath: "/dagster-workspace/"
             {{- end }}
           resources:
             {{- toYaml .Values.dagsterDaemon.resources | nindent 12 }}
@@ -126,7 +126,7 @@ spec:
         {{- if $userDeployments.enabled }}
         - name: dagster-workspace-yaml
           configMap:
-            name: {{ template "dagster.fullname" . }}-workspace-yaml
+            name: {{ include "dagit.workspace.configmapName" . }}
         {{- end }}
       affinity:
         {{- toYaml .Values.dagsterDaemon.affinity | nindent 8 }}

diff --git a/helm/dagster/templates/helpers/_deployment-dagit.tpl b/helm/dagster/templates/helpers/_deployment-dagit.tpl
@@ -93,9 +93,9 @@ spec:
               mountPath: "{{ .Values.global.dagsterHome }}/dagster.yaml"
               subPath: dagster.yaml
             {{- if $userDeployments.enabled }}
+            # Do not use `subPath` to allow the configmap to update if modified
             - name: dagster-workspace-yaml
-              mountPath: "/dagster-workspace/workspace.yaml"
-              subPath: workspace.yaml
+              mountPath: "/dagster-workspace/"
             {{- end }}
           ports:
             - name: http
@@ -128,7 +128,7 @@ spec:
         {{- if $userDeployments.enabled }}
         - name: dagster-workspace-yaml
           configMap:
-            name: {{ template "dagster.fullname" . }}-workspace-yaml
+            name: {{ include "dagit.workspace.configmapName" . }}
         {{- end }}
     {{- with .Values.dagit.affinity }}
       affinity:

diff --git a/helm/dagster/templates/helpers/_helpers.tpl b/helm/dagster/templates/helpers/_helpers.tpl
@@ -206,3 +206,12 @@ DAGSTER_K8S_PIPELINE_RUN_IMAGE_PULL_POLICY: "{{ .Values.pipelineRun.image.pullPo
   number: {{ .servicePort }}
   {{- end }}
 {{- end }}
+
+{{- define "dagit.workspace.configmapName" -}}
+{{- $dagitWorkspace := .Values.dagit.workspace }}
+{{- if and $dagitWorkspace.enabled $dagitWorkspace.externalConfigmap }}
+{{- $dagitWorkspace.externalConfigmap -}}
+{{- else -}}
+{{ template "dagster.fullname" . }}-workspace-yaml
+{{- end -}}
+{{- end -}}
diff --git a/helm/dagster/values.schema.json b/helm/dagster/values.schema.json
diff --git a/helm/dagster/values.yaml b/helm/dagster/values.yaml
@@ -47,14 +47,23 @@ dagit:
 
   # Defines a workspace for Dagit. This should only be set if user deployments are enabled, but
   # the subchart is disabled to manage user deployments in a separate Helm release.
-  # In this case, Dagit will need the addresses of the servers in order to load the user code.
+  # In this case, Dagit will need the addresses of the servers in order to load the user code,
+  # or the name of an existing configmap to mount as the workspace file.
   workspace:
     enabled: false
+
+    # List of servers to include in the workflow file. When set,
+    # `externalConfigmap` must be empty.
     servers:
       - host: "k8s-example-user-code-1"
         port: 3030
         name: "user-code-example"
 
+    # Defines the name of a configmap provisioned outside of the
+    # Helm release to use as workspace file. When set, `servers`
+    # must be empty.
+    externalConfigmap: ~
+
   # Deploy a separate instance of Dagit in --read-only mode (can't launch runs, disable schedules, etc.)
   enableReadOnly: false