Cleanup remote file path on bundle destroy (#1374)

## 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.
This commit is contained in:
shreyas-goenka 2024-04-19 17:18:04 +05:30 committed by GitHub
parent f6c4d6d152
commit e008c2bd8c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 89 additions and 18 deletions

View File

@ -3,10 +3,12 @@ package files
import ( import (
"context" "context"
"fmt" "fmt"
"os"
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/cmdio"
"github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/sync"
"github.com/databricks/databricks-sdk-go/service/workspace" "github.com/databricks/databricks-sdk-go/service/workspace"
"github.com/fatih/color" "github.com/fatih/color"
) )
@ -46,20 +48,31 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
} }
// Clean up sync snapshot file // Clean up sync snapshot file
sync, err := GetSync(ctx, bundle.ReadOnly(b)) err = deleteSnapshotFile(ctx, b)
if err != nil {
return diag.FromErr(err)
}
err = sync.DestroySnapshot(ctx)
if err != nil { if err != nil {
return diag.FromErr(err) return diag.FromErr(err)
} }
cmdio.LogString(ctx, fmt.Sprintf("Deleted snapshot file at %s", sync.SnapshotPath()))
cmdio.LogString(ctx, "Successfully deleted files!") cmdio.LogString(ctx, "Successfully deleted files!")
return nil 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 { func Delete() bundle.Mutator {
return &delete{} return &delete{}
} }

View File

@ -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)
}

View File

@ -138,14 +138,6 @@ func (s *Snapshot) Save(ctx context.Context) error {
return nil 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) { func loadOrNewSnapshot(ctx context.Context, opts *SyncOptions) (*Snapshot, error) {
snapshot, err := newSnapshot(ctx, opts) snapshot, err := newSnapshot(ctx, opts)
if err != nil { if err != nil {

View File

@ -216,10 +216,6 @@ func (s *Sync) GetFileList(ctx context.Context) ([]fileset.File, error) {
return all.Iter(), nil return all.Iter(), nil
} }
func (s *Sync) DestroySnapshot(ctx context.Context) error {
return s.snapshot.Destroy(ctx)
}
func (s *Sync) SnapshotPath() string { func (s *Sync) SnapshotPath() string {
return s.snapshot.SnapshotPath return s.snapshot.SnapshotPath
} }