removed status check, and added retries

Author: varundeepsaini

bundle/direct/dresources/app.go

@@ -2,6 +2,7 @@ package dresources
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -31,21 +32,28 @@ func (r *ResourceApp) DoRead(ctx context.Context, id string) (*apps.App, error)
 }
 
 func (r *ResourceApp) DoCreate(ctx context.Context, config *apps.App) (string, *apps.App, error) {
-	if err := r.waitForDeletion(ctx, config.Name); err != nil {
-		return "", nil, err
-	}
-
 	request := apps.CreateAppRequest{
 		App:             *config,
 		NoCompute:       true,
 		ForceSendFields: nil,
 	}
-	waiter, err := r.client.Apps.Create(ctx, request)
+
+	retrier := retries.New[apps.App](retries.WithTimeout(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) {
+				return nil, retries.Continues("app already exists, retrying")
+			}
+			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) {
@@ -80,32 +88,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")
-		default:
-			err := fmt.Errorf("app %s already exists", name)
-			return nil, retries.Halt(err)
-		}
-	})
-	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,76 @@
+package dresources
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/databricks/databricks-sdk-go/apierr"
+	"github.com/databricks/databricks-sdk-go/experimental/mocks"
+	"github.com/databricks/databricks-sdk-go/service/apps"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDoCreate_RetriesOnAlreadyExists(t *testing.T) {
+	ctx := context.Background()
+	m := mocks.NewMockWorkspaceClient(t)
+	appsAPI := m.GetMockAppsAPI()
+
+	callCount := 0
+	appsAPI.EXPECT().Create(mock.Anything, mock.Anything).RunAndReturn(
+		func(ctx context.Context, req apps.CreateAppRequest) (*apps.WaitGetAppActive[apps.App], error) {
+			callCount++
+			if callCount == 1 {
+				return nil, apierr.ErrResourceAlreadyExists
+			}
+			return &apps.WaitGetAppActive[apps.App]{Response: &apps.App{Name: "test-app"}}, nil
+		},
+	)
+
+	r := (&ResourceApp{}).New(m.WorkspaceClient)
+	name, _, err := r.DoCreate(ctx, &apps.App{Name: "test-app"})
+
+	require.NoError(t, err)
+	assert.Equal(t, "test-app", name)
+	assert.Equal(t, 2, callCount, "expected Create to be called twice (1 retry)")
+}
+
+func TestDoCreate_FailsAfterTimeout(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	m := mocks.NewMockWorkspaceClient(t)
+	appsAPI := m.GetMockAppsAPI()
+
+	callCount := 0
+	appsAPI.EXPECT().Create(mock.Anything, mock.Anything).RunAndReturn(
+		func(ctx context.Context, req apps.CreateAppRequest) (*apps.WaitGetAppActive[apps.App], error) {
+			callCount++
+			return nil, apierr.ErrResourceAlreadyExists
+		},
+	)
+
+	r := (&ResourceApp{}).New(m.WorkspaceClient)
+	_, _, err := r.DoCreate(ctx, &apps.App{Name: "test-app"})
+
+	require.Error(t, err)
+	assert.Greater(t, callCount, 1, "expected Create to be called multiple times before timeout")
+}
+
+func TestDoCreate_FailsImmediatelyOnOtherErrors(t *testing.T) {
+	ctx := context.Background()
+	m := mocks.NewMockWorkspaceClient(t)
+	appsAPI := m.GetMockAppsAPI()
+
+	// Return a different error - should not retry
+	appsAPI.EXPECT().Create(mock.Anything, mock.Anything).
+		Return(nil, apierr.ErrPermissionDenied).Once()
+
+	r := (&ResourceApp{}).New(m.WorkspaceClient)
+	_, _, err := r.DoCreate(ctx, &apps.App{Name: "test-app"})
+
+	require.Error(t, err)
+	assert.ErrorIs(t, err, apierr.ErrPermissionDenied)
+}