Fix bundle file resync after remote directory deletion

.nextchanges/bundles/resync-deleted-bundle-files.md

@@ -0,0 +1 @@
+* Restore bundle files on redeploy when the remote files directory has been deleted or recreated. ([#6661](https://github.com/databricks/cli/pull/6661))

acceptance/bundle/deploy/files/out-of-band-delete/output.txt

@@ -15,17 +15,34 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged
 === Delete the remote bundle directory out-of-band (simulates deleting it in the UI)
 >>> [CLI] workspace delete --recursive /Workspace/Users/[USERNAME]/.bundle/out-of-band-delete
 
-=== Redeploy reuses the stale snapshot, so the python_file is NOT re-uploaded
+=== Redeploy restores the python_file without removing the local sync snapshot
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/out-of-band-delete/default/files...
+Files: 4 uploaded, 0 deleted
+Resources: 0 created, 0 changed, 0 deleted, 1 unchanged
+
+>>> [CLI] workspace get-status /Workspace/Users/[USERNAME]/.bundle/out-of-band-delete/default/files/hello_world.py
+{
+  "object_type": "FILE",
+  "path": "/Workspace/Users/[USERNAME]/.bundle/out-of-band-delete/default/files/hello_world.py"
+}
+
+=== An unchanged redeploy does not upload files again
 >>> [CLI] bundle deploy
 Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/out-of-band-delete/default/files...
 Files: 0 uploaded, 0 deleted
 Resources: 0 created, 0 changed, 0 deleted, 1 unchanged
 
->>> musterr [CLI] workspace get-status /Workspace/Users/[USERNAME]/.bundle/out-of-band-delete/default/files/hello_world.py
-Error: Path (/Workspace/Users/[USERNAME]/.bundle/out-of-band-delete/default/files/hello_world.py) doesn't exist.
+>>> [CLI] workspace get-status /Workspace/Users/[USERNAME]/.bundle/out-of-band-delete/default/files/hello_world.py
+{
+  "object_type": "FILE",
+  "path": "/Workspace/Users/[USERNAME]/.bundle/out-of-band-delete/default/files/hello_world.py"
+}
+
+=== Recreating only the files directory also invalidates the old snapshot
+>>> [CLI] workspace delete --recursive /Workspace/Users/[USERNAME]/.bundle/out-of-band-delete/default/files
 
-=== Removing the sync snapshot forces a full re-upload, restoring the python_file
->>> rm -rf .databricks/bundle/default/sync-snapshots
+>>> [CLI] workspace mkdirs /Workspace/Users/[USERNAME]/.bundle/out-of-band-delete/default/files
 
 >>> [CLI] bundle deploy
 Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/out-of-band-delete/default/files...

acceptance/bundle/deploy/files/out-of-band-delete/script

@@ -11,11 +11,16 @@ trace $CLI workspace get-status "$BUNDLE_PATH/files/hello_world.py" | jq '{objec
 title "Delete the remote bundle directory out-of-band (simulates deleting it in the UI)"
 trace $CLI workspace delete --recursive "/Workspace/Users/${CURRENT_USER_NAME}/.bundle/out-of-band-delete"
 
-title "Redeploy reuses the stale snapshot, so the python_file is NOT re-uploaded"
+title "Redeploy restores the python_file without removing the local sync snapshot"
 trace $CLI bundle deploy
-trace musterr $CLI workspace get-status "$BUNDLE_PATH/files/hello_world.py"
+trace $CLI workspace get-status "$BUNDLE_PATH/files/hello_world.py" | jq '{object_type,path}'
+
+title "An unchanged redeploy does not upload files again"
+trace $CLI bundle deploy
+trace $CLI workspace get-status "$BUNDLE_PATH/files/hello_world.py" | jq '{object_type,path}'
 
-title "Removing the sync snapshot forces a full re-upload, restoring the python_file"
-trace rm -rf .databricks/bundle/default/sync-snapshots
+title "Recreating only the files directory also invalidates the old snapshot"
+trace $CLI workspace delete --recursive "$BUNDLE_PATH/files"
+trace $CLI workspace mkdirs "$BUNDLE_PATH/files"
 trace $CLI bundle deploy
 trace $CLI workspace get-status "$BUNDLE_PATH/files/hello_world.py" | jq '{object_type,path}'

acceptance/bundle/deploy/files/out-of-band-delete/test.toml

@@ -3,8 +3,6 @@
 # handed over to the service (see the TODO in dstate.Open).
 EnvMatrix.DMS = [""]
 
-Badness = "After the remote bundle files are deleted out-of-band, the next deploy does not re-upload them until the local sync snapshot is removed."
-
 # This test passes absolute workspace paths like /Workspace/Users/.../hello_world.py
 # as CLI arguments. On Windows the script runs under MSYS2, which rewrites such
 # leading-slash arguments to Windows paths (e.g. C:/Program Files/Git/Workspace/...)

libs/sync/path.go

@@ -26,14 +26,20 @@ func repoPathForPath(me *iam.User, remotePath string) string {
 // expected base paths and if it is a directory or repository.
 // If dryRun is set, a missing remote directory is not created.
 func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClient, remotePath string, me *iam.User, dryRun bool) error {
+	_, err := ensureRemotePathIsUsable(ctx, wsc, remotePath, me, dryRun)
+	return err
+}
+
+// ensureRemotePathIsUsable returns the directory's object ID, or zero when it is missing in a dry run.
+func ensureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClient, remotePath string, me *iam.User, dryRun bool) (int64, error) {
 	var err error
 
 	// TODO: we should cache CurrentUser.Me at the SDK level
 	//      for now we let clients pass in any existing user they might already have
 	if me == nil {
 		me, err = wsc.CurrentUser.Me(ctx, iam.MeRequest{})
 		if err != nil {
-			return err
+			return 0, err
 		}
 	}
 
@@ -44,31 +50,31 @@ func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClie
 	if err != nil {
 		// We only deal with 404s below.
 		if !apierr.IsMissing(err) {
-			return err
+			return 0, err
 		}
 
 		// If the path is nested under a repo, the repo has to exist.
 		if strings.HasPrefix(remotePath, "/Repos/") {
 			repoPath := repoPathForPath(me, remotePath)
 			_, err = wsc.Workspace.GetStatusByPath(ctx, repoPath)
 			if err != nil && apierr.IsMissing(err) {
-				return fmt.Errorf("%s does not exist; please create it first", repoPath)
+				return 0, fmt.Errorf("%s does not exist; please create it first", repoPath)
 			}
 		}
 
 		// A dry run must not create the missing directory; nothing left to validate.
 		if dryRun {
-			return nil
+			return 0, nil
 		}
 
 		// The workspace path doesn't exist. Create it and try again.
 		err = wsc.Workspace.MkdirsByPath(ctx, remotePath)
 		if err != nil {
-			return fmt.Errorf("unable to create directory at %s: %w", remotePath, err)
+			return 0, fmt.Errorf("unable to create directory at %s: %w", remotePath, err)
 		}
 		info, err = wsc.Workspace.GetStatusByPath(ctx, remotePath)
 		if err != nil {
-			return err
+			return 0, err
 		}
 	}
 
@@ -82,8 +88,8 @@ func EnsureRemotePathIsUsable(ctx context.Context, wsc *databricks.WorkspaceClie
 
 	// We expect the object at path to be a directory or a repo.
 	if info.ObjectType == workspace.ObjectTypeDirectory || info.ObjectType == workspace.ObjectTypeRepo {
-		return nil
+		return info.ObjectId, nil
 	}
 
-	return fmt.Errorf("%s points to a %s", remotePath, strings.ToLower(info.ObjectType.String()))
+	return 0, fmt.Errorf("%s points to a %s", remotePath, strings.ToLower(info.ObjectType.String()))
 }

libs/sync/snapshot.go

@@ -49,6 +49,9 @@ type Snapshot struct {
 	// Path in workspace for project repo
 	RemotePath string `json:"remote_path"`
 
+	// RemoteObjectID identifies the directory whose files this snapshot tracks.
+	RemoteObjectID int64 `json:"remote_object_id,omitempty"`
+
 	*SnapshotState
 }
 

libs/sync/sync.go

@@ -84,9 +84,10 @@ func New(ctx context.Context, opts SyncOptions) (*Sync, error) {
 
 	WriteGitIgnore(ctx, opts.LocalRoot.Native())
 
+	var remoteObjectID int64
 	if !opts.NoValidateRemotePath {
 		// Verify that the remote path we're about to synchronize to is valid and allowed.
-		err = EnsureRemotePathIsUsable(ctx, opts.WorkspaceClient, opts.RemotePath, opts.CurrentUser, opts.DryRun)
+		remoteObjectID, err = ensureRemotePathIsUsable(ctx, opts.WorkspaceClient, opts.RemotePath, opts.CurrentUser, opts.DryRun)
 		if err != nil {
 			return nil, err
 		}
@@ -114,6 +115,26 @@ func New(ctx context.Context, opts SyncOptions) (*Sync, error) {
 		}
 	}
 
+	if !opts.NoValidateRemotePath {
+		// Validation may already have recreated a deleted directory before upload.
+		// Compare its identity with the last successful sync, not just its existence.
+		if !snapshot.New {
+			switch {
+			case remoteObjectID == 0 || (snapshot.RemoteObjectID != 0 && snapshot.RemoteObjectID != remoteObjectID):
+				log.Debugf(ctx, "Remote directory changed, discarding sync snapshot for %s", opts.RemotePath)
+				snapshot, err = newSnapshot(ctx, &opts)
+				if err != nil {
+					return nil, fmt.Errorf("unable to reset sync snapshot: %w", err)
+				}
+			case snapshot.RemoteObjectID == 0:
+				// Older snapshots and snapshots rebuilt from deployment state lack a directory ID.
+				// Re-upload files but retain their mappings to delete files removed locally.
+				snapshot.ResetLastModifiedTimes()
+			}
+		}
+		snapshot.RemoteObjectID = remoteObjectID
+	}
+
 	filer, err := filer.NewWorkspaceFilesClient(opts.WorkspaceClient, opts.RemotePath)
 	if err != nil {
 		return nil, err

libs/sync/sync_test.go

@@ -0,0 +1,102 @@
+package sync_test
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/databricks/cli/libs/fileset"
+	"github.com/databricks/cli/libs/sync"
+	"github.com/databricks/cli/libs/testserver"
+	"github.com/databricks/cli/libs/vfs"
+	"github.com/databricks/databricks-sdk-go"
+	"github.com/databricks/databricks-sdk-go/apierr"
+	"github.com/databricks/databricks-sdk-go/service/iam"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSyncSnapshotDirectoryIdentity(t *testing.T) {
+	for _, tc := range []struct {
+		name        string
+		exists      bool
+		legacy      bool
+		replaced    bool
+		wantUploads int
+	}{
+		{name: "unchanged", exists: true},
+		{name: "legacy snapshot", exists: true, legacy: true, wantUploads: 1},
+		{name: "replaced directory", exists: true, replaced: true, wantUploads: 1},
+		{name: "deleted directory", wantUploads: 1},
+		{name: "deleted directory with legacy snapshot", legacy: true, wantUploads: 1},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := t.Context()
+			server := testserver.New(t)
+			testserver.AddDefaultHandlers(server)
+			client, err := databricks.NewWorkspaceClient(&databricks.Config{
+				Host:  server.URL,
+				Token: "test-token",
+			})
+			require.NoError(t, err)
+
+			root := t.TempDir()
+			require.NoError(t, os.WriteFile(filepath.Join(root, "file.txt"), []byte("hello"), 0o600))
+			opts := sync.SyncOptions{
+				WorktreeRoot:     vfs.MustNew(root),
+				LocalRoot:        vfs.MustNew(root),
+				Paths:            []string{"."},
+				RemotePath:       "/test-sync",
+				SnapshotBasePath: t.TempDir(),
+				Host:             server.URL,
+				WorkspaceClient:  client,
+				CurrentUser:      &iam.User{UserName: "test-user"},
+				DryRun:           true,
+			}
+			var remoteID int64
+			if tc.exists {
+				require.NoError(t, client.Workspace.MkdirsByPath(ctx, opts.RemotePath))
+				info, err := client.Workspace.GetStatusByPath(ctx, opts.RemotePath)
+				require.NoError(t, err)
+				remoteID = info.ObjectId
+				require.NotZero(t, remoteID)
+			}
+
+			files, err := fileset.New(opts.LocalRoot).Files()
+			require.NoError(t, err)
+			snapshot, err := sync.NewSnapshot(files, &opts)
+			require.NoError(t, err)
+			for _, file := range files {
+				snapshot.LastModifiedTimes[file.Relative] = file.Modified()
+			}
+			switch {
+			case tc.legacy:
+				snapshot.RemoteObjectID = 0
+			case tc.replaced || !tc.exists:
+				snapshot.RemoteObjectID = remoteID + 1
+			default:
+				snapshot.RemoteObjectID = remoteID
+			}
+			require.NoError(t, snapshot.Save(ctx))
+			snapshotPath, err := sync.SnapshotPath(&opts)
+			require.NoError(t, err)
+			before, err := os.ReadFile(snapshotPath)
+			require.NoError(t, err)
+
+			require.NoError(t, sync.EnsureRemotePathIsUsable(ctx, client, opts.RemotePath, opts.CurrentUser, true))
+			s, err := sync.New(ctx, opts)
+			require.NoError(t, err)
+			defer s.Close()
+			_, err = s.RunOnce(ctx)
+			require.NoError(t, err)
+			assert.Equal(t, sync.FileCounts{Uploaded: tc.wantUploads}, s.FileCounts())
+			after, err := os.ReadFile(snapshotPath)
+			require.NoError(t, err)
+			assert.Equal(t, before, after, "a dry run must not persist snapshot invalidation")
+			if !tc.exists {
+				_, err = client.Workspace.GetStatusByPath(ctx, opts.RemotePath)
+				assert.True(t, apierr.IsMissing(err), "a dry run must not recreate the remote directory")
+			}
+		})
+	}
+}