Skip shadow deployment history when the deployment read fails

acceptance/bundle/dms/read-failure/deploy/databricks.yml.tmpl

@@ -0,0 +1,9 @@
+bundle:
+  name: dms-read-failure-$UNIQUE_NAME
+experimental:
+  deployment_history: true
+
+resources:
+  jobs:
+    foo:
+      name: foo

acceptance/bundle/dms/read-failure/deploy/out.test.toml

@@ -0,0 +1,2 @@
+Cloud = false
+EnvMatrix.DMS = ["true"]

acceptance/bundle/dms/read-failure/deploy/output.txt

@@ -0,0 +1,52 @@
+
+=== Plan with a failed read: the job keeps the deployment id the lookup found, so nothing changes
+>>> fault.py GET /api/2.0/bundle/deployments/* 403 0 1 PERMISSION_DENIED
+
+>>> [CLI] bundle plan
+Warn: Could not read deployment history, not recording it for this run: Fault injected by test.
+Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged
+
+=== Deploy with a failed read: no history is recorded, so no second deployment is created, and the job keeps its deployment id
+>>> update_file.py databricks.yml name: foo name: renamed
+
+>>> fault.py GET /api/2.0/bundle/deployments/* 403 0 1 PERMISSION_DENIED
+
+>>> [CLI] bundle deploy
+Warn: Could not read deployment history, not recording it for this run: Fault injected by test.
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/dms-read-failure-[UNIQUE_NAME]/default/files...
+Updated jobs.foo
+Files: 2 uploaded, 0 deleted
+Resources: 0 created, 1 changed, 0 deleted, 0 unchanged
+
+>>> print_requests.py --dms //api/2.0/bundle --get --oneline
+{"method": "GET", "path": "/api/2.0/bundle/deployments/[DEPLOYMENT_ID]"}
+
+>>> [CLI] jobs get [FOO_ID]
+{
+  "deployment_id": "[DEPLOYMENT_ID]",
+  "kind": "BUNDLE",
+  "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/dms-read-failure-[UNIQUE_NAME]/default/state/metadata.json"
+}
+
+=== The next deploy resumes history on the same deployment and stamps the new version
+>>> update_file.py databricks.yml name: renamed name: resumed
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/dms-read-failure-[UNIQUE_NAME]/default/files...
+Updated jobs.foo
+Files: 1 uploaded, 0 deleted
+Resources: 0 created, 1 changed, 0 deleted, 0 unchanged
+
+>>> print_requests.py --dms //api/2.0/bundle --get --oneline
+{"method": "GET", "path": "/api/2.0/bundle/deployments/[DEPLOYMENT_ID]"}
+{"method": "POST", "path": "/api/2.0/bundle/deployments/[DEPLOYMENT_ID]/versions", "q": {"version_id": "3"}, "body": {"cli_version": "[CLI_VERSION]", "operations": [{"action_type": "OPERATION_ACTION_TYPE_UPDATE", "resource_key": "jobs.foo"}], "previous_version_id": "1", "version_type": "VERSION_TYPE_DEPLOY"}}
+{"method": "PATCH", "path": "/api/2.0/bundle/deployments/[DEPLOYMENT_ID]/versions/3/operations/jobs.foo", "q": {"update_mask": "state,error_message,resource_id,status"}, "body": {"error_message": "", "resource_id": "[FOO_ID]", "sequence_id": 0, "state": "{\"state\":{\"deployment\":{\"deployment_id\":\"[DEPLOYMENT_ID]\",\"kind\":\"BUNDLE\",\"metadata_file_path\":\"/Workspace/Users/[USERNAME]/.bundle/dms-read-failure-[UNIQUE_NAME]/default/state/metadata.json\",\"version_id\":\"3\"},\"edit_mode\":\"UI_LOCKED\",\"format\":\"MULTI_TASK\",\"max_concurrent_runs\":1,\"name\":\"resumed\",\"queue\":{\"enabled\":true}}}", "status": "OPERATION_STATUS_SUCCEEDED"}}
+{"method": "POST", "path": "/api/2.0/bundle/deployments/[DEPLOYMENT_ID]/versions/3/complete", "body": {"completion_reason": "VERSION_COMPLETE_SUCCESS"}}
+
+>>> [CLI] jobs get [FOO_ID]
+{
+  "deployment_id": "[DEPLOYMENT_ID]",
+  "kind": "BUNDLE",
+  "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/dms-read-failure-[UNIQUE_NAME]/default/state/metadata.json",
+  "version_id": "3"
+}

acceptance/bundle/dms/read-failure/deploy/script

@@ -0,0 +1,19 @@
+deploy_recorded foo
+
+title "Plan with a failed read: the job keeps the deployment id the lookup found, so nothing changes"
+fail_deployment_read
+trace $CLI bundle plan
+rm -f "$OUT_REQUESTS"
+
+title "Deploy with a failed read: no history is recorded, so no second deployment is created, and the job keeps its deployment id"
+trace update_file.py databricks.yml 'name: foo' 'name: renamed'
+fail_deployment_read
+trace $CLI bundle deploy
+trace print_requests.py --dms //api/2.0/bundle --get --oneline
+trace $CLI jobs get "$foo_id" | jq .settings.deployment
+
+title "The next deploy resumes history on the same deployment and stamps the new version"
+trace update_file.py databricks.yml 'name: renamed' 'name: resumed'
+trace $CLI bundle deploy
+trace print_requests.py --dms //api/2.0/bundle --get --oneline
+trace $CLI jobs get "$foo_id" | jq .settings.deployment

acceptance/bundle/dms/read-failure/destroy/databricks.yml.tmpl

@@ -0,0 +1,9 @@
+bundle:
+  name: dms-read-failure-$UNIQUE_NAME
+experimental:
+  deployment_history: true
+
+resources:
+  jobs:
+    foo:
+      name: foo

acceptance/bundle/dms/read-failure/destroy/out.test.toml

@@ -0,0 +1,2 @@
+Cloud = false
+EnvMatrix.DMS = ["true"]

acceptance/bundle/dms/read-failure/destroy/output.txt

@@ -0,0 +1,20 @@
+
+=== Destroy with a failed read: no history is recorded, so no second deployment is created and then deleted
+>>> fault.py GET /api/2.0/bundle/deployments/* 403 0 1 PERMISSION_DENIED
+
+>>> [CLI] bundle destroy --auto-approve
+Warn: Could not read deployment history, not recording it for this run: Fault injected by test.
+The following resources will be deleted:
+  delete resources.jobs.foo
+
+All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/dms-read-failure-[UNIQUE_NAME]/default
+
+Warn: Keeping the deployment history record, if any: it could not be read, so this destroy does not delete it
+Destroy: 1 deleted
+
+>>> print_requests.py --dms //api/2.0/bundle --get --oneline
+{"method": "GET", "path": "/api/2.0/bundle/deployments/[DEPLOYMENT_ID]"}
+
+=== The deployment record is kept
+>>> MSYS_NO_PATHCONV=1 [CLI] bundle-deployments get-deployment deployments/[DEPLOYMENT_ID] -o json
+{"last_version_id":"1"}

acceptance/bundle/dms/read-failure/destroy/script

@@ -0,0 +1,10 @@
+deploy_recorded
+
+title "Destroy with a failed read: no history is recorded, so no second deployment is created and then deleted"
+fail_deployment_read
+trace $CLI bundle destroy --auto-approve
+trace print_requests.py --dms //api/2.0/bundle --get --oneline
+
+title "The deployment record is kept"
+trace MSYS_NO_PATHCONV=1 $CLI bundle-deployments get-deployment "deployments/${deployment_id}" -o json | jq -c '{last_version_id}'
+rm -f "$OUT_REQUESTS"

acceptance/bundle/dms/read-failure/failed-delete/databricks.yml.tmpl

@@ -0,0 +1,10 @@
+bundle:
+  name: dms-read-failure-$UNIQUE_NAME
+experimental:
+  deployment_history: true
+
+resources:
+  jobs:
+    foo:
+      name: foo
+    bar: {name: bar}

acceptance/bundle/dms/read-failure/failed-delete/out.test.toml

@@ -0,0 +1,2 @@
+Cloud = false
+EnvMatrix.DMS = ["true"]

acceptance/bundle/dms/read-failure/failed-delete/output.txt

@@ -0,0 +1,30 @@
+
+=== A delete that fails in a run with a failed read still fails the deploy and keeps the job in state
+>>> update_file.py databricks.yml bar: {name: bar} 
+
+>>> fault.py GET /api/2.0/bundle/deployments/* 403 0 1 PERMISSION_DENIED
+
+>>> fault.py POST /api/2.2/jobs/delete 400 0 1 INVALID_PARAMETER_VALUE
+
+>>> musterr [CLI] bundle deploy
+Warn: Could not read deployment history, not recording it for this run: Fault injected by test.
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/dms-read-failure-[UNIQUE_NAME]/default/files...
+Error: cannot delete resources.jobs.bar: deleting id=[BAR_ID]: Fault injected by test. (400 INVALID_PARAMETER_VALUE)
+
+Endpoint: POST [DATABRICKS_URL]/api/2.2/jobs/delete
+HTTP Status: 400 Bad Request
+API error_code: INVALID_PARAMETER_VALUE
+API message: Fault injected by test.
+
+Files: 2 uploaded, 0 deleted
+
+>>> print_requests.py --dms //api/2.0/bundle --get --oneline
+{"method": "GET", "path": "/api/2.0/bundle/deployments/[DEPLOYMENT_ID]"}
+
+>>> [CLI] jobs get [BAR_ID]
+"bar"
+
+>>> [CLI] bundle plan
+delete jobs.bar
+
+Plan: 0 to add, 0 to change, 1 to delete, 1 unchanged

acceptance/bundle/dms/read-failure/failed-delete/script

@@ -0,0 +1,11 @@
+deploy_recorded bar
+
+title "A delete that fails in a run with a failed read still fails the deploy and keeps the job in state"
+trace update_file.py databricks.yml 'bar: {name: bar}' ''
+fail_deployment_read
+trace fault.py "POST /api/2.2/jobs/delete" 400 0 1 INVALID_PARAMETER_VALUE
+trace musterr $CLI bundle deploy
+trace print_requests.py --dms //api/2.0/bundle --get --oneline
+trace $CLI jobs get "$bar_id" | jq .settings.name
+trace $CLI bundle plan
+rm -f "$OUT_REQUESTS"

acceptance/bundle/dms/read-failure/not-found/databricks.yml.tmpl

@@ -0,0 +1,9 @@
+bundle:
+  name: dms-read-failure-$UNIQUE_NAME
+experimental:
+  deployment_history: true
+
+resources:
+  jobs:
+    foo:
+      name: foo

acceptance/bundle/dms/read-failure/not-found/out.test.toml

@@ -0,0 +1,2 @@
+Cloud = false
+EnvMatrix.DMS = ["true"]

acceptance/bundle/dms/read-failure/not-found/output.txt

@@ -0,0 +1,18 @@
+
+=== A deployment node without a record (404) is a half-created deployment: the deploy registers it again and records history
+>>> update_file.py databricks.yml name: foo name: renamed
+
+>>> fault.py GET /api/2.0/bundle/deployments/* 404 0 1 NOT_FOUND
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/dms-read-failure-[UNIQUE_NAME]/default/files...
+Updated jobs.foo
+Files: 2 uploaded, 0 deleted
+Resources: 0 created, 1 changed, 0 deleted, 0 unchanged
+
+>>> print_requests.py --dms //api/2.0/bundle --get --oneline
+{"method": "GET", "path": "/api/2.0/bundle/deployments/[DEPLOYMENT_ID]"}
+{"method": "POST", "path": "/api/2.0/bundle/deployments", "body": {"display_name": "dms-read-failure-[UNIQUE_NAME]", "initial_parent_path": "/Workspace/Users/[USERNAME]/.bundle/dms-read-failure-[UNIQUE_NAME]/default/state", "target_name": "default", "workspace_info": {"file_path": "/Workspace/Users/[USERNAME]/.bundle/dms-read-failure-[UNIQUE_NAME]/default/files", "root_path": "/Workspace/Users/[USERNAME]/.bundle/dms-read-failure-[UNIQUE_NAME]/default"}}}
+{"method": "POST", "path": "/api/2.0/bundle/deployments/[NEW_DEPLOYMENT_ID]/versions", "q": {"version_id": "2"}, "body": {"cli_version": "[CLI_VERSION]", "operations": [{"action_type": "OPERATION_ACTION_TYPE_UPDATE", "resource_key": "jobs.foo"}], "version_type": "VERSION_TYPE_DEPLOY"}}
+{"method": "PATCH", "path": "/api/2.0/bundle/deployments/[NEW_DEPLOYMENT_ID]/versions/2/operations/jobs.foo", "q": {"update_mask": "state,error_message,resource_id,status"}, "body": {"error_message": "", "resource_id": "[FOO_ID]", "sequence_id": 0, "state": "{\"state\":{\"deployment\":{\"deployment_id\":\"[NEW_DEPLOYMENT_ID]\",\"kind\":\"BUNDLE\",\"metadata_file_path\":\"/Workspace/Users/[USERNAME]/.bundle/dms-read-failure-[UNIQUE_NAME]/default/state/metadata.json\",\"version_id\":\"2\"},\"edit_mode\":\"UI_LOCKED\",\"format\":\"MULTI_TASK\",\"max_concurrent_runs\":1,\"name\":\"renamed\",\"queue\":{\"enabled\":true}}}", "status": "OPERATION_STATUS_SUCCEEDED"}}
+{"method": "POST", "path": "/api/2.0/bundle/deployments/[NEW_DEPLOYMENT_ID]/versions/2/complete", "body": {"completion_reason": "VERSION_COMPLETE_SUCCESS"}}

acceptance/bundle/dms/read-failure/not-found/script

@@ -0,0 +1,9 @@
+deploy_recorded foo
+
+title "A deployment node without a record (404) is a half-created deployment: the deploy registers it again and records history"
+trace update_file.py databricks.yml 'name: foo' 'name: renamed'
+trace fault.py "GET /api/2.0/bundle/deployments/*" 404 0 1 NOT_FOUND
+trace $CLI bundle deploy
+# The fake registers a new node, so the deployment gets a new id.
+add_repl "$(deployment_node_id)" NEW_DEPLOYMENT_ID
+trace print_requests.py --dms //api/2.0/bundle --get --oneline

acceptance/bundle/dms/read-failure/script.prepare

@@ -0,0 +1,25 @@
+# Each test starts from a recorded deployment (version 1). The setup output is in
+# LOG.setup and its requests are drained, so output.txt shows only the step under test.
+# Sets deployment_id, and <name>_id for each job name passed.
+deploy_recorded() {
+    envsubst < databricks.yml.tmpl > databricks.yml
+    $CLI bundle deploy &> LOG.setup
+    deployment_id=$(deployment_node_id)
+    add_repl "$deployment_id" DEPLOYMENT_ID
+    local name
+    for name in "$@"; do
+        printf -v "${name}_id" '%s' "$(read_id.py "$name")"
+    done
+    rm -f "$OUT_REQUESTS"
+}
+
+# The deployment id is the id of the deployment's workspace node. Extracted with
+# python, not jq: the id exceeds 2^53 and jq 1.6 rounds it.
+deployment_node_id() {
+    MSYS_NO_PATHCONV=1 $CLI workspace get-status "/Workspace/Users/${CURRENT_USER_NAME}/.bundle/dms-read-failure-${UNIQUE_NAME}/default/state/resources.deployment.json" | python3 -c 'import sys, json; print(json.load(sys.stdin)["object_id"])'
+}
+
+# A 403 is a read failure of unknown cause, and it is not retried.
+fail_deployment_read() {
+    trace fault.py "GET /api/2.0/bundle/deployments/*" 403 0 1 PERMISSION_DENIED
+}

acceptance/bundle/dms/read-failure/test.toml

@@ -0,0 +1,2 @@
+# Local only: fault.py registers its rule on the fake server.
+Cloud = false

bundle/direct/apply.go

@@ -265,8 +265,9 @@ func (d *DeploymentUnit) Delete(ctx context.Context, db *dstate.DeploymentState,
 			log.Warnf(ctx, "Ignoring permission error when deleting %s id=%s: %s", d.ResourceKey, oldID, err)
 		} else if d.deleteConfirmedGone(ctx, oldID) {
 			log.Warnf(ctx, "Treating %s id=%s as already deleted despite delete error: %s", d.ResourceKey, oldID, err)
-		} else if db.RecordsHistory() {
-			// When using the deployment metadata service, record the error.
+		} else if db.RecordsHistory() || db.HistorySkipped() {
+			// When using the deployment metadata service, record the error. A run that skips history
+			// after a failed DMS read still fails here, so a DMS outage does not change delete behavior.
 			err = fmt.Errorf("deleting id=%s: %w", oldID, err)
 			db.RecordFailure(d.ResourceKey, oldID, err)
 			return err

bundle/direct/dstate/state.go

@@ -119,6 +119,11 @@ type DeploymentState struct {
 	// resources.json as Serial to ensure outdated resource state is not read.
 	// Open always takes the authoritative value from the service.
 	VersionID int
+
+	// historySkipped turns off history recording for this run without touching the mode, so the
+	// shadow marker is still written. Kept across reset, like mode, so telemetry reads it after
+	// Finalize.
+	historySkipped bool
 }
 
 // OpenDmsArgs identifies the recorded deployment Open reads from. The zero value means the
@@ -475,10 +480,29 @@ func (db *DeploymentState) IsDeploymentMetadataService() bool {
 	return db.isDeploymentMetadataService()
 }
 
+// SkipHistory stops this run from recording deployment history. Shadow mode calls it when the
+// recorded deployment cannot be read: without its id and version, recording would create a second
+// deployment for the bundle.
+func (db *DeploymentState) SkipHistory() {
+	db.mu.Lock()
+	defer db.mu.Unlock()
+	db.historySkipped = true
+}
+
+// HistorySkipped reports whether SkipHistory was called since the state was opened.
+func (db *DeploymentState) HistorySkipped() bool {
+	db.mu.Lock()
+	defer db.mu.Unlock()
+	return db.historySkipped
+}
+
 // RecordsHistory reports whether the loaded file state or selected write mode records DMS history.
 func (db *DeploymentState) RecordsHistory() bool {
 	db.mu.Lock()
 	defer db.mu.Unlock()
+	if db.historySkipped {
+		return false
+	}
 	switch db.mode {
 	case ReadingFile:
 		_, history := db.Data.Features[FeatureDeploymentHistoryShadow]
@@ -570,6 +594,7 @@ func (db *DeploymentState) reset() {
 func (db *DeploymentState) unlockedOpen(ctx context.Context, path string, mode Mode, withRecovery WithRecovery, dmsDeployment OpenDmsArgs) error {
 	db.Path = path
 	db.mode = mode
+	db.historySkipped = false
 	db.DeploymentID = dmsDeployment.DeploymentID
 	db.VersionID = dmsDeployment.LastVersionID
 
@@ -701,6 +726,7 @@ func (db *DeploymentState) OpenWithData(path string, data Database) {
 
 	db.Path = path
 	db.mode = ReadingFile
+	db.historySkipped = false
 	db.Data = data
 	db.stateIDs = make(map[string]string)
 	for key, entry := range data.State {

bundle/direct/dstate/state_test.go

@@ -89,6 +89,28 @@ func TestFinalizeWithoutResourceWrites(t *testing.T) {
 	}
 }
 
+func TestSkipHistoryKeepsShadowMarker(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "state.json")
+	var db DeploymentState
+	require.NoError(t, db.Open(t.Context(), path, ReadingFile, WithRecovery(true), OpenDmsArgs{}))
+	db.Data.Features = map[string]struct{}{FeatureDeploymentHistoryShadow: {}}
+	require.True(t, db.RecordsHistory())
+
+	db.SkipHistory()
+	assert.False(t, db.RecordsHistory())
+	require.NoError(t, db.UpgradeToWrite())
+	mustFinalize(t, &db)
+	// Telemetry reads it after Finalize.
+	assert.False(t, db.RecordsHistory())
+
+	// The skip covers one run: the marker is persisted, and the next Open records history again.
+	require.NoError(t, db.Open(t.Context(), path, ReadingFile, WithRecovery(false), OpenDmsArgs{}))
+	assert.False(t, db.HistorySkipped())
+	assert.True(t, db.RecordsHistory())
+	assert.Contains(t, db.StateFeatures(), FeatureDeploymentHistoryShadow)
+	mustFinalize(t, &db)
+}
+
 func TestDataForFileSerial(t *testing.T) {
 	tests := []struct {
 		name       string

bundle/phases/destroy.go

@@ -374,6 +374,8 @@ func Destroy(ctx context.Context, b *bundle.Bundle) {
 				}
 				log.Warnf(ctx, "Could not record deployment history for this destroy: %s", err)
 			}
+		} else if b.DeploymentBundle.StateDB.HistorySkipped() {
+			log.Warnf(ctx, "Keeping the deployment history record, if any: it could not be read, so this destroy does not delete it")
 		}
 		destroyCore(ctx, b, plan)
 	} else {