experimental/ssh: reject unsupported serverless env v5 base environments#5825
Open
TanishqDatabricks wants to merge 1 commit into
Open
experimental/ssh: reject unsupported serverless env v5 base environments#5825TanishqDatabricks wants to merge 1 commit into
TanishqDatabricks wants to merge 1 commit into
Conversation
ssh connect --base-environment against a serverless environment on environment_version 5+ never connects: the driver proxy does not route to the server's port on that runtime, so the health check returns 503 for the whole startup budget and the command fails with an opaque timeout. A v4 base environment connects normally on the same workspace. Reject a known-v5 base environment up front, before any workspace side effects, with an actionable error. The version is read from the base-environment listing, which covers the display-name and resource-ID input forms. The env.yaml path form carries its version only inside the file and is not checkable here, so we emit a warning by default that its version is unverified. The check is fail-open: any inability to determine the version (list error, no match, nil spec, unparseable version) proceeds rather than blocking a valid connect. Co-authored-by: Isaac
TanishqDatabricks
force-pushed
the
ssh-reject-env-v5
branch
from
85e541e to
65b2901
Compare
Contributor
Waiting for approvalBased on git history, these people are best suited to review:
Eligible reviewers: Suggestions based on git history. See OWNERS for ownership rules. |
TanishqDatabricks
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
TanishqDatabricks
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Collaborator
Integration test reportCommit: 65b2901
21 interesting tests: 14 SKIP, 4 RECOVERED, 3 KNOWN
Top 5 slowest tests (at least 2 minutes):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
databricks ssh connect --base-environment <X>submits a serverless job and reaches the SSH server through the driver proxy. When the base environment uses serverlessenvironment_version5 or newer, the connection never succeeds: the server task runs and binds its port, but the driver proxy does not route to that port on the v5 runtime, so the client polls/metadataand gets503 remote connection failurefor the entire startup budget, then fails with an opaquetimed out ... status code 503. A v4 base environment connects normally on the same workspace and binary — the only material difference is the environment version.Serverless env v5 is not supported by the SSH tunnel (confirmed with the team; the routing gap is on the platform side). This makes the CLI fail fast with an actionable error instead of hanging until the timeout.
validateBaseEnvironmentVersion, called early inclient.Run— before any workspace side effects (secret scope, key upload, notebook import) — so the rejection is immediate and leaves nothing behind.ListWorkspaceBaseEnvironmentsAll(Spec.EnvironmentVersion), which covers the display-name andworkspace-base-environments/...resource-ID input forms./) carries its version only inside the file, not the environments API, so it is not checkable here without net-new WSFS-download + YAML-parse code for the least-used form. Instead, the command emits a warning by default when a path form is used, so the user knows the version wasn't verified and that v5 will fail.Introduces
maxSupportedEnvironmentVersion = 4alongside the existingminEnvironmentVersion = 4.Why
Without this, a user pointing
--base-environmentat a v5 environment waits out the full CPU/GPU startup timeout (10–45 min) for an opaque 503, with no indication that the configuration is unsupported. This turns that into an instant, actionable error.Tests
TestValidateBaseEnvironmentVersion(table-driven, mock workspace client): v5 rejected for display-name and resource-ID forms; v4 allowed; path form skipped (no API call); and the four fail-open cases (list error, no match, nil spec, unparseable version).go test ./experimental/ssh/...passes;gofmtandgo vetclean.TestAccept/sshacceptance suite passes.Manual validation (e2-dogfood)
--base-environment RemoteDevelopmentTest(v5, display name)--base-environment workspace-base-environments/remotedevelopmenttest-...(v5, resource ID)--base-environment ssh-tunnel-test-env(v4, display name)--base-environment /Workspace/.../env.yaml(path form)This pull request and its description were written by Isaac.