[for 0.15.0] default includeConfigInLaunchedRuns in helm chart to true (

#7488)

Summary:
I'll sit on this until we cut the branch for 0.15.0, but just holding onto it so I don't forget. Since it could be a minor breaking change (especically for people who are launching runs in a different namespace than their gRPC servers), but makes the most sense as the default going forward IMO.

helm/dagster/charts/dagster-user-deployments/templates/deployment-user.yaml

@@ -51,14 +51,13 @@ spec:
                 secretKeyRef:
                   name: {{ include "dagsterUserDeployments.postgresql.secretName" $ }}
                   key: postgresql-password
-            {{- if $deployment.includeConfigInLaunchedRuns }}
-            {{- if $deployment.includeConfigInLaunchedRuns.enabled }}
+            {{- $includeConfigInLaunchedRuns := $deployment.includeConfigInLaunchedRuns | default (dict "enabled" true) }}
+            {{- if $includeConfigInLaunchedRuns.enabled }}
             # uses the auto_envvar_prefix of the dagster cli to set the --container-context arg
             # on 'dagster api grpc'
             - name: DAGSTER_CLI_API_GRPC_CONTAINER_CONTEXT
               value: {{ include "dagsterUserDeployments.k8sContainerContext" (list $ $deployment) | fromYaml | toJson | quote }}
             {{- end }}
-            {{- end }}
           envFrom:
             - configMapRef:
                 name: {{ include "dagster.fullname" $ }}-user-deployments-shared-env

helm/dagster/charts/dagster-user-deployments/values.yaml

@@ -47,7 +47,7 @@ deployments:
     # Whether or not to include configuration specified for this user code deployment in the pods
     # launched for runs from that deployment
     includeConfigInLaunchedRuns:
-      enabled: false
+      enabled: true
 
     # Additional volumes that should be included in the Deployment's Pod. See:
     # https://v1-18.docs.kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#volume-v1-core

helm/dagster/schema/schema_tests/test_user_deployments.py

@@ -533,6 +533,11 @@ def _assert_no_container_context(user_deployment):
     assert "DAGSTER_CLI_API_GRPC_CONTAINER_CONTEXT" not in env_names
 
 
+def _assert_has_container_context(user_deployment):
+    env_names = [env.name for env in user_deployment.spec.template.spec.containers[0].env]
+    assert "DAGSTER_CLI_API_GRPC_CONTAINER_CONTEXT" in env_names
+
+
 def test_user_deployment_image(template: HelmTemplate):
     deployment = create_simple_user_deployment("foo")
     helm_values = DagsterHelmValues.construct(
@@ -553,23 +558,27 @@ def test_user_deployment_image(template: HelmTemplate):
     assert image_name == deployment.image.repository
     assert image_tag == deployment.image.tag
 
-    _assert_no_container_context(user_deployments[0])
+    _assert_has_container_context(user_deployments[0])
 
 
-def test_user_deployment_include_config(template: HelmTemplate):
-    deployment = create_simple_user_deployment("foo", include_config_in_launched_runs=True)
+def test_user_deployment_include_config_in_launched_runs(template: HelmTemplate):
+    deployments = [
+        create_simple_user_deployment("foo", include_config_in_launched_runs=True),
+        create_simple_user_deployment("bar", include_config_in_launched_runs=None),
+        create_simple_user_deployment("baz", include_config_in_launched_runs=False),
+    ]
+
     helm_values = DagsterHelmValues.construct(
         dagsterUserDeployments=UserDeployments.construct(
-            enabled=True,
-            enableSubchart=True,
-            deployments=[deployment],
+            enabled=True, enableSubchart=True, deployments=deployments
         )
     )
 
     user_deployments = template.render(helm_values)
 
-    assert len(user_deployments) == 1
+    assert len(user_deployments) == 3
 
+    # Setting to true results in container context being set
     container_context = user_deployments[0].spec.template.spec.containers[0].env[2]
     assert container_context.name == "DAGSTER_CLI_API_GRPC_CONTAINER_CONTEXT"
     assert json.loads(container_context.value) == {
@@ -581,6 +590,15 @@ def test_user_deployment_include_config(template: HelmTemplate):
         }
     }
 
+    # Setting to None also results in container context being set
+    assert (
+        user_deployments[1].spec.template.spec.containers[0].env[2].name
+        == "DAGSTER_CLI_API_GRPC_CONTAINER_CONTEXT"
+    )
+
+    # setting to false means no container context
+    _assert_no_container_context(user_deployments[2])
+
 
 @pytest.mark.parametrize("include_config_in_launched_runs", [False, True])
 def test_user_deployment_volumes(template: HelmTemplate, include_config_in_launched_runs: bool):

helm/dagster/schema/schema_tests/utils.py

@@ -8,7 +8,7 @@
 
 
 def create_simple_user_deployment(
-    name: str, include_config_in_launched_runs: Optional[bool] = False
+    name: str, include_config_in_launched_runs: Optional[bool] = None
 ) -> UserDeployment:
     return UserDeployment(
         name=name,

helm/dagster/values.yaml

@@ -268,7 +268,7 @@ dagster-user-deployments:
       # Whether or not to include configuration specified for this user code deployment in the pods
       # launched for runs from that deployment
       includeConfigInLaunchedRuns:
-        enabled: false
+        enabled: true
 
       # Additional environment variables to set.
       # A Kubernetes ConfigMap will be created with these environment variables. See: