Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .nextchanges/bundles/state-cli-version-last-writer.md
Original file line numberDiff line numberDiff line change
@@ -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.
Original file line numberDiff line numberDiff line change
Expand Up@@ -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": {
Expand Down
25 changes: 20 additions & 5 deletions bundle/direct/dstate/state.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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++
Expand All@@ -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 {
Expand DownExpand Up@@ -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
Expand Down
47 changes: 47 additions & 0 deletions bundle/direct/dstate/state_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@ import (
"path/filepath"
"testing"

"github.com/databricks/cli/internal/build"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand DownExpand Up@@ -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
Expand Down
Loading