From 4f277dbb606fcde24477d984c2ad65bd5a82f62b Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 21 Aug 2026 16:42:43 +0300 Subject: [PATCH 1/8] test(e2e): give the shared cluster warm-up a longer, single deadline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit startCluster runs in onPrepare, before any spec or wdio session — so specFileRetries can't recover a failure here; if it throws, the whole shard aborts. The old budget was a fixed 100 attempts x 10s (~17 min) of polling while the shared test cluster sat PENDING. Cloud-side node placement can take much longer than that: the shared cluster has been observed reaching RUNNING only ~1h after an UNEXPECTED_LAUNCH_FAILURE ("Timeout while placing nodes") that Databricks retried internally. Shards polling PENDING gave up at ~17 min and failed en masse, even though the change under test was unrelated (a pre-test infra timeout, not an assertion failure). Rework startCluster: - Poll against a single 60-minute deadline instead of a fixed attempt count. The e2e job has no timeout-minutes (GitHub's 6h default), so 60 min is safe. - Drop the SDK .wait() on the start path so the deadline is the only budget (the waiter carried its own, shorter, hidden timeout). - Tolerate a concurrent start() on the shared cluster (a sibling shard may have already issued it) and keep polling instead of erroring out. - Log state_message so the cloud-side reason for a slow/failed launch is visible directly in CI. Co-authored-by: Isaac --- .../src/test/e2e/wdio.conf.ts | 98 ++++++++++++------- 1 file changed, 61 insertions(+), 37 deletions(-) diff --git a/packages/databricks-vscode/src/test/e2e/wdio.conf.ts b/packages/databricks-vscode/src/test/e2e/wdio.conf.ts index 8d85dc86e..ea5cc6e17 100644 --- a/packages/databricks-vscode/src/test/e2e/wdio.conf.ts +++ b/packages/databricks-vscode/src/test/e2e/wdio.conf.ts @@ -739,46 +739,70 @@ function getWorkspaceClient(config: Config) { return client; } +// All e2e shards share one test cluster and warm it up here, in onPrepare, +// before any spec (or even any wdio session) runs — so specFileRetries can't +// recover a failure at this stage; if this throws, the whole shard aborts. +// Cloud-side node placement can be slow: we've seen the shared cluster take +// ~1h to reach RUNNING after an UNEXPECTED_LAUNCH_FAILURE ("Timeout while +// placing nodes") that Databricks then retried internally. The old fixed +// budget (100 attempts x 10s ~= 17min) gave up long before that, failing many +// shards on transient slowness. Poll against a single generous deadline +// instead (the e2e job itself has no timeout, so 60min is well within it), and +// log state_message so the cloud-side reason is visible directly in CI. +const CLUSTER_START_TIMEOUT_MS = 60 * 60 * 1000; // 60 minutes +const CLUSTER_POLL_INTERVAL_MS = 10_000; + async function startCluster( workspaceClient: WorkspaceClient, - clusterId: string, - attempt = 0 + clusterId: string ) { console.log(`Cluster ID: ${clusterId}`); - if (attempt > 100) { - throw new Error("Failed to start the cluster: too many attempts"); - } - const cluster = await workspaceClient.clusters.get({ - cluster_id: clusterId, - }); - console.log(`Cluster State: ${cluster.state}`); - switch (cluster.state) { - case "RUNNING": - console.log("Cluster is already running"); - break; - case "TERMINATED": - case "ERROR": - case "UNKNOWN": - console.log("Starting the cluster..."); - await ( - await workspaceClient.clusters.start({ - cluster_id: clusterId, - }) - ).wait({ - onProgress: async (state) => { - console.log(`Cluster state: ${state.state}`); - }, - }); - break; - case "PENDING": - case "RESIZING": - case "TERMINATING": - case "RESTARTING": - console.log("Waiting and retrying..."); - await sleep(10000); - await startCluster(workspaceClient, clusterId, attempt + 1); - break; - default: - throw new Error(`Unknown cluster state: ${cluster.state}`); + const deadline = Date.now() + CLUSTER_START_TIMEOUT_MS; + for (;;) { + const cluster = await workspaceClient.clusters.get({ + cluster_id: clusterId, + }); + const stateMessage = cluster.state_message + ? ` - ${cluster.state_message}` + : ""; + console.log(`Cluster State: ${cluster.state}${stateMessage}`); + switch (cluster.state) { + case "RUNNING": + console.log("Cluster is running"); + return; + case "TERMINATED": + case "ERROR": + case "UNKNOWN": + // Kick off a start, then keep polling below until it reaches + // RUNNING. We deliberately don't use the SDK waiter so this + // deadline is the single source of truth for how long we wait. + console.log("Starting the cluster..."); + try { + await workspaceClient.clusters.start({ + cluster_id: clusterId, + }); + } catch (e) { + // The cluster is shared: a concurrent shard may have already + // issued the start, racing this call into an "unexpected + // state" error. Keep polling - it's coming up regardless. + console.log(`clusters.start failed, continuing to poll: ${e}`); + } + break; + case "PENDING": + case "RESIZING": + case "TERMINATING": + case "RESTARTING": + console.log("Waiting and retrying..."); + break; + default: + throw new Error(`Unknown cluster state: ${cluster.state}`); + } + if (Date.now() >= deadline) { + const timeoutMin = CLUSTER_START_TIMEOUT_MS / 60_000; + throw new Error( + `Failed to start the cluster within ${timeoutMin} min; last state ${cluster.state}${stateMessage}` + ); + } + await sleep(CLUSTER_POLL_INTERVAL_MS); } } From a4f236948ddd0dd4da49f8fcc757b6ad8b348d00 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 21 Aug 2026 19:30:20 +0300 Subject: [PATCH 2/8] test(e2e): reuse production Cluster.start() instead of a hand-rolled loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the previous commit's hand-rolled deadline/polling loop with a reuse of the production Cluster.start(), addressing review feedback (a blanket catch that turned permanent start failures into hour-long retries; a hand-rolled loop duplicating logic the repo already has). Cluster.start() already polls a stopped cluster to RUNNING and fails fast on TERMINATED/ERROR (surfacing termination_reason) via the SDK retry() idiom. Its only limitation here was the timeout: - Add an optional `timeout` param to Cluster.start(), defaulting to the SDK DEFAULT_MAX_TIMEOUT so existing callers are unchanged — mirroring ExecutionContext/Command which already expose the same knob. Covered by a new Cluster.test.ts case. - The e2e harness builds the cluster via Cluster.fromClusterId and passes a 60 min timeout (the shared cluster's node placement has been seen taking ~1h; the e2e job has no timeout-minutes, so GitHub's 6h default bounds it). Co-authored-by: Isaac --- .../src/sdk-extensions/Cluster.test.ts | 41 +++++++++ .../src/sdk-extensions/Cluster.ts | 4 +- .../src/test/e2e/wdio.conf.ts | 86 ++++++------------- 3 files changed, 68 insertions(+), 63 deletions(-) diff --git a/packages/databricks-vscode/src/sdk-extensions/Cluster.test.ts b/packages/databricks-vscode/src/sdk-extensions/Cluster.test.ts index 71c0d7b21..15c46565a 100644 --- a/packages/databricks-vscode/src/sdk-extensions/Cluster.test.ts +++ b/packages/databricks-vscode/src/sdk-extensions/Cluster.test.ts @@ -107,6 +107,47 @@ describe(__filename, function () { ).never(); }); + it("start accepts a caller-provided timeout and polls to RUNNING", async () => { + when( + mockedClient.request( + objectContaining({ + path: "/api/2.1/clusters/get", + method: "GET", + }), + anything() + ) + ).thenResolve( + {...testClusterDetails, state: "TERMINATED"}, + {...testClusterDetails, state: "TERMINATED"}, + {...testClusterDetails, state: "PENDING"}, + {...testClusterDetails, state: "RUNNING"} + ); + when( + mockedClient.request( + objectContaining({ + path: "/api/2.1/clusters/start", + method: "POST", + }), + anything() + ) + ).thenResolve({}); + + await mockedCluster.refresh(); + assert.equal(mockedCluster.state, "TERMINATED"); + + // Third arg is the new caller-provided timeout: start() must accept it + // and still poll a stopped cluster through to RUNNING. + const startPromise = mockedCluster.start( + undefined, + () => {}, + new Time(60, TimeUnits.minutes) + ); + await fakeTimer.runToLastAsync(); + await startPromise; + + assert.equal(mockedCluster.state, "RUNNING"); + }); + it("should terminate cluster", async () => { when( mockedClient.request( diff --git a/packages/databricks-vscode/src/sdk-extensions/Cluster.ts b/packages/databricks-vscode/src/sdk-extensions/Cluster.ts index 393f4cddf..9339ab3c1 100644 --- a/packages/databricks-vscode/src/sdk-extensions/Cluster.ts +++ b/packages/databricks-vscode/src/sdk-extensions/Cluster.ts @@ -196,7 +196,8 @@ export class Cluster { async start( token?: CancellationToken, - onProgress: (state: compute.State) => void = () => {} + onProgress: (state: compute.State) => void = () => {}, + timeout: Time = retries.DEFAULT_MAX_TIMEOUT ) { await this.refresh(); onProgress(this.state); @@ -241,6 +242,7 @@ export class Cluster { this._canExecute = undefined; await retry({ + timeout, fn: async () => { if (token?.isCancellationRequested) { return; diff --git a/packages/databricks-vscode/src/test/e2e/wdio.conf.ts b/packages/databricks-vscode/src/test/e2e/wdio.conf.ts index ea5cc6e17..84133ae22 100644 --- a/packages/databricks-vscode/src/test/e2e/wdio.conf.ts +++ b/packages/databricks-vscode/src/test/e2e/wdio.conf.ts @@ -6,7 +6,12 @@ import path from "node:path"; import {fileURLToPath} from "url"; import assert from "assert"; import fs from "fs/promises"; -import {Config, WorkspaceClient} from "@databricks/sdk-experimental"; +import { + Config, + Time, + TimeUnits, + WorkspaceClient, +} from "@databricks/sdk-experimental"; import * as ElementCustomCommands from "./customCommands/elementCustomCommands.ts"; import {execFile as execFileCb} from "node:child_process"; import { @@ -31,6 +36,7 @@ import { formatRecoveredSpecsReport, } from "../retry.ts"; import {SpecRetryTracker} from "../SpecRetryTracker.ts"; +import {Cluster} from "../../sdk-extensions/Cluster.ts"; // WebdriverIO v9 loads TypeScript by injecting `--import ` into // NODE_OPTIONS for every worker process. wdio-vscode-service installs the @@ -739,70 +745,26 @@ function getWorkspaceClient(config: Config) { return client; } -// All e2e shards share one test cluster and warm it up here, in onPrepare, -// before any spec (or even any wdio session) runs — so specFileRetries can't -// recover a failure at this stage; if this throws, the whole shard aborts. -// Cloud-side node placement can be slow: we've seen the shared cluster take -// ~1h to reach RUNNING after an UNEXPECTED_LAUNCH_FAILURE ("Timeout while -// placing nodes") that Databricks then retried internally. The old fixed -// budget (100 attempts x 10s ~= 17min) gave up long before that, failing many -// shards on transient slowness. Poll against a single generous deadline -// instead (the e2e job itself has no timeout, so 60min is well within it), and -// log state_message so the cloud-side reason is visible directly in CI. -const CLUSTER_START_TIMEOUT_MS = 60 * 60 * 1000; // 60 minutes -const CLUSTER_POLL_INTERVAL_MS = 10_000; +// The e2e shards share one test cluster, warmed up here in onPrepare (before +// any spec/session — a throw here aborts the whole shard and specFileRetries +// can't recover it). Reuse the production Cluster.start(): it polls a stopped +// cluster to RUNNING and fails fast on TERMINATED/ERROR with the +// termination_reason. Its default timeout is the SDK's ~20min, but the shared +// cluster's cloud node placement has been seen taking ~1h, so pass 60min (the +// e2e job has no timeout-minutes, so GitHub's 6h default bounds it). +const CLUSTER_START_TIMEOUT = new Time(60, TimeUnits.minutes); async function startCluster( workspaceClient: WorkspaceClient, clusterId: string ) { - console.log(`Cluster ID: ${clusterId}`); - const deadline = Date.now() + CLUSTER_START_TIMEOUT_MS; - for (;;) { - const cluster = await workspaceClient.clusters.get({ - cluster_id: clusterId, - }); - const stateMessage = cluster.state_message - ? ` - ${cluster.state_message}` - : ""; - console.log(`Cluster State: ${cluster.state}${stateMessage}`); - switch (cluster.state) { - case "RUNNING": - console.log("Cluster is running"); - return; - case "TERMINATED": - case "ERROR": - case "UNKNOWN": - // Kick off a start, then keep polling below until it reaches - // RUNNING. We deliberately don't use the SDK waiter so this - // deadline is the single source of truth for how long we wait. - console.log("Starting the cluster..."); - try { - await workspaceClient.clusters.start({ - cluster_id: clusterId, - }); - } catch (e) { - // The cluster is shared: a concurrent shard may have already - // issued the start, racing this call into an "unexpected - // state" error. Keep polling - it's coming up regardless. - console.log(`clusters.start failed, continuing to poll: ${e}`); - } - break; - case "PENDING": - case "RESIZING": - case "TERMINATING": - case "RESTARTING": - console.log("Waiting and retrying..."); - break; - default: - throw new Error(`Unknown cluster state: ${cluster.state}`); - } - if (Date.now() >= deadline) { - const timeoutMin = CLUSTER_START_TIMEOUT_MS / 60_000; - throw new Error( - `Failed to start the cluster within ${timeoutMin} min; last state ${cluster.state}${stateMessage}` - ); - } - await sleep(CLUSTER_POLL_INTERVAL_MS); - } + const cluster = await Cluster.fromClusterId( + workspaceClient.apiClient, + clusterId + ); + await cluster.start( + undefined, + (state) => console.log(`Cluster ${clusterId} state: ${state}`), + CLUSTER_START_TIMEOUT + ); } From 4382a354832a51c8c66a067459acc78b20da5a2a Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 21 Aug 2026 19:47:20 +0300 Subject: [PATCH 3/8] test(e2e): extract cluster warm-up into a dedicated test util Round-2 review of the previous approach (reusing the production Cluster.start()) surfaced that fitting the shared-cluster e2e scenario would need production changes: a caller timeout threaded through the TERMINATING wait, tolerance for the concurrent-start race across ~40 shards, and per-poll state_message logging. Changing Cluster.start() (used by 10+ callers incl. the UI) for a test-only need is too wide a blast radius. Instead keep production untouched and give the e2e path its own helper (src/test/startCluster.ts), duplicating the SDK retry() idiom: - One 60-minute deadline over the whole start (the shared cluster's cloud node placement has been seen taking ~1h; the e2e job has no timeout-minutes). - Fails fast: a terminal state after start is surfaced as ClusterStartError with the cloud reason, not retried to the deadline. - Tolerates the shared-cluster start race: a sibling shard's start() is swallowed and the poll (fatal-on-terminal) still catches real failures. - Logs state_message each poll for CI visibility. Reverts the Cluster.start()/Cluster.test.ts changes from the previous commit; covered by src/test/startCluster.test.ts. Co-authored-by: Isaac --- .../src/sdk-extensions/Cluster.test.ts | 41 ------- .../src/sdk-extensions/Cluster.ts | 4 +- .../src/test/e2e/wdio.conf.ts | 37 +----- .../src/test/startCluster.test.ts | 115 ++++++++++++++++++ .../src/test/startCluster.ts | 104 ++++++++++++++++ 5 files changed, 226 insertions(+), 75 deletions(-) create mode 100644 packages/databricks-vscode/src/test/startCluster.test.ts create mode 100644 packages/databricks-vscode/src/test/startCluster.ts diff --git a/packages/databricks-vscode/src/sdk-extensions/Cluster.test.ts b/packages/databricks-vscode/src/sdk-extensions/Cluster.test.ts index 15c46565a..71c0d7b21 100644 --- a/packages/databricks-vscode/src/sdk-extensions/Cluster.test.ts +++ b/packages/databricks-vscode/src/sdk-extensions/Cluster.test.ts @@ -107,47 +107,6 @@ describe(__filename, function () { ).never(); }); - it("start accepts a caller-provided timeout and polls to RUNNING", async () => { - when( - mockedClient.request( - objectContaining({ - path: "/api/2.1/clusters/get", - method: "GET", - }), - anything() - ) - ).thenResolve( - {...testClusterDetails, state: "TERMINATED"}, - {...testClusterDetails, state: "TERMINATED"}, - {...testClusterDetails, state: "PENDING"}, - {...testClusterDetails, state: "RUNNING"} - ); - when( - mockedClient.request( - objectContaining({ - path: "/api/2.1/clusters/start", - method: "POST", - }), - anything() - ) - ).thenResolve({}); - - await mockedCluster.refresh(); - assert.equal(mockedCluster.state, "TERMINATED"); - - // Third arg is the new caller-provided timeout: start() must accept it - // and still poll a stopped cluster through to RUNNING. - const startPromise = mockedCluster.start( - undefined, - () => {}, - new Time(60, TimeUnits.minutes) - ); - await fakeTimer.runToLastAsync(); - await startPromise; - - assert.equal(mockedCluster.state, "RUNNING"); - }); - it("should terminate cluster", async () => { when( mockedClient.request( diff --git a/packages/databricks-vscode/src/sdk-extensions/Cluster.ts b/packages/databricks-vscode/src/sdk-extensions/Cluster.ts index 9339ab3c1..393f4cddf 100644 --- a/packages/databricks-vscode/src/sdk-extensions/Cluster.ts +++ b/packages/databricks-vscode/src/sdk-extensions/Cluster.ts @@ -196,8 +196,7 @@ export class Cluster { async start( token?: CancellationToken, - onProgress: (state: compute.State) => void = () => {}, - timeout: Time = retries.DEFAULT_MAX_TIMEOUT + onProgress: (state: compute.State) => void = () => {} ) { await this.refresh(); onProgress(this.state); @@ -242,7 +241,6 @@ export class Cluster { this._canExecute = undefined; await retry({ - timeout, fn: async () => { if (token?.isCancellationRequested) { return; diff --git a/packages/databricks-vscode/src/test/e2e/wdio.conf.ts b/packages/databricks-vscode/src/test/e2e/wdio.conf.ts index 84133ae22..6df2b84bc 100644 --- a/packages/databricks-vscode/src/test/e2e/wdio.conf.ts +++ b/packages/databricks-vscode/src/test/e2e/wdio.conf.ts @@ -6,12 +6,7 @@ import path from "node:path"; import {fileURLToPath} from "url"; import assert from "assert"; import fs from "fs/promises"; -import { - Config, - Time, - TimeUnits, - WorkspaceClient, -} from "@databricks/sdk-experimental"; +import {Config, WorkspaceClient} from "@databricks/sdk-experimental"; import * as ElementCustomCommands from "./customCommands/elementCustomCommands.ts"; import {execFile as execFileCb} from "node:child_process"; import { @@ -36,7 +31,7 @@ import { formatRecoveredSpecsReport, } from "../retry.ts"; import {SpecRetryTracker} from "../SpecRetryTracker.ts"; -import {Cluster} from "../../sdk-extensions/Cluster.ts"; +import {startCluster} from "../startCluster.ts"; // WebdriverIO v9 loads TypeScript by injecting `--import ` into // NODE_OPTIONS for every worker process. wdio-vscode-service installs the @@ -387,7 +382,10 @@ export const config: WebdriverIO.Config = { await fs.mkdir(WORKSPACE_PATH, {recursive: true}); const client = getWorkspaceClient(config); - await startCluster(client, process.env["TEST_DEFAULT_CLUSTER_ID"]); + await startCluster( + client.apiClient, + process.env["TEST_DEFAULT_CLUSTER_ID"] + ); process.env.DATABRICKS_HOST = config.host!; process.env.DATABRICKS_VSCODE_INTEGRATION_TEST = "true"; @@ -745,26 +743,3 @@ function getWorkspaceClient(config: Config) { return client; } -// The e2e shards share one test cluster, warmed up here in onPrepare (before -// any spec/session — a throw here aborts the whole shard and specFileRetries -// can't recover it). Reuse the production Cluster.start(): it polls a stopped -// cluster to RUNNING and fails fast on TERMINATED/ERROR with the -// termination_reason. Its default timeout is the SDK's ~20min, but the shared -// cluster's cloud node placement has been seen taking ~1h, so pass 60min (the -// e2e job has no timeout-minutes, so GitHub's 6h default bounds it). -const CLUSTER_START_TIMEOUT = new Time(60, TimeUnits.minutes); - -async function startCluster( - workspaceClient: WorkspaceClient, - clusterId: string -) { - const cluster = await Cluster.fromClusterId( - workspaceClient.apiClient, - clusterId - ); - await cluster.start( - undefined, - (state) => console.log(`Cluster ${clusterId} state: ${state}`), - CLUSTER_START_TIMEOUT - ); -} diff --git a/packages/databricks-vscode/src/test/startCluster.test.ts b/packages/databricks-vscode/src/test/startCluster.test.ts new file mode 100644 index 000000000..9d120baf5 --- /dev/null +++ b/packages/databricks-vscode/src/test/startCluster.test.ts @@ -0,0 +1,115 @@ +/* eslint-disable @typescript-eslint/naming-convention */ + +import {ApiClient, Time, TimeUnits, compute} from "@databricks/sdk-experimental"; +import {startCluster, ClusterStartError} from "./startCluster"; +import * as assert from "node:assert"; +import {mock, when, instance, anything, objectContaining, verify} from "ts-mockito"; +import FakeTimers from "@sinonjs/fake-timers"; + +describe(__filename, function () { + this.timeout(new Time(10, TimeUnits.minutes).toMillSeconds().value); + + const clusterId = "test-cluster-id"; + let mockedClient: ApiClient; + let fakeTimer: FakeTimers.Clock; + + const details = ( + state: compute.State, + extra: Partial = {} + ): compute.ClusterDetails => + ({cluster_id: clusterId, state, ...extra}) as compute.ClusterDetails; + + const whenGet = () => + when( + mockedClient.request( + objectContaining({path: "/api/2.1/clusters/get", method: "GET"}), + anything() + ) + ); + const whenStart = () => + when( + mockedClient.request( + objectContaining({ + path: "/api/2.1/clusters/start", + method: "POST", + }), + anything() + ) + ); + const verifyStarted = (times: number) => + verify( + mockedClient.request( + objectContaining({ + path: "/api/2.1/clusters/start", + method: "POST", + }), + anything() + ) + ).times(times); + + beforeEach(() => { + mockedClient = mock(ApiClient); + fakeTimer = FakeTimers.install({shouldClearNativeTimers: true}); + }); + + afterEach(() => { + fakeTimer.uninstall(); + }); + + it("returns without starting when the cluster is already RUNNING", async () => { + whenGet().thenResolve(details("RUNNING")); + + await startCluster(instance(mockedClient), clusterId); + + verifyStarted(0); + }); + + it("starts a stopped cluster and polls until RUNNING", async () => { + whenGet().thenResolve( + details("TERMINATED"), + details("PENDING"), + details("RUNNING") + ); + whenStart().thenResolve({}); + + const startPromise = startCluster(instance(mockedClient), clusterId); + await fakeTimer.runToLastAsync(); + await startPromise; + + verifyStarted(1); + }); + + it("fails fast when the cluster returns to a terminal state after start", async () => { + whenGet().thenResolve( + details("TERMINATED"), + details("TERMINATED", {state_message: "bad spark config"}) + ); + whenStart().thenResolve({}); + + const startPromise = startCluster(instance(mockedClient), clusterId); + const rejection = assert.rejects( + startPromise, + (e: Error) => + e instanceof ClusterStartError && /bad spark config/.test(e.message) + ); + await fakeTimer.runToLastAsync(); + await rejection; + }); + + it("tolerates a concurrent start race on the shared cluster", async () => { + // Initial TERMINATED -> our start() races a sibling and throws -> + // the cluster is already coming up (PENDING) -> RUNNING. + whenGet().thenResolve( + details("TERMINATED"), + details("PENDING"), + details("RUNNING") + ); + whenStart().thenReject( + new Error(`Cluster ${clusterId} is in unexpected state Pending.`) + ); + + const startPromise = startCluster(instance(mockedClient), clusterId); + await fakeTimer.runToLastAsync(); + await startPromise; + }); +}); diff --git a/packages/databricks-vscode/src/test/startCluster.ts b/packages/databricks-vscode/src/test/startCluster.ts new file mode 100644 index 000000000..9784438aa --- /dev/null +++ b/packages/databricks-vscode/src/test/startCluster.ts @@ -0,0 +1,104 @@ +/* eslint-disable no-console */ + +import { + ApiClient, + Time, + TimeUnits, + compute, + retry, + retries, +} from "@databricks/sdk-experimental"; + +// Warms up the shared e2e test cluster from wdio's onPrepare, before any spec +// or session runs (a throw here aborts the whole shard — specFileRetries can't +// recover it). Deliberately duplicates the SDK retry() idiom rather than +// reusing the production Cluster.start() so the e2e path can own a long +// timeout (the shared cluster's cloud node placement has been seen taking ~1h) +// and tolerate the shared-cluster start race, without changing production. +export class ClusterStartError extends Error {} + +const DEFAULT_START_TIMEOUT = new Time(60, TimeUnits.minutes); +const POLL_INTERVAL = new Time(10, TimeUnits.seconds); + +export async function startCluster( + client: ApiClient, + clusterId: string, + timeout: Time = DEFAULT_START_TIMEOUT +) { + const clusterApi = new compute.ClustersService(client); + const log = (c: compute.ClusterDetails) => + console.log( + `Cluster ${clusterId} state: ${c.state}${ + c.state_message ? ` - ${c.state_message}` : "" + }` + ); + + let cluster = await clusterApi.get({cluster_id: clusterId}); + log(cluster); + if (cluster.state === "RUNNING") { + return; + } + + // If it's shutting down, wait for that to finish before restarting it. + if (cluster.state === "TERMINATING") { + await retry({ + timeout, + retryPolicy: new retries.LinearRetryPolicy(POLL_INTERVAL), + fn: async () => { + cluster = await clusterApi.get({cluster_id: clusterId}); + log(cluster); + if (cluster.state === "TERMINATING") { + throw new retries.RetriableError(); + } + }, + }); + } + + if ( + cluster.state === "TERMINATED" || + cluster.state === "ERROR" || + cluster.state === "UNKNOWN" + ) { + // The cluster is shared across ~40 shards: a sibling may have already + // issued the start, racing this call into an "unexpected state" error. + // Swallow it and let the poll below decide — a genuine failure lands the + // cluster back in a terminal state, which the poll treats as fatal, so + // this does not mask permanent failures. + try { + await clusterApi.start({cluster_id: clusterId}); + } catch (e) { + console.log( + `clusters.start on shared cluster ${clusterId}: ${ + e instanceof Error ? e.message : String(e) + }` + ); + } + } + + // Poll to RUNNING under one deadline. A terminal state here is a real launch + // failure (the start was already issued), so fail fast with the cloud-side + // reason instead of burning the whole timeout. + await retry({ + timeout, + retryPolicy: new retries.LinearRetryPolicy(POLL_INTERVAL), + fn: async () => { + cluster = await clusterApi.get({cluster_id: clusterId}); + log(cluster); + switch (cluster.state) { + case "RUNNING": + return; + case "TERMINATED": + case "ERROR": + throw new ClusterStartError( + `Cluster ${clusterId} failed to start (${ + cluster.state + }): ${JSON.stringify( + cluster.state_message ?? cluster.termination_reason + )}` + ); + default: + throw new retries.RetriableError(); + } + }, + }); +} From df63bd0bcc26e2c5c5763a8f9d5487d2c64ec6bc Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 21 Aug 2026 20:08:02 +0300 Subject: [PATCH 4/8] test(e2e): narrow the start-race catch and cover more paths Round-3 review fixes for the cluster warm-up util: - Narrow the catch: only tolerate the concurrent-start race (on re-check the cluster is already coming up); rethrow genuine start failures (auth, permissions, bad request) instead of masking them behind a later terminal-state error. - Fall back to "unknown reason" in the failure message when neither state_message nor termination_reason is set (avoids a literal "undefined"). - Add tests for the TERMINATING wait path and for a propagated non-race start error; fix the test's import order. Co-authored-by: Isaac --- .../src/test/startCluster.test.ts | 36 +++++++++++++++++-- .../src/test/startCluster.ts | 28 +++++++++------ 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/packages/databricks-vscode/src/test/startCluster.test.ts b/packages/databricks-vscode/src/test/startCluster.test.ts index 9d120baf5..4203c74bf 100644 --- a/packages/databricks-vscode/src/test/startCluster.test.ts +++ b/packages/databricks-vscode/src/test/startCluster.test.ts @@ -1,10 +1,10 @@ /* eslint-disable @typescript-eslint/naming-convention */ import {ApiClient, Time, TimeUnits, compute} from "@databricks/sdk-experimental"; -import {startCluster, ClusterStartError} from "./startCluster"; import * as assert from "node:assert"; import {mock, when, instance, anything, objectContaining, verify} from "ts-mockito"; import FakeTimers from "@sinonjs/fake-timers"; +import {startCluster, ClusterStartError} from "./startCluster"; describe(__filename, function () { this.timeout(new Time(10, TimeUnits.minutes).toMillSeconds().value); @@ -97,8 +97,8 @@ describe(__filename, function () { }); it("tolerates a concurrent start race on the shared cluster", async () => { - // Initial TERMINATED -> our start() races a sibling and throws -> - // the cluster is already coming up (PENDING) -> RUNNING. + // Initial TERMINATED -> our start() races a sibling and throws -> the + // re-check finds it already coming up (PENDING) -> RUNNING. whenGet().thenResolve( details("TERMINATED"), details("PENDING"), @@ -112,4 +112,34 @@ describe(__filename, function () { await fakeTimer.runToLastAsync(); await startPromise; }); + + it("propagates a non-race start error when the cluster stays stopped", async () => { + // start() fails and the re-check shows the cluster still stopped, so the + // original (actionable) error surfaces rather than being masked. + whenGet().thenResolve(details("TERMINATED"), details("TERMINATED")); + whenStart().thenReject(new Error("permission denied")); + + const startPromise = startCluster(instance(mockedClient), clusterId); + const rejection = assert.rejects( + startPromise, + (e: Error) => /permission denied/.test(e.message) + ); + await fakeTimer.runToLastAsync(); + await rejection; + }); + + it("waits for a TERMINATING cluster to stop, then starts it", async () => { + whenGet().thenResolve( + details("TERMINATING"), + details("TERMINATED"), + details("RUNNING") + ); + whenStart().thenResolve({}); + + const startPromise = startCluster(instance(mockedClient), clusterId); + await fakeTimer.runToLastAsync(); + await startPromise; + + verifyStarted(1); + }); }); diff --git a/packages/databricks-vscode/src/test/startCluster.ts b/packages/databricks-vscode/src/test/startCluster.ts index 9784438aa..3554dec36 100644 --- a/packages/databricks-vscode/src/test/startCluster.ts +++ b/packages/databricks-vscode/src/test/startCluster.ts @@ -59,19 +59,23 @@ export async function startCluster( cluster.state === "ERROR" || cluster.state === "UNKNOWN" ) { - // The cluster is shared across ~40 shards: a sibling may have already - // issued the start, racing this call into an "unexpected state" error. - // Swallow it and let the poll below decide — a genuine failure lands the - // cluster back in a terminal state, which the poll treats as fatal, so - // this does not mask permanent failures. try { await clusterApi.start({cluster_id: clusterId}); } catch (e) { - console.log( - `clusters.start on shared cluster ${clusterId}: ${ - e instanceof Error ? e.message : String(e) - }` - ); + // The cluster is shared across ~40 shards, so a sibling may have + // already started it, racing this call into an error. Re-check: if + // it's now coming up we merely raced, so poll below; if it's still + // stopped the start genuinely failed (auth, permissions, bad + // request), so surface that actionable error rather than mask it. + cluster = await clusterApi.get({cluster_id: clusterId}); + log(cluster); + if ( + cluster.state === "TERMINATED" || + cluster.state === "ERROR" || + cluster.state === "UNKNOWN" + ) { + throw e; + } } } @@ -93,7 +97,9 @@ export async function startCluster( `Cluster ${clusterId} failed to start (${ cluster.state }): ${JSON.stringify( - cluster.state_message ?? cluster.termination_reason + cluster.state_message ?? + cluster.termination_reason ?? + "unknown reason" )}` ); default: From b206f56b638d301f5612c296ca79b83fdfcc228f Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 21 Aug 2026 20:18:03 +0300 Subject: [PATCH 5/8] test(e2e): fail fast on UNKNOWN and bound the whole start to one deadline Round-4 review fixes for the cluster warm-up util: - Treat UNKNOWN as terminal in the start poll (flagged by Codex and Claude): once the start is issued, UNKNOWN is a failed launch, so fail fast instead of retrying to the timeout. Consistent with the pre-start / race-recheck sets. - Share one deadline across the TERMINATING wait and the RUNNING poll: each retry() previously received the full timeout, so a slow shutdown could nearly double the caller's bound. Compute the deadline once and pass the remaining time to each phase. - Add a test for UNKNOWN-after-start. Co-authored-by: Isaac --- .../src/test/startCluster.test.ts | 18 ++++++++++++++++++ .../databricks-vscode/src/test/startCluster.ts | 12 ++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/databricks-vscode/src/test/startCluster.test.ts b/packages/databricks-vscode/src/test/startCluster.test.ts index 4203c74bf..65c57e28b 100644 --- a/packages/databricks-vscode/src/test/startCluster.test.ts +++ b/packages/databricks-vscode/src/test/startCluster.test.ts @@ -142,4 +142,22 @@ describe(__filename, function () { verifyStarted(1); }); + + it("fails fast when the cluster is UNKNOWN after start", async () => { + whenGet().thenResolve( + details("TERMINATED"), + details("UNKNOWN", {state_message: "lost the cluster"}) + ); + whenStart().thenResolve({}); + + const startPromise = startCluster(instance(mockedClient), clusterId); + const rejection = assert.rejects( + startPromise, + (e: Error) => + e instanceof ClusterStartError && + /lost the cluster/.test(e.message) + ); + await fakeTimer.runToLastAsync(); + await rejection; + }); }); diff --git a/packages/databricks-vscode/src/test/startCluster.ts b/packages/databricks-vscode/src/test/startCluster.ts index 3554dec36..b575c6420 100644 --- a/packages/databricks-vscode/src/test/startCluster.ts +++ b/packages/databricks-vscode/src/test/startCluster.ts @@ -33,6 +33,13 @@ export async function startCluster( }` ); + // One deadline across both the shutdown wait and the start poll, so a slow + // TERMINATING phase can't hand the poll a fresh full timeout and let the + // total exceed the caller's bound. + const deadline = Date.now() + timeout.toMillSeconds().value; + const remaining = () => + new Time(Math.max(0, deadline - Date.now()), TimeUnits.milliseconds); + let cluster = await clusterApi.get({cluster_id: clusterId}); log(cluster); if (cluster.state === "RUNNING") { @@ -42,7 +49,7 @@ export async function startCluster( // If it's shutting down, wait for that to finish before restarting it. if (cluster.state === "TERMINATING") { await retry({ - timeout, + timeout: remaining(), retryPolicy: new retries.LinearRetryPolicy(POLL_INTERVAL), fn: async () => { cluster = await clusterApi.get({cluster_id: clusterId}); @@ -83,7 +90,7 @@ export async function startCluster( // failure (the start was already issued), so fail fast with the cloud-side // reason instead of burning the whole timeout. await retry({ - timeout, + timeout: remaining(), retryPolicy: new retries.LinearRetryPolicy(POLL_INTERVAL), fn: async () => { cluster = await clusterApi.get({cluster_id: clusterId}); @@ -93,6 +100,7 @@ export async function startCluster( return; case "TERMINATED": case "ERROR": + case "UNKNOWN": throw new ClusterStartError( `Cluster ${clusterId} failed to start (${ cluster.state From 2dc0164d8cbe8d93fcb84a584c041db0f4eb5620 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 21 Aug 2026 20:24:48 +0300 Subject: [PATCH 6/8] test(e2e): assert start was attempted in the concurrent-race test Make the race test's success explicit (verifyStarted) rather than relying only on the awaited promise not throwing. Round-5 review nit. Co-authored-by: Isaac --- packages/databricks-vscode/src/test/startCluster.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/databricks-vscode/src/test/startCluster.test.ts b/packages/databricks-vscode/src/test/startCluster.test.ts index 65c57e28b..b5fbfb38a 100644 --- a/packages/databricks-vscode/src/test/startCluster.test.ts +++ b/packages/databricks-vscode/src/test/startCluster.test.ts @@ -111,6 +111,9 @@ describe(__filename, function () { const startPromise = startCluster(instance(mockedClient), clusterId); await fakeTimer.runToLastAsync(); await startPromise; + + // The raced start() was attempted, and polling still reached RUNNING. + verifyStarted(1); }); it("propagates a non-race start error when the cluster stays stopped", async () => { From 1b96ee0bd81db61eeed2e31df7be8f6a889e4ae1 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 21 Aug 2026 20:30:59 +0300 Subject: [PATCH 7/8] test(e2e): don't quote a plain state_message in the failure error state_message is a string and termination_reason is an object; stringify only the object so a plain message reads cleanly (no wrapping quotes). Round-6 review nit. Co-authored-by: Isaac --- .../src/test/startCluster.ts | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/databricks-vscode/src/test/startCluster.ts b/packages/databricks-vscode/src/test/startCluster.ts index b575c6420..29ca40b5b 100644 --- a/packages/databricks-vscode/src/test/startCluster.ts +++ b/packages/databricks-vscode/src/test/startCluster.ts @@ -100,16 +100,24 @@ export async function startCluster( return; case "TERMINATED": case "ERROR": - case "UNKNOWN": + case "UNKNOWN": { + // state_message is a string; termination_reason is an + // object — stringify only the latter so a plain message + // isn't wrapped in quotes. + const reason = + cluster.state_message ?? + cluster.termination_reason ?? + "unknown reason"; throw new ClusterStartError( `Cluster ${clusterId} failed to start (${ cluster.state - }): ${JSON.stringify( - cluster.state_message ?? - cluster.termination_reason ?? - "unknown reason" - )}` + }): ${ + typeof reason === "string" + ? reason + : JSON.stringify(reason) + }` ); + } default: throw new retries.RetriableError(); } From 67a7e2f4ad038f2ac869b9b92eeb572c76743a75 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Mon, 24 Aug 2026 09:24:17 +0300 Subject: [PATCH 8/8] test(e2e): trim the startCluster header comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cut the module comment to the load-bearing lines (§4b). Comment-only. Co-authored-by: Isaac --- packages/databricks-vscode/src/test/startCluster.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/databricks-vscode/src/test/startCluster.ts b/packages/databricks-vscode/src/test/startCluster.ts index 29ca40b5b..7f945321e 100644 --- a/packages/databricks-vscode/src/test/startCluster.ts +++ b/packages/databricks-vscode/src/test/startCluster.ts @@ -9,12 +9,9 @@ import { retries, } from "@databricks/sdk-experimental"; -// Warms up the shared e2e test cluster from wdio's onPrepare, before any spec -// or session runs (a throw here aborts the whole shard — specFileRetries can't -// recover it). Deliberately duplicates the SDK retry() idiom rather than -// reusing the production Cluster.start() so the e2e path can own a long -// timeout (the shared cluster's cloud node placement has been seen taking ~1h) -// and tolerate the shared-cluster start race, without changing production. +// Warms up the shared e2e cluster from onPrepare (a throw here aborts the +// shard). Owns a long timeout — node placement has been seen taking ~1h, past +// the SDK default — and tolerates the shared-cluster start race. export class ClusterStartError extends Error {} const DEFAULT_START_TIMEOUT = new Time(60, TimeUnits.minutes);