Skip to content

Commit

Permalink
Cleanup remote file path on bundle destroy (#1374)
Browse files Browse the repository at this point in the history
## Changes
The sync struct initialization would recreate the deleted `file_path`.
This PR moves to not initializing the sync object to delete the
snapshot, thus fixing the lingering `file_path` after `bundle destroy`.

## Tests
Manually, and a integration test to prevent regression.
  • Loading branch information
shreyas-goenka authored Apr 19, 2024
1 parent f6c4d6d commit e008c2b
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 18 deletions.
25 changes: 19 additions & 6 deletions bundle/deploy/files/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package files
import (
"context"
"fmt"
"os"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/cmdio"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/sync"
"github.com/databricks/databricks-sdk-go/service/workspace"
"github.com/fatih/color"
)
Expand Down Expand Up @@ -46,20 +48,31 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
}

// Clean up sync snapshot file
sync, err := GetSync(ctx, bundle.ReadOnly(b))
if err != nil {
return diag.FromErr(err)
}
err = sync.DestroySnapshot(ctx)
err = deleteSnapshotFile(ctx, b)
if err != nil {
return diag.FromErr(err)
}

cmdio.LogString(ctx, fmt.Sprintf("Deleted snapshot file at %s", sync.SnapshotPath()))
cmdio.LogString(ctx, "Successfully deleted files!")
return nil
}

func deleteSnapshotFile(ctx context.Context, b *bundle.Bundle) error {
opts, err := GetSyncOptions(ctx, bundle.ReadOnly(b))
if err != nil {
return fmt.Errorf("cannot get sync options: %w", err)
}
sp, err := sync.SnapshotPath(opts)
if err != nil {
return err
}
err = os.Remove(sp)
if err != nil && !os.IsNotExist(err) {
return fmt.Errorf("failed to destroy sync snapshot file: %s", err)
}
return nil
}

func Delete() bundle.Mutator {
return &delete{}
}
70 changes: 70 additions & 0 deletions internal/bundle/destroy_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package bundle

import (
"errors"
"os"
"path/filepath"
"testing"

"github.com/databricks/cli/internal/acc"
"github.com/databricks/databricks-sdk-go/apierr"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestAccBundleDestroy(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)
w := wt.W

uniqueId := uuid.New().String()
bundleRoot, err := initTestTemplate(t, ctx, "deploy_then_remove_resources", map[string]any{
"unique_id": uniqueId,
})
require.NoError(t, err)

snapshotsDir := filepath.Join(bundleRoot, ".databricks", "bundle", "default", "sync-snapshots")

// Assert the snapshot file does not exist
_, err = os.ReadDir(snapshotsDir)
assert.ErrorIs(t, err, os.ErrNotExist)

// deploy pipeline
err = deployBundle(t, ctx, bundleRoot)
require.NoError(t, err)

// Assert the snapshot file exists
entries, err := os.ReadDir(snapshotsDir)
assert.NoError(t, err)
assert.Len(t, entries, 1)

// Assert bundle deployment path is created
remoteRoot := getBundleRemoteRootPath(w, t, uniqueId)
_, err = w.Workspace.GetStatusByPath(ctx, remoteRoot)
assert.NoError(t, err)

// assert pipeline is created
pipelineName := "test-bundle-pipeline-" + uniqueId
pipeline, err := w.Pipelines.GetByName(ctx, pipelineName)
require.NoError(t, err)
assert.Equal(t, pipeline.Name, pipelineName)

// destroy bundle
err = destroyBundle(t, ctx, bundleRoot)
require.NoError(t, err)

// assert pipeline is deleted
_, err = w.Pipelines.GetByName(ctx, pipelineName)
assert.ErrorContains(t, err, "does not exist")

// Assert snapshot file is deleted
entries, err = os.ReadDir(snapshotsDir)
require.NoError(t, err)
assert.Len(t, entries, 0)

// Assert bundle deployment path is deleted
_, err = w.Workspace.GetStatusByPath(ctx, remoteRoot)
apiErr := &apierr.APIError{}
assert.True(t, errors.As(err, &apiErr))
assert.Equal(t, "RESOURCE_DOES_NOT_EXIST", apiErr.ErrorCode)
}
8 changes: 0 additions & 8 deletions libs/sync/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,6 @@ func (s *Snapshot) Save(ctx context.Context) error {
return nil
}

func (s *Snapshot) Destroy(ctx context.Context) error {
err := os.Remove(s.SnapshotPath)
if err != nil && !os.IsNotExist(err) {
return fmt.Errorf("failed to destroy sync snapshot file: %s", err)
}
return nil
}

func loadOrNewSnapshot(ctx context.Context, opts *SyncOptions) (*Snapshot, error) {
snapshot, err := newSnapshot(ctx, opts)
if err != nil {
Expand Down
4 changes: 0 additions & 4 deletions libs/sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,6 @@ func (s *Sync) GetFileList(ctx context.Context) ([]fileset.File, error) {
return all.Iter(), nil
}

func (s *Sync) DestroySnapshot(ctx context.Context) error {
return s.snapshot.Destroy(ctx)
}

func (s *Sync) SnapshotPath() string {
return s.snapshot.SnapshotPath
}
Expand Down

0 comments on commit e008c2b

Please sign in to comment.