Repository navigation
Conversation
In deployment history shadow mode, a failed read of the recorded deployment (workspace lookup, GetDeployment, or parsing last_version_id) only warned and left the deployment ID empty. Deploy and destroy then took the first-deploy path and called CreateDeployment again, recording the run against a second deployment for the same bundle. When the read fails for an unknown reason (403, 5xx, unparseable last_version_id), skip history for this run through RecordsHistory, so deploy, destroy and telemetry all see it while the shadow marker and WAL mode stay unchanged. When the workspace lookup found the deployment, keep stamping its id on jobs and pipelines so the skipped run does not update them just to clear it. A node whose record is missing (404) is a half-created deployment and keeps taking the first-deploy path, which re-registers it. A destroy that skipped history warns that the record is kept. A failed delete still fails the apply in a skipped run, as it does when history is recorded. DMS-primary state is unchanged: a read failure there already fails the command. Co-authored-by: Isaac <no-reply@databricks.com>
Contributor
|
Commit: Regression test reportSelected 4 changed tests (limit=20, 0 not selected) 4 tests: regression=4 unreleased=0 coverage=0 failing=0
TestAccept/bundle/dms/read-failure/deploy/DMS=true: branch ✅ | main (7adc80d) ❌ | latest (v1.24.0) ❌main (7adc80d): latest (v1.24.0): TestAccept/bundle/dms/read-failure/destroy/DMS=true: branch ✅ | main (7adc80d) ❌ | latest (v1.24.0) ❌main (7adc80d): latest (v1.24.0): TestAccept/bundle/dms/read-failure/failed-delete/DMS=true: branch ✅ | main (7adc80d) ❌ | latest (v1.24.0) ❌main (7adc80d): latest (v1.24.0): TestAccept/bundle/dms/read-failure/not-found/DMS=true: branch ✅ | main (7adc80d) ❌ | latest (v1.24.0) ❌main (7adc80d): latest (v1.24.0): |
Collaborator
Integration test reportCommit: 5e15578
Top 3 slowest tests (at least 2 minutes):
|
acceptance/bundle/dms/read-failure ran six steps in one script, each depending on the state the previous one left, so it was hard to tell which behavior a given output line pinned. Split it into four tests that each start from their own recorded deployment (setup output goes to LOG.setup and its requests are drained): - deploy: plan and deploy with a failed read send only the GET, keep the job's deployment_id stamp, and the next deploy resumes on the same deployment. - not-found: a 404 (half-created deployment) re-registers the deployment and records history. - failed-delete: a failed job delete in a run with a failed read still fails the deploy and keeps the job in state. - destroy: a destroy with a failed read records nothing and keeps the deployment record. The shared setup and fault helpers live in read-failure/script.prepare, and read-failure/test.toml keeps the tests local-only. Co-authored-by: Isaac <no-reply@databricks.com>
This branch has not been deployed
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
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
In deployment-history shadow mode, if reading the existing deployment from DMS fails, the CLI warned and continued with no deployment ID, so deploy and destroy treated the bundle as a first deploy and called
CreateDeploymentagain: history went to a new deployment, jobs were stamped with the new ID, and destroy created and then deleted a fresh record.Now a read failure of unknown cause (403, 5xx, unparseable
last_version_id, failed workspace lookup) skips history for that run, which matches the documented shadow-mode contract that history errors only warn:GetDeployment(node present, record missing) still takes the existing "no deployment yet" path, which re-registers it.bundle plandoesn't show a spurious update.RecordsHistory(), so telemetry and the deploy/destroy gates see it; a failed delete still fails the apply, as in a normal shadow run.Open questions: when even the workspace lookup fails, jobs lose their deployment stamp for that run (fall back to resource state, or fail?); a skipped destroy leaves the DMS record orphaned; telemetry reports
deployment_history_enabled=falsefor a skipped run.Why
Bug in the best-effort (shadow) deployment history mode added in #7169. The mode is experimental and has no changelog entry, so this PR doesn't add one; happy to add one if we think opted-in users should hear about it.
Tests
acceptance/bundle/dms/read-failure/(local only, faults injected withfault.py), one per scenario:deploy(403 on plan and deploy, the job's deployment stamp, and the resuming deploy),not-found(a half-created deployment recovers),failed-delete(a failed job delete still fails the deploy), anddestroy(the record is kept). Each fails without the fix.TestSkipHistoryKeepsShadowMarker.go test ./cmd/bundle/utils/... ./bundle/direct/... ./bundle/phases/...; acceptancebundle/(dms|deploy|destroy|templates/deployment-history|migrate/auto/dms-migration|state|telemetry);task fmt-q/lint-q.This pull request and its description were written by Isaac.