Skip to content

ci: nightly E2E workflow for the Thrift and kernel backends#402

Merged
mani-mathur-arch merged 2 commits into
mani/sea-kernel-consolidatedfrom
mani/sea-kernel-nightly-e2e
Jul 15, 2026
Merged

ci: nightly E2E workflow for the Thrift and kernel backends#402
mani-mathur-arch merged 2 commits into
mani/sea-kernel-consolidatedfrom
mani/sea-kernel-nightly-e2e

Conversation

@mani-mathur-arch

@mani-mathur-arch mani-mathur-arch commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Context for reviewers

Adds a scheduled CI workflow that runs the credential-gated E2E suites against a real test SQL warehouse, for both the Thrift backend and the SEA-via-kernel backend (the alternative backend that talks to Databricks SQL through the Rust databricks-sql-kernel static lib over a C ABI, behind the databricks_kernel build tag).

Stack position:

#399  OAuth / namespace / metric-view / params / types / query-id   (base)
  └─ #400  richer TLS options
       └─ #401  kernel logging → driver log level
            └─ #402  nightly E2E workflow          ← THIS PR
                 └─ #403  CI dispatch label + CLOSE_STATEMENT telemetry

Reviewer note on the small test-file diffs: the changes to kernel_e2e_test.go / kernel_parity_test.go are credential-variable renames (see §Credentials), not behavior changes. The substance of the PR is the new nightly-e2e.yml.


What

A scheduled (07:00 UTC daily) + manually dispatchable workflow that runs the credential-gated end-to-end suites against a real test SQL warehouse. The ordinary "Go" workflow injects no warehouse secrets, so every E2E test t.Skip()s there and real-warehouse behaviour (large multi-page CloudFetch, S3 downloads, drain-past-deadline, real auth) is otherwise not exercised in CI.

Closes PECOBLR-3308.

Two jobs, one test warehouse

  • thrift-e2e — pure-Go (CGO_ENABLED=0); runs the high-value CloudFetch drain-past-caller-deadline + exact-row-count scenarios (-count=1, not -short, so the -short-guarded exact-row-count test runs).
  • kernel-e2e — reuses the go.yml kernel-build machinery (JFrog + cargo proxy, pinned Rust, kernel App-token clone, kernel-lib/cargo caches), then runs the databricks_kernel-tagged E2E funcs plus the Thrift-vs-kernel parity funcs (TestKernelThriftParity / TestKernelParamsVsThrift), which need both the kernel lib and live credentials.

Both jobs are gated on vars.NIGHTLY_E2E_ENABLED != 'false', so the workflow can be disabled via a repo variable without editing the file.

Credentials (one secret set)

Both suites now read the same DATABRICKS_PECOTESTING_* credentials, so a single secret set drives the whole workflow. Rather than map env-var names in the workflow, the kernel E2E / parity test helpers were renamed from the ad-hoc DATABRICKS_HOST/_HTTP_PATH/_TOKEN vars to the PECOTESTING set (with the _TOKEN_PERSONAL fallback the Thrift suite already uses). Each job has a fail-loud credential guard so a missing secret errors instead of silently skipping into a misleading green — a silent skip reporting green is exactly the gap this closes.

Live validation before merge (done — both jobs green, tests actually ran)

The workflow's only triggers are schedule + workflow_dispatch, both gated to the default branch, so a feature branch can't exercise it normally. A temporary push: trigger was added to validate against the real warehouse, then removed once validation passed (no temp trigger ships).

The first validation run failed at make kernel-lib because the pinned KERNEL_REV was orphaned when the kernel dependency PR force-pushed (the documented GC merge-gate actually firing). The whole stack was re-pinned to a fetchable kernel rev, and the re-run went green:

  • Thrift E2E: 2 PASS / 0 SKIP / 0 FAIL.
  • Kernel E2E: 46 PASS / 0 SKIP / 0 FAIL — incl. both parity funcs, CloudFetch, StatementID, cancellation, datatypes.

The 0-SKIP counts confirm the credentials wired through and the tests genuinely executed — the whole point of the ticket.

M2M kept skipped (decision)

TestKernelE2EM2M is excluded (-skip) — not a silent skip. It's the only test that needs a service-principal DATABRICKS_CLIENT_ID/_CLIENT_SECRET, and we don't add an SP credential to the public repo for one test when (a) the PAT already covers the real-warehouse path for both backends and (b) the M2M setter wiring is already unit-covered by TestSetAuthByMode. It runs locally for a developer who exports the two vars; the job comment documents how to enable it in CI.

Remaining step (repo admin — cannot be done from a PR)

Provision these repository secrets to make the nightly run green (the kernel job's App-token secrets already exist):

  • DATABRICKS_PECOTESTING_SERVER_HOSTNAME
  • DATABRICKS_PECOTESTING_HTTP_PATH2
  • DATABRICKS_PECOTESTING_TOKEN (or _TOKEN_PERSONAL)

Until then the credential-guard step fails by design (never a silent skip). A failed scheduled run surfaces via GitHub's built-in notification (no external webhook).

Testing

  • Both E2E suites SKIP cleanly with no creds (the safe local/PR default); the renames caused no regressions — default CGO_ENABLED=0 and tagged databricks_kernel suites both 24 pkg ok / 0 fail.
  • go test -list / -skip confirmed the kernel job selects all E2E + both parity funcs and excludes M2M.
  • Workflow YAML validated; Isaac Review clean (0 critical / 0 major).

Stacking

Stacked on #401 (mani/sea-kernel-log-unify).

This pull request and its description were written by Isaac.

@mani-mathur-arch

Copy link
Copy Markdown
Collaborator Author

One operational concern: the kernel E2E job has a 40-minute job timeout, but it first runs a potentially cold make kernel-lib Rust build and then gives go test its own -timeout 30m budget.

On a cache miss or slow kernel build, the build only needs to take more than ~10 minutes before GitHub can kill the job before Go's timeout produces useful diagnostics about which E2E test hung or failed. Could we either raise the job timeout to something like 60-70 minutes, or split the budget explicitly, e.g. a timeout around make kernel-lib plus a test timeout that fits inside the remaining job budget?

@mani-mathur-arch

Copy link
Copy Markdown
Collaborator Author

Fixed in 6376422. Good catch on the timeout math — the 40-min job cap minus go test's own -timeout 30m left only ~10 min for setup + a potentially cold ~200-crate make kernel-lib, so a slow build could get SIGKILLed at the job cap before Go's timeout could emit its which-test-hung goroutine dump.

Went with the explicit-split option you suggested:

  • Raised the job timeout-minutes 40 → 65.
  • Added a 25-min per-step timeout on make kernel-lib.

Worst case is now 25 (build) + 30 (go test) + setup < 65, so a wedged build is killed distinctly as a build-step failure, and a hung test still hits Go's own -timeout 30m first (preserving diagnostics). Also moved the E2E step's descriptive comment down onto the Run kernel E2E step it actually documents.


This comment was generated with GitHub MCP.

@mani-mathur-arch mani-mathur-arch marked this pull request as ready for review July 15, 2026 08:07
@mani-mathur-arch mani-mathur-arch force-pushed the mani/sea-kernel-nightly-e2e branch from 6376422 to 4512646 Compare July 15, 2026 08:46
@mani-mathur-arch mani-mathur-arch force-pushed the mani/sea-kernel-log-unify branch from ef7e5ec to 752e19b Compare July 15, 2026 09:19
@mani-mathur-arch mani-mathur-arch force-pushed the mani/sea-kernel-nightly-e2e branch from 4512646 to 091accc Compare July 15, 2026 09:20
@mani-mathur-arch mani-mathur-arch force-pushed the mani/sea-kernel-log-unify branch from 752e19b to 125eec7 Compare July 15, 2026 09:48
@mani-mathur-arch mani-mathur-arch force-pushed the mani/sea-kernel-nightly-e2e branch from 091accc to ac112d4 Compare July 15, 2026 09:49
Add a scheduled (07:00 UTC) + manually dispatchable workflow that runs the
credential-gated end-to-end suites against a real test SQL warehouse. The
ordinary "Go" workflow injects no warehouse secrets, so every E2E test t.Skip()s
there and real-warehouse behaviour — large multi-page CloudFetch, S3 downloads,
drain-past-deadline, real auth — is otherwise not exercised in CI.

Two jobs against one test warehouse:
  - thrift-e2e:  pure-Go (CGO_ENABLED=0), the high-value CloudFetch drain +
    exact-row-count scenarios.
  - kernel-e2e:  reuses the go.yml kernel-build machinery (JFrog/cargo proxy,
    pinned Rust, kernel App-token clone, kernel-lib/cargo caches), then runs the
    databricks_kernel-tagged E2E funcs plus the Thrift-vs-kernel parity funcs
    (which need both the kernel lib and live credentials).

Both suites now read the same DATABRICKS_PECOTESTING_* credentials, so one secret
set drives the whole workflow — the kernel E2E / parity test helpers are updated
from the ad-hoc DATABRICKS_HOST/_HTTP_PATH/_TOKEN vars to the PECOTESTING set
(with the _TOKEN_PERSONAL fallback the Thrift suite already uses). Each job has a
fail-loud credential guard so a missing secret errors instead of silently
skipping into a misleading green. TestKernelE2EM2M is excluded (-skip) since it
needs a service-principal client id/secret the warehouse secret set doesn't
carry; the job comment documents how to enable it.

Alerting is GitHub's built-in failed-scheduled-run notification. Provisioning the
DATABRICKS_PECOTESTING_* secrets in the repository is the remaining step to make
the nightly run green.

Closes PECOBLR-3308.

Co-authored-by: Isaac
Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
…fires first

The kernel E2E job had a 40-min job cap while `go test` alone was given
-timeout 30m, leaving only ~10 min for checkout, toolchains, caches, and a
potentially cold ~200-crate `make kernel-lib` Rust build. On a cache miss the
build can exceed that, so GitHub SIGKILLs the whole job at 40 min before Go's
timeout can emit its which-test-hung goroutine dump — losing the diagnostics.

Split the budget explicitly: raise the job cap to 65 min and add a 25-min
per-step timeout on `make kernel-lib`. Worst case is 25 (build) + 30 (go test)
+ setup < 65, so a wedged build is killed distinctly as a build failure and a
hung test still hits Go's own -timeout first. Also moved the E2E step's
descriptive comment down onto the Run step it documents.

Co-authored-by: Isaac
@mani-mathur-arch mani-mathur-arch force-pushed the mani/sea-kernel-log-unify branch from 125eec7 to aa27368 Compare July 15, 2026 12:40
@mani-mathur-arch mani-mathur-arch force-pushed the mani/sea-kernel-nightly-e2e branch from ac112d4 to 0c68dfb Compare July 15, 2026 12:40
Base automatically changed from mani/sea-kernel-log-unify to mani/sea-kernel-consolidated July 15, 2026 13:48
@mani-mathur-arch mani-mathur-arch merged commit 0c68dfb into mani/sea-kernel-consolidated Jul 15, 2026
5 checks passed
@mani-mathur-arch mani-mathur-arch deleted the mani/sea-kernel-nightly-e2e branch July 15, 2026 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant