diff --git a/.nextchanges/bundles/state-cli-version-last-writer.md b/.nextchanges/bundles/state-cli-version-last-writer.md new file mode 100644 index 00000000000..dc3881a710b --- /dev/null +++ b/.nextchanges/bundles/state-cli-version-last-writer.md @@ -0,0 +1 @@ +The `cli_version` field in the direct engine's deployment state (`resources.json`) now records the CLI version that last wrote the state. Previously it kept the version of the CLI that first created the state. diff --git a/acceptance/bundle/state/permission_level_migration/output.txt b/acceptance/bundle/state/permission_level_migration/output.txt index 2d6157efa8f..d6d2e3f71ca 100644 --- a/acceptance/bundle/state/permission_level_migration/output.txt +++ b/acceptance/bundle/state/permission_level_migration/output.txt @@ -13,7 +13,7 @@ Deployment complete! >>> print_state.py { "state_version": 2, - "cli_version": "0.0.0-test", + "cli_version": "[CLI_VERSION]", "lineage": "test-lineage", "serial": 2, "state": { diff --git a/bundle/direct/dstate/state.go b/bundle/direct/dstate/state.go index 7e92aa7c1d2..68f7df8b53b 100644 --- a/bundle/direct/dstate/state.go +++ b/bundle/direct/dstate/state.go @@ -73,10 +73,15 @@ type DeploymentState struct { } type Header struct { - StateVersion int `json:"state_version"` - CLIVersion string `json:"cli_version"` - Lineage string `json:"lineage"` - Serial int `json:"serial"` + StateVersion int `json:"state_version"` + + // CLIVersion is the version of the CLI that last wrote this state. It is + // refreshed from the WAL header on every deploy that commits changes, so it + // tracks the most recent writer rather than the CLI that created the state. + CLIVersion string `json:"cli_version"` + + Lineage string `json:"lineage"` + Serial int `json:"serial"` // Features maps each feature flag this state depends on to a (currently empty) // value. This CLI writes no features; it only reads the field to detect a state @@ -338,7 +343,10 @@ func (db *DeploymentState) mergeWalIntoState(ctx context.Context) (bool, error) scanner.Buffer(make([]byte, 0, initialBufferSize), maxWalEntrySize) lineNumber := 0 var corruptedLines [][]byte - var newSerial int + var ( + newSerial int + newCLIVersion string + ) for scanner.Scan() { lineNumber++ @@ -363,6 +371,7 @@ func (db *DeploymentState) mergeWalIntoState(ctx context.Context) (bool, error) return false, fmt.Errorf("WAL serial (%d) is ahead of expected (%d), state may be corrupted", header.Serial, expectedSerial) } newSerial = header.Serial + newCLIVersion = header.CLIVersion } else { var entry WALEntry if err := json.Unmarshal(line, &entry); err != nil { @@ -405,8 +414,14 @@ func (db *DeploymentState) mergeWalIntoState(ctx context.Context) (bool, error) // for it leaves the in-memory serial ahead of the persisted one, so the // next deploy writes its WAL header at serial+2 and recovery rejects it as // "ahead of expected". See acceptance/bundle/deploy/wal/header-only-wal. + // + // The CLI version moves with the serial for the same reason: it records the + // CLI that last wrote the state, so it is only accurate once that write is + // persisted. Without this the field keeps the version of the CLI that first + // created the state, no matter how many times a newer CLI deploys over it. if hasEntries { db.Data.Serial = newSerial + db.Data.CLIVersion = newCLIVersion } return hasEntries, nil diff --git a/bundle/direct/dstate/state_test.go b/bundle/direct/dstate/state_test.go index 11589944472..3b5dc06221a 100644 --- a/bundle/direct/dstate/state_test.go +++ b/bundle/direct/dstate/state_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "testing" + "github.com/databricks/cli/internal/build" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -101,6 +102,52 @@ func TestPanicOnDoubleOpen(t *testing.T) { mustFinalize(t, &db) } +// TestCLIVersionRecordsLastWriter pins that cli_version tracks the CLI that last +// wrote the state, not the one that created it. Previously the field was only set +// when the state was first created: the WAL header carried the deploying CLI's +// version but replay dropped it, so a state stayed pinned to its original writer +// no matter how many times a newer CLI deployed over it. +func TestCLIVersionRecordsLastWriter(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.json") + + // A state written by some older CLI. + seed := `{"state_version":2,"cli_version":"0.1.2","lineage":"test-lineage","serial":1,"state":{}}` + require.NoError(t, os.WriteFile(path, []byte(seed), 0o600)) + + var db DeploymentState + require.NoError(t, db.Open(t.Context(), path, WithRecovery(true), WithWrite(true))) + require.NoError(t, db.SaveState("resources.jobs.my_job", "123", map[string]string{"k": "v"}, nil)) + mustFinalize(t, &db) + + var reopened DeploymentState + require.NoError(t, reopened.Open(t.Context(), path, WithRecovery(false), WithWrite(false))) + assert.Equal(t, build.GetInfo().Version, reopened.Data.CLIVersion) + assert.Equal(t, 2, reopened.Data.Serial) + mustFinalize(t, &reopened) +} + +// TestHeaderOnlyWALDoesNotUpdateCLIVersion is the counterpart to the serial +// invariant below: a deploy that commits nothing does not persist a state file, +// so it must not claim to have written one. +func TestHeaderOnlyWALDoesNotUpdateCLIVersion(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.json") + walPath := path + walSuffix + + seed := `{"state_version":2,"cli_version":"0.1.2","lineage":"test-lineage","serial":1,"state":{}}` + require.NoError(t, os.WriteFile(path, []byte(seed), 0o600)) + + header := Header{Lineage: "test-lineage", Serial: 2, StateVersion: currentStateVersion, CLIVersion: build.GetInfo().Version} + headerLine, err := json.Marshal(header) + require.NoError(t, err) + require.NoError(t, os.WriteFile(walPath, append(headerLine, '\n'), 0o600)) + + var recovered DeploymentState + require.NoError(t, recovered.Open(t.Context(), path, WithRecovery(true), WithWrite(false))) + assert.Equal(t, "0.1.2", recovered.Data.CLIVersion, "a header-only WAL wrote no state, so the version must not move") + assert.Equal(t, 1, recovered.Data.Serial) + mustFinalize(t, &recovered) +} + func TestHeaderOnlyWALRecoveryDoesNotAdvanceSerial(t *testing.T) { path := filepath.Join(t.TempDir(), "state.json") walPath := path + walSuffix