bundle/deploy/lock: introduce DeploymentManager interface aligned with DMS versioning model by shreyas-goenka · Pull Request #5314 · databricks/cli

shreyas-goenka · 2026-05-22T11:45:53Z

Summary

Introduces a DeploymentManager interface that abstracts deployment locking and versioning, shaped around DMS (Deployment Management Service) versioning semantics.

Interface:

type DeploymentManager interface {
    CreateVersion(ctx context.Context, goal terraform.DeploymentGoal) (int64, error)
    CompleteVersion(ctx context.Context, version int64, status terraform.DeploymentStatus) error
}

For DMS - CreateVersion will atomically succeed only if the new version is +1 relative to the latest — this is the DMS contract. The workspace-filesystem implementation acquires the workspace lock file and returns version 0 as a placeholder (since the lock file doesn't track versions).
CompleteVersion records the outcome of a deployment and releases the lock. The workspace-filesystem implementation releases the lock file.

Factory:

func NewDeploymentManager(ctx context.Context, b *bundle.Bundle) (DeploymentManager, error)

Goal is not passed to the constructor — it moves to CreateVersion to match the DMS versioning model.

Files deleted: acquire.go, release.go (logic folded into workspaceFilesystemLock).

Updated callers: bundle/phases/deploy.go, bundle/phases/destroy.go, bundle/phases/bind.go all use the new interface.

This interface is a no-op shim today; a follow-on PR will wire in a real DMS client when DATABRICKS_BUNDLE_USE_CLIENT_VERSIONING is set.

This pull request and its description were written by Isaac.

Pure code-movement refactor. Wraps the existing workspace-filesystem lock behavior behind a DeploymentLock interface so a follow-up PR can introduce an alternative metadata-service-backed lock implementation without touching deploy/destroy/bind callers again. What changed: - New bundle/deploy/lock/lock.go: DeploymentLock interface, Goal enum (moved from release.go), DeploymentStatus enum, and a NewDeploymentLock factory that unconditionally returns the workspace filesystem implementation. - New bundle/deploy/lock/workspace_filesystem.go: workspaceFilesystemLock struct that implements DeploymentLock. Preserves the historical behavior of the deleted acquire.go / release.go mutators: lock-disabled short-circuit, locker.CreateLocker initialization, the permissions.ReportPossiblePermissionDenied branch on fs.ErrPermission / fs.ErrNotExist, and the destroy-mode locker.AllowLockFileNotExist unlock quirk. - Deleted bundle/deploy/lock/acquire.go and bundle/deploy/lock/release.go. - Updated bundle/phases/{deploy,destroy,bind}.go to construct the lock once via NewDeploymentLock and call Acquire / Release directly instead of through bundle.ApplyContext. The deferred Release now reports DeploymentSuccess / DeploymentFailure based on logdiag.HasError so a future DMS-backed implementation can record the outcome. Behavior is preserved end-to-end: lock-related acceptance goldens (pipelines/{deploy,destroy}/force-lock, bundle/help/bundle-{deploy, destroy}) all pass unchanged.

eng-dev-ecosystem-bot · 2026-05-22T12:31:34Z

Commit: 715a18b

Run: 26724748931

…dle.Bundle Pre-refactor the locker was stashed on b.Locker so the two mutators (acquire then release) could share it via the Bundle. After PR #5314 both lifecycle methods live on a single struct, so the cross-method state-passing through bundle.Bundle is redundant — grep finds zero external consumers of b.Locker. Move the *locker.Locker to a field on workspaceFilesystemLock and delete the now-unused Locker field on bundle.Bundle. Co-authored-by: Isaac

The struct now holds only the primitives + workspace client + a permission-error callback it needs. The bundle-aware wiring lives in NewDeploymentLock, which captures everything from the *bundle.Bundle at construction time. Why: keeps the type-level dependency surface narrow (the impl no longer imports bundle or bundle/permissions), removes a class of accidental coupling that would make alternative lock implementations awkward, and forecloses the possibility of a future bundle <-> lock import cycle. NewDeploymentLock now takes ctx so it can call b.WorkspaceClient(ctx) at construction; the three callers in bundle/phases are updated in one line each. Co-authored-by: Isaac

The lock and workspace_filesystem files live in the same package, so the package-level import of bundle/permissions exists regardless of whether the struct hides the call behind a function value. The callback only added a layer of indirection; switch to a direct permissions.ReportPossiblePermissionDenied call and keep a narrow b field purely for that error path. Co-authored-by: Isaac

The error is returned to the caller; logging it here just produces a duplicate line in the user-facing output. Drop the log; preserve the permission-denied branch and the bare return. Co-authored-by: Isaac

…or semantics) The original release.go mutator returned diag.FromErr on unlock failure, which surfaced as an error diagnostic to the user. The defer pattern introduced in #5314 dropped it to log.Warnf — a silent demotion that would hide a stuck lock from the user (who normally has to recover with --force-lock). Switch the defer to logdiag.LogError so the unlock failure shows up as a proper diagnostic, matching the pre-refactor behavior. The deploy/destroy/ bind/unbind phases all share the same fix. Co-authored-by: Isaac

shreyas-goenka · 2026-05-28T10:09:37Z


 	defer func() {
-		bundle.ApplyContext(ctx, b, lock.Release(lock.GoalBind))
+		status := lock.DeploymentSuccess


status is not used for now - but will be used with deployment metadata service.

…lesystemLock ReportPossiblePermissionDenied has a second caller (bundle/deploy/files/ upload.go) and walks bundle.Config.RunAs / Permissions / Resources via analyzeBundlePermissions, so the lock can't simply inline it or call a simplified variant. But the struct doesn't need to pin a *bundle.Bundle field — the bundle-aware wiring can live in a callback closure that NewDeploymentLock builds at construction time. After this commit workspaceFilesystemLock holds only primitives, a workspace client, and the callback; no bundle types appear in its field list. Co-authored-by: Isaac

…ment lock" goldens Earlier in this PR we removed the log.Errorf wrapper in workspaceFilesystemLock because the same error is already surfaced via the returned diag. That removed the duplicate "Error: Failed to acquire deployment lock: ..." line from the CLI's stderr output, which two force-lock acceptance goldens were still expecting. Update both goldens to match the single-line shape. Co-authored-by: Isaac

The log.Errorf in workspaceFilesystemLock.Acquire isn't redundant with the downstream logdiag.LogError — it emits a separate user-visible "Error: Failed to acquire deployment lock: ..." stderr line that the force-lock acceptance goldens depend on. Removing it (commit 0433a83) was a real behavior change; this PR is supposed to be a pure refactor. Restore the log call and the two golden files. Co-authored-by: Isaac

github-actions · 2026-05-28T11:59:25Z

Waiting for approval

Based on git history, these people are best suited to review:

@denik -- recent work in bundle/phases/, bundle/, bundle/deploy/lock/

Eligible reviewers: @andrewnester, @anton-107, @janniklasrose, @lennartkats-db, @pietern

_{Suggestions based on git history. See OWNERS for ownership rules.}

The original acquire.go mutator never called b.WorkspaceClient(ctx) when lock.enabled was false — the disabled check returned early in Apply before init() ran. After moving primitives to construction time the client was being eagerly initialized regardless, which adds a network call for the common dev-mode case where locking is disabled. Gate the client init on the captured enabled flag. Co-authored-by: Isaac

…oseVersion Renames the DeploymentLock interface to DeploymentManager and replaces Acquire/Release with CreateVersion/CloseVersion to align with the Deployment Metadata Service (DMS) data model, where deployments are versioned operations rather than mutual-exclusion locks. - CreateVersion(ctx, goal) (int64, error): begins a deployment; the workspace-filesystem implementation acquires the lock file and returns version 0 as a placeholder. DMS will return the atomically assigned version number (+1 to latest closed version). - CloseVersion(ctx, version, status): ends the deployment; the workspace-filesystem implementation releases the lock file. The goal moves from the NewDeploymentManager constructor to CreateVersion so a single manager instance is decoupled from a specific operation type. Co-authored-by: Shreyas Goenka <shreyas.goenka@databricks.com>

Co-authored-by: Shreyas Goenka <shreyas.goenka@databricks.com>

shreyas-goenka temporarily deployed to test-trigger-is May 22, 2026 11:46 — with GitHub Actions Inactive

This was referenced May 26, 2026

bundle: add DATABRICKS_BUNDLE_MANAGED_STATE gate + DMS-backed deployment lock #5331

Draft

bundle: add bundle.managed_state config field + DMS test server #5333

Draft

shreyas-goenka temporarily deployed to test-trigger-is May 28, 2026 09:48 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is May 28, 2026 09:55 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is May 28, 2026 10:02 — with GitHub Actions Inactive

bundle/deploy/lock: drop redundant Errorf on failed lock acquire

0433a83

The error is returned to the caller; logging it here just produces a duplicate line in the user-facing output. Drop the log; preserve the permission-denied branch and the bare return. Co-authored-by: Isaac

shreyas-goenka temporarily deployed to test-trigger-is May 28, 2026 10:03 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is May 28, 2026 10:07 — with GitHub Actions Inactive

shreyas-goenka commented May 28, 2026

View reviewed changes

shreyas-goenka temporarily deployed to test-trigger-is May 28, 2026 10:13 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is May 28, 2026 11:18 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is May 28, 2026 11:36 — with GitHub Actions Inactive

shreyas-goenka marked this pull request as ready for review May 28, 2026 11:58

shreyas-goenka requested review from andrewnester, denik and pietern May 28, 2026 11:58

shreyas-goenka temporarily deployed to test-trigger-is May 28, 2026 12:03 — with GitHub Actions Inactive

denik reviewed May 29, 2026

View reviewed changes

Comment thread bundle/deploy/lock/lock.go

shreyas-goenka temporarily deployed to test-trigger-is May 31, 2026 17:45 — with GitHub Actions Inactive

bundle/deploy/lock: rename CloseVersion to CompleteVersion

715a18b

Co-authored-by: Shreyas Goenka <shreyas.goenka@databricks.com>

shreyas-goenka temporarily deployed to test-trigger-is May 31, 2026 21:17 — with GitHub Actions Inactive

shreyas-goenka changed the title ~~bundle: extract DeploymentLock interface + workspace filesystem impl~~ bundle/deploy/lock: introduce DeploymentManager interface aligned with DMS versioning model May 31, 2026

This was referenced May 31, 2026

bundle/deploy/lock: write deployment_version.json on every CreateVersion #5385

Closed

bundle/deploy/lock: add DMS-backed DeploymentManager implementation using SDK bundle client #5386

Draft

bundle/direct: record resource operations with DMS #5387

Draft

shreyas-goenka requested a review from denik May 31, 2026 22:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bundle/deploy/lock: introduce DeploymentManager interface aligned with DMS versioning model#5314

bundle/deploy/lock: introduce DeploymentManager interface aligned with DMS versioning model#5314
shreyas-goenka wants to merge 12 commits into
mainfrom
shreyas-goenka/bundle-lock-abstraction

shreyas-goenka commented May 22, 2026 •

edited

Loading

Uh oh!

eng-dev-ecosystem-bot commented May 22, 2026 •

edited

Loading

Uh oh!

shreyas-goenka May 28, 2026

Uh oh!

github-actions Bot commented May 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

shreyas-goenka commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

eng-dev-ecosystem-bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shreyas-goenka May 28, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 28, 2026

Waiting for approval

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shreyas-goenka commented May 22, 2026 •

edited

Loading

eng-dev-ecosystem-bot commented May 22, 2026 •

edited

Loading