job_runs: plan skip for in-progress runs and trigger removal

.nextchanges/bundles/job-runs-skip-in-progress.md

@@ -0,0 +1 @@
+`job_runs` now plans skip for a run that is still in progress and when `on_bundle_deploy` is removed, instead of treating either case as an update. ([#6357](https://github.com/databricks/cli/pull/6357))

acceptance/bundle/resources/job_runs/interrupted_run/output.txt

@@ -30,7 +30,7 @@ Exit code: 1
   }
 }
 
-=== a run that is still going is an update
+=== a run that is still going is skipped
 >>> jq .plan["resources.job_runs.my_run"] tmp.plan.json
 {
   "depends_on": [
@@ -39,13 +39,7 @@ Exit code: 1
       "label": "${resources.jobs.my_job.id}"
     }
   ],
-  "action": "update",
-  "new_state": {
-    "value": {
-      "job_id": [MY_JOB_ID],
-      "result_state": "SUCCESS"
-    }
-  },
+  "action": "skip",
   "remote_state": {
     "job_id": [MY_JOB_ID],
     "run_id": [MY_RUN_ID],
@@ -58,23 +52,20 @@ Exit code: 1
   },
   "changes": {
     "result_state": {
-      "action": "update",
+      "action": "skip",
       "reason": "run in progress",
       "new": "SUCCESS"
     }
   }
 }
 
-=== the deploy adopts that run and waits for it to finish
+=== Badness: the next deploy does not wait for that run
 >>> [CLI] bundle deploy --plan tmp.plan.json
 Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/job-runs-interrupted-run/default/files...
-Output from job_runs.my_run: id=[MY_RUN_ID]: Run URL: [DATABRICKS_URL]/jobs/[MY_JOB_ID]/runs/[MY_RUN_ID]?o=[NUMID]
-Output from job_runs.my_run: id=[MY_RUN_ID]: SUCCESS
-Updated job_runs.my_run
 Files: 3 uploaded, 0 deleted
-Resources: 0 created, 1 changed, 0 deleted, 1 unchanged
+Resources: 0 created, 0 changed, 0 deleted, 2 unchanged
 
-=== the same run finished, and nothing was cancelled, deleted or triggered
+=== the same run is still tracked, and nothing was cancelled, deleted or triggered
 >>> read_state.py job_runs my_run id result_state
 job_runs my_run id='[MY_RUN_ID]' result_state=None
 

acceptance/bundle/resources/job_runs/interrupted_run/script

@@ -17,15 +17,15 @@ trace read_id.py my_job
 trace read_id.py my_run
 trace print_requests.py //jobs/run-now
 
-title "a run that is still going is an update"
+title "a run that is still going is skipped"
 $CLI bundle plan -o json > tmp.plan.json
 trace jq '.plan["resources.job_runs.my_run"]' tmp.plan.json
 
-# The testserver settles a run on its first poll, so the plan above is replayed
-# here: the second read of the run reports it finished.
-title "the deploy adopts that run and waits for it to finish"
+# Replay the plan captured while the run was still going: a fresh plan would
+# re-read it, and the testserver would settle it on that GET.
+title "Badness: the next deploy does not wait for that run"
 trace $CLI bundle deploy --plan tmp.plan.json
 
-title "the same run finished, and nothing was cancelled, deleted or triggered"
+title "the same run is still tracked, and nothing was cancelled, deleted or triggered"
 trace read_state.py job_runs my_run id result_state
 trace print_requests.py //jobs

acceptance/bundle/resources/job_runs/interrupted_run/test.toml

@@ -1,3 +1,4 @@
 # The interruption is staged by injecting a fault into the testserver, so this
 # stays off cloud.
+Badness = "An in-progress run is planned as skip, so a deploy after an interrupted wait does not resume waiting for it to finish."
 Ignore = ["tmp.plan.json"]

acceptance/bundle/resources/job_runs/on_bundle_deploy/output.txt

@@ -102,16 +102,14 @@ Resources:
   }
 }
 
-=== removing on_bundle_deploy rewrites state without a run
+=== removing on_bundle_deploy is unchanged and does not run
 >>> [CLI] bundle plan
-update job_runs.my_run
-
-Plan: 0 to add, 1 to change, 0 to delete, 1 unchanged
+Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged
 
 >>> jq .plan["resources.job_runs.my_run"].changes tmp.plan.json
 {
   "lifecycle": {
-    "action": "update",
+    "action": "skip",
     "reason": "trigger removed",
     "old": {
       "triggers": {
@@ -129,9 +127,8 @@ Plan: 0 to add, 1 to change, 0 to delete, 1 unchanged
 
 === bundle deploy
 Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/job-runs-on-bundle-deploy/default/files...
-Updated job_runs.my_run
 Files: 4 uploaded, 0 deleted
-Resources: 0 created, 1 changed, 0 deleted, 1 unchanged
+Resources: 0 created, 0 changed, 0 deleted, 2 unchanged
 
 >>> print_requests.py //jobs/run-now
 

acceptance/bundle/resources/job_runs/on_bundle_deploy/script

@@ -26,9 +26,9 @@ title "second run-now after recreate"
 trace print_requests.py --keep //jobs/runs/delete
 trace print_requests.py //jobs/run-now
 
-title "removing on_bundle_deploy rewrites state without a run"
+title "removing on_bundle_deploy is unchanged and does not run"
 # Drop the lifecycle block so the next plan compares a nil fingerprint against
-# the UUID left in state: a state-only update that clears it, never a recreate.
+# the UUID left in state. The removed trigger is skipped, never recreated.
 update_file.py databricks.yml "      lifecycle:
         triggers:
           - on_bundle_deploy: true

bundle/direct/bundle_plan_test.go

@@ -353,9 +353,9 @@ func TestJobRunFinishedWithoutSuccessIsRecreate(t *testing.T) {
 	}
 }
 
-// A run that has not stopped yet may still succeed, so the deploy adopts it and
-// waits for it.
-func TestJobRunInProgressIsUpdate(t *testing.T) {
+// A run that has not stopped yet may still succeed, so the plan leaves it
+// alone rather than recreating it. Skip does not resume an abandoned wait.
+func TestJobRunInProgressIsSkip(t *testing.T) {
 	for _, lifeCycleState := range []jobs.RunLifeCycleState{
 		jobs.RunLifeCycleStatePending,
 		jobs.RunLifeCycleStateRunning,
@@ -364,7 +364,7 @@ func TestJobRunInProgressIsUpdate(t *testing.T) {
 		t.Run(string(lifeCycleState), func(t *testing.T) {
 			change := jobRunResultStateAction(t, &jobs.RunState{LifeCycleState: lifeCycleState})
 
-			assert.Equal(t, deployplan.Update, change.Action)
+			assert.Equal(t, deployplan.Skip, change.Action)
 			assert.Equal(t, "run in progress", change.Reason)
 		})
 	}

bundle/direct/dresources/job_run.go

@@ -365,33 +365,18 @@ func reportRunLine(ctx context.Context, runID int64, msg string) {
 	cmdio.LogString(ctx, fmt.Sprintf("Output from %s: id=%d: %s", ResourceKey(ctx), runID, msg))
 }
 
-// DoUpdate rewrites state after a cleared trigger (no API call) or finishes the
-// wait an interrupted deploy abandoned.
-func (r *ResourceJobRun) DoUpdate(ctx context.Context, id string, config *JobRunState, entry *PlanEntry) (*JobRunRemote, error) {
-	// Clearing a trigger only drops its local-only fingerprint from state; wait on
-	// the run only when some other field changed.
-	if !entry.Changes.HasChangeExcept("lifecycle", "lifecycle.triggers", "lifecycle.triggers.on_bundle_deploy") {
-		config.ResultState = ""
-		return nil, nil
-	}
-	remote, err := r.waitForRun(ctx, id)
-	config.ResultState = ""
-	return remote, err
-}
-
-// OverrideChangeDesc downgrades result_state drift to an update while the run is
-// still going, so a run that may yet succeed is adopted and waited on. A run that
+// OverrideChangeDesc downgrades result_state drift to skip while the run is
+// still going, so a run that may yet succeed is not recreated. A run that
 // stopped without succeeding keeps its recreate. A SKIPPED run reports no
 // result_state either, so the lifecycle state is what tells the two apart.
-// Clearing a trigger downgrades the recreate to a state-only update so the
-// fingerprint is dropped from state without re-firing the run.
+// Clearing a trigger skips its local-only fingerprint without re-firing the run.
 func (*ResourceJobRun) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, remote *JobRunRemote) error {
 	switch path.String() {
 	case "lifecycle", "lifecycle.triggers", "lifecycle.triggers.on_bundle_deploy":
 		// A cleared trigger sets New empty; structdiff may report it at lifecycle,
-		// lifecycle.triggers, or the leaf. DoUpdate treats these paths as no-ops.
+		// lifecycle.triggers, or the leaf.
 		if change.New == nil || change.New == "" {
-			change.Action = deployplan.Update
+			change.Action = deployplan.Skip
 			change.Reason = "trigger removed"
 		}
 		return nil
@@ -400,7 +385,7 @@ func (*ResourceJobRun) OverrideChangeDesc(_ context.Context, path *structpath.Pa
 		if remote == nil || runIsTerminal(remote.State.LifeCycleState) {
 			return nil
 		}
-		change.Action = deployplan.Update
+		change.Action = deployplan.Skip
 		change.Reason = "run in progress"
 		return nil
 	default:

bundle/direct/dresources/job_run_test.go

@@ -379,25 +379,25 @@ func TestJobRunPrepareStateOnBundleDeploy(t *testing.T) {
 func TestJobRunOverrideChangeDescTriggerRemoved(t *testing.T) {
 	r := &ResourceJobRun{}
 
-	t.Run("clearing lifecycle downgrades to update", func(t *testing.T) {
+	t.Run("clearing lifecycle downgrades to skip", func(t *testing.T) {
 		change := &ChangeDesc{
 			Action: deployplan.Recreate,
 			Old:    &JobRunLifecycleState{Triggers: &JobRunTriggersState{OnBundleDeploy: "old"}},
 			New:    nil,
 		}
 		require.NoError(t, r.OverrideChangeDesc(t.Context(), structpath.MustParsePath("lifecycle"), change, nil))
-		assert.Equal(t, deployplan.Update, change.Action)
+		assert.Equal(t, deployplan.Skip, change.Action)
 		assert.Equal(t, "trigger removed", change.Reason)
 	})
 
-	t.Run("clearing on_bundle_deploy leaf downgrades to update", func(t *testing.T) {
+	t.Run("clearing on_bundle_deploy leaf downgrades to skip", func(t *testing.T) {
 		change := &ChangeDesc{
 			Action: deployplan.Recreate,
 			Old:    "old",
 			New:    "",
 		}
 		require.NoError(t, r.OverrideChangeDesc(t.Context(), structpath.MustParsePath("lifecycle.triggers.on_bundle_deploy"), change, nil))
-		assert.Equal(t, deployplan.Update, change.Action)
+		assert.Equal(t, deployplan.Skip, change.Action)
 		assert.Equal(t, "trigger removed", change.Reason)
 	})
 

bundle/direct/dresources/resources.yml

@@ -178,7 +178,7 @@ resources:
     # A run is immutable and fire-once, so any change recreates it. Omitting
     # `field` matches every field (root; see TestFieldRuleOmittedIsRoot).
     # `field: ""` would instead match nothing. The one exception is a run that is
-    # still going, which ResourceJobRun.OverrideChangeDesc downgrades to an update.
+    # still going, which ResourceJobRun.OverrideChangeDesc downgrades to skip.
     recreate_on_changes:
       - reason: immutable