Skip to content

Commit

Permalink
Fixes for running the user-code-deployment helm chart in a different …
Browse files Browse the repository at this point in the history
…namespace than the system namespace (#7597)

Summary:
Added a test suite that runs the user code deployments in a separate namespace, and verifies that it can launch runs with includeConfigInLaunchedRuns on (which includes the namespace and service account in launched runs for that location).

Doing this in a way that worked out of the box required adding a way to let the service account launch jobs (similar to how the main helm chart can launch runs). It also exposed a few rough edges in our cross-namespace story, in that you need to be sure to include some additional configmaps in the user-deployments helm chart for things to work.

Test Plan: BK
  • Loading branch information
gibsondan committed Apr 27, 2022
1 parent ed8323c commit 0a1616b
Show file tree
Hide file tree
Showing 11 changed files with 356 additions and 198 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def build_spec_daemon_suite():


def build_spec_k8s_suite():
tox_env_suffixes = ["-default"]
tox_env_suffixes = ["-default", "-subchart"]
directory = os.path.join("integration_tests", "test_suites", "k8s-integration-test-suite")
return build_steps_integration_suite(directory, tox_env_suffixes, upload_coverage=True)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,5 +122,7 @@ DAGSTER_K8S_PIPELINE_RUN_ENV_CONFIGMAP: "{{ template "dagster.fullname" . }}-pip
{{- if .labels }}
labels: {{- .labels | toYaml | nindent 6 }}
{{- end }}
namespace: {{ $.Release.Namespace }}
service_account_name: {{ include "dagsterUserDeployments.serviceAccountName" $ }}
{{- end }}
{{- end -}}
8 changes: 8 additions & 0 deletions helm/dagster/schema/schema_tests/test_user_deployments.py
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,8 @@ def test_user_deployment_include_config(template: HelmTemplate):
"k8s": {
"image_pull_policy": "Always",
"env_config_maps": ["release-name-dagster-user-deployments-foo-user-env"],
"namespace": "default",
"service_account_name": "release-name-dagster-user-deployments-user-deployments",
}
}

Expand Down Expand Up @@ -645,6 +647,8 @@ def test_user_deployment_volumes(template: HelmTemplate, include_config_in_launc
"image_pull_policy": "Always",
"volume_mounts": volume_mounts,
"volumes": volumes,
"namespace": "default",
"service_account_name": "release-name-dagster-user-deployments-user-deployments",
}
}
else:
Expand Down Expand Up @@ -699,6 +703,8 @@ def test_user_deployment_secrets_and_configmaps(
"my-configmap",
"my-other-configmap",
],
"namespace": "default",
"service_account_name": "release-name-dagster-user-deployments-user-deployments",
}
}
else:
Expand Down Expand Up @@ -744,6 +750,8 @@ def test_user_deployment_labels(template: HelmTemplate, include_config_in_launch
"release-name-dagster-user-deployments-foo-user-env",
],
"labels": labels,
"namespace": "default",
"service_account_name": "release-name-dagster-user-deployments-user-deployments",
}
}
else:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,9 @@ def local_port_forward_postgres(namespace):


@pytest.fixture(scope="session")
def helm_postgres_url_for_k8s_run_launcher(helm_namespace_for_k8s_run_launcher):
def helm_postgres_url_for_k8s_run_launcher(system_namespace_for_k8s_run_launcher):
with local_port_forward_postgres(
namespace=helm_namespace_for_k8s_run_launcher
namespace=system_namespace_for_k8s_run_launcher
) as local_forward_port:
postgres_url = "postgresql://test:test@localhost:{local_forward_port}/test".format(
local_forward_port=local_forward_port
Expand Down

0 comments on commit 0a1616b

Please sign in to comment.