Wait for app deletion in DoCreate instead of DoDelete

.golangci.yaml

@@ -93,7 +93,7 @@ linters:
       - path-except: bundle/direct/dresources
         linters:
           - exhaustruct
-      - path: bundle/direct/dresources/all_test.go
+      - path: bundle/direct/dresources/.*_test.go
         linters:
           - exhaustruct
       - path-except: ^cmd

bundle/direct/dresources/app.go

@@ -2,6 +2,7 @@ package dresources
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -36,12 +37,36 @@ func (r *ResourceApp) DoCreate(ctx context.Context, config *apps.App) (string, *
 		NoCompute:       true,
 		ForceSendFields: nil,
 	}
-	waiter, err := r.client.Apps.Create(ctx, request)
+
+	retrier := retries.New[apps.App](retries.WithTimeout(15*time.Minute), retries.WithRetryFunc(shouldRetry))
+	app, err := retrier.Run(ctx, func(ctx context.Context) (*apps.App, error) {
+		waiter, err := r.client.Apps.Create(ctx, request)
+		if err != nil {
+			if errors.Is(err, apierr.ErrResourceAlreadyExists) {
+				// Check if the app is in DELETING state - only then should we retry
+				existingApp, getErr := r.client.Apps.GetByName(ctx, config.Name)
+				if getErr != nil {
+					// If we can't get the app (e.g., it was just deleted), retry the create
+					if apierr.IsMissing(getErr) {
+						return nil, retries.Continues("app was deleted, retrying create")
+					}
+					return nil, retries.Halt(err)
+				}
+				if existingApp.ComputeStatus != nil && existingApp.ComputeStatus.State == apps.ComputeStateDeleting {
+					return nil, retries.Continues("app is deleting, retrying create")
+				}
+				// App exists and is not being deleted - this is a hard error
+				return nil, retries.Halt(err)
+			}
+			return nil, retries.Halt(err)
+		}
+		return waiter.Response, nil
+	})
 	if err != nil {
 		return "", nil, err
 	}
 
-	return waiter.Response.Name, nil, nil
+	return app.Name, nil, nil
 }
 
 func (r *ResourceApp) DoUpdate(ctx context.Context, id string, config *apps.App, _ *Changes) (*apps.App, error) {
@@ -63,10 +88,7 @@ func (r *ResourceApp) DoUpdate(ctx context.Context, id string, config *apps.App,
 
 func (r *ResourceApp) DoDelete(ctx context.Context, id string) error {
 	_, err := r.client.Apps.DeleteByName(ctx, id)
-	if err != nil {
-		return err
-	}
-	return r.waitForDeletion(ctx, id)
+	return err
 }
 
 func (*ResourceApp) FieldTriggers(_ bool) map[string]deployplan.ActionType {
@@ -79,34 +101,6 @@ func (r *ResourceApp) WaitAfterCreate(ctx context.Context, config *apps.App) (*a
 	return r.waitForApp(ctx, r.client, config.Name)
 }
 
-func (r *ResourceApp) waitForDeletion(ctx context.Context, name string) error {
-	retrier := retries.New[struct{}](retries.WithTimeout(10*time.Minute), retries.WithRetryFunc(shouldRetry))
-	_, err := retrier.Run(ctx, func(ctx context.Context) (*struct{}, error) {
-		app, err := r.client.Apps.GetByName(ctx, name)
-		if err != nil {
-			if apierr.IsMissing(err) {
-				return nil, nil
-			}
-			return nil, retries.Halt(err)
-		}
-
-		if app.ComputeStatus == nil {
-			return nil, retries.Continues("waiting for compute status")
-		}
-
-		switch app.ComputeStatus.State {
-		case apps.ComputeStateDeleting:
-			return nil, retries.Continues("app is deleting")
-		case apps.ComputeStateActive, apps.ComputeStateStopped, apps.ComputeStateError:
-			err := fmt.Errorf("app %s was not deleted, current state: %s", name, app.ComputeStatus.State)
-			return nil, retries.Halt(err)
-		default:
-			return nil, retries.Continues(fmt.Sprintf("app is in %s state", app.ComputeStatus.State))
-		}
-	})
-	return err
-}
-
 // waitForApp waits for the app to reach the target state. The target state is either ACTIVE or STOPPED.
 // Apps with no_compute set to true will reach the STOPPED state, otherwise they will reach the ACTIVE state.
 // We can't use the default waiter from SDK because it only waits on ACTIVE state but we need also STOPPED state.

bundle/direct/dresources/app_test.go

@@ -0,0 +1,170 @@
+package dresources
+
+import (
+	"context"
+	"testing"
+
+	"github.com/databricks/cli/libs/testserver"
+	"github.com/databricks/databricks-sdk-go"
+	"github.com/databricks/databricks-sdk-go/service/apps"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestAppDoCreate_RetriesWhenAppIsDeleting verifies that DoCreate retries when
+// an app already exists but is in DELETING state.
+func TestAppDoCreate_RetriesWhenAppIsDeleting(t *testing.T) {
+	server := testserver.New(t)
+
+	createCallCount := 0
+	getCallCount := 0
+
+	server.Handle("POST", "/api/2.0/apps", func(req testserver.Request) any {
+		createCallCount++
+		if createCallCount == 1 {
+			return testserver.Response{
+				StatusCode: 409,
+				Body: map[string]string{
+					"error_code": "RESOURCE_ALREADY_EXISTS",
+					"message":    "An app with the same name already exists.",
+				},
+			}
+		}
+		return apps.App{
+			Name: "test-app",
+			ComputeStatus: &apps.ComputeStatus{
+				State: apps.ComputeStateActive,
+			},
+		}
+	})
+
+	server.Handle("GET", "/api/2.0/apps/{name}", func(req testserver.Request) any {
+		getCallCount++
+		return apps.App{
+			Name: req.Vars["name"],
+			ComputeStatus: &apps.ComputeStatus{
+				State: apps.ComputeStateDeleting,
+			},
+		}
+	})
+
+	testserver.AddDefaultHandlers(server)
+
+	client, err := databricks.NewWorkspaceClient(&databricks.Config{
+		Host:  server.URL,
+		Token: "testtoken",
+	})
+	require.NoError(t, err)
+
+	r := (&ResourceApp{}).New(client)
+	ctx := context.Background()
+	name, _, err := r.DoCreate(ctx, &apps.App{Name: "test-app"})
+
+	require.NoError(t, err)
+	assert.Equal(t, "test-app", name)
+	assert.Equal(t, 2, createCallCount, "expected Create to be called twice (1 retry)")
+	assert.Equal(t, 1, getCallCount, "expected Get to be called once to check app state")
+}
+
+// TestAppDoCreate_RetriesWhenGetReturnsNotFound verifies that DoCreate retries
+// when the app was just deleted between the create call and the get call.
+func TestAppDoCreate_RetriesWhenGetReturnsNotFound(t *testing.T) {
+	server := testserver.New(t)
+
+	createCallCount := 0
+	getCallCount := 0
+
+	server.Handle("POST", "/api/2.0/apps", func(req testserver.Request) any {
+		createCallCount++
+		if createCallCount == 1 {
+			return testserver.Response{
+				StatusCode: 409,
+				Body: map[string]string{
+					"error_code": "RESOURCE_ALREADY_EXISTS",
+					"message":    "An app with the same name already exists.",
+				},
+			}
+		}
+		return apps.App{
+			Name: "test-app",
+			ComputeStatus: &apps.ComputeStatus{
+				State: apps.ComputeStateActive,
+			},
+		}
+	})
+
+	server.Handle("GET", "/api/2.0/apps/{name}", func(req testserver.Request) any {
+		getCallCount++
+		return testserver.Response{
+			StatusCode: 404,
+			Body: map[string]string{
+				"error_code": "RESOURCE_DOES_NOT_EXIST",
+				"message":    "App not found.",
+			},
+		}
+	})
+
+	testserver.AddDefaultHandlers(server)
+
+	client, err := databricks.NewWorkspaceClient(&databricks.Config{
+		Host:  server.URL,
+		Token: "testtoken",
+	})
+	require.NoError(t, err)
+
+	r := (&ResourceApp{}).New(client)
+	ctx := context.Background()
+	name, _, err := r.DoCreate(ctx, &apps.App{Name: "test-app"})
+
+	require.NoError(t, err)
+	assert.Equal(t, "test-app", name)
+	assert.Equal(t, 2, createCallCount, "expected Create to be called twice")
+	assert.Equal(t, 1, getCallCount, "expected Get to be called once to check app state")
+}
+
+// TestAppDoCreate_FailsWhenAppExistsAndNotDeleting verifies that DoCreate returns
+// a hard error when an app already exists but is NOT in DELETING state.
+func TestAppDoCreate_FailsWhenAppExistsAndNotDeleting(t *testing.T) {
+	server := testserver.New(t)
+
+	createCallCount := 0
+	getCallCount := 0
+
+	server.Handle("POST", "/api/2.0/apps", func(req testserver.Request) any {
+		createCallCount++
+		return testserver.Response{
+			StatusCode: 409,
+			Body: map[string]string{
+				"error_code": "RESOURCE_ALREADY_EXISTS",
+				"message":    "An app with the same name already exists.",
+			},
+		}
+	})
+
+	server.Handle("GET", "/api/2.0/apps/{name}", func(req testserver.Request) any {
+		getCallCount++
+		return apps.App{
+			Name: req.Vars["name"],
+			ComputeStatus: &apps.ComputeStatus{
+				State: apps.ComputeStateActive,
+			},
+		}
+	})
+
+	testserver.AddDefaultHandlers(server)
+
+	client, err := databricks.NewWorkspaceClient(&databricks.Config{
+		Host:  server.URL,
+		Token: "testtoken",
+	})
+	require.NoError(t, err)
+
+	r := (&ResourceApp{}).New(client)
+	ctx := context.Background()
+	_, _, err = r.DoCreate(ctx, &apps.App{Name: "test-app"})
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "already exists")
+	assert.Equal(t, 1, createCallCount, "expected Create to be called only once")
+	assert.Equal(t, 1, getCallCount, "expected Get to be called once to check app state")
+}