From 8a98365e10e2ef3e40b652c682e27f656d35b233 Mon Sep 17 00:00:00 2001 From: Varun Deep Saini Date: Sat, 6 Dec 2025 21:08:46 +0530 Subject: [PATCH 1/3] Wait for app in DELETING state before create --- .../pluginfw/products/app/resource_app.go | 23 +++++ .../products/app/resource_app_test.go | 83 +++++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 internal/providers/pluginfw/products/app/resource_app_test.go diff --git a/internal/providers/pluginfw/products/app/resource_app.go b/internal/providers/pluginfw/products/app/resource_app.go index 5f86998770..eb7facbc42 100644 --- a/internal/providers/pluginfw/products/app/resource_app.go +++ b/internal/providers/pluginfw/products/app/resource_app.go @@ -127,6 +127,11 @@ func (a *resourceApp) Create(ctx context.Context, req resource.CreateRequest, re return } + if err := waitForAppDeleted(ctx, w, appGoSdk.Name); err != nil { + resp.Diagnostics.AddError("failed to wait for app deletion", err.Error()) + return + } + // Create the app var forceSendFields []string if !app.NoCompute.IsNull() { @@ -211,6 +216,24 @@ func (a *resourceApp) waitForApp(ctx context.Context, w *databricks.WorkspaceCli }) } +func waitForAppDeleted(ctx context.Context, w *databricks.WorkspaceClient, name string) error { + retrier := retries.New[struct{}](retries.WithTimeout(-1), retries.WithRetryFunc(shouldRetry)) + _, err := retrier.Run(ctx, func(ctx context.Context) (*struct{}, error) { + app, err := w.Apps.GetByName(ctx, name) + if apierr.IsMissing(err) { + return &struct{}{}, nil + } + if err != nil { + return nil, retries.Halt(err) + } + if app.ComputeStatus.State == apps.ComputeStateDeleting { + return nil, retries.Continues(fmt.Sprintf("app %s is still deleting", name)) + } + return &struct{}{}, nil + }) + return err +} + func (a *resourceApp) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { ctx = pluginfwcontext.SetUserAgentInResourceContext(ctx, resourceName) diff --git a/internal/providers/pluginfw/products/app/resource_app_test.go b/internal/providers/pluginfw/products/app/resource_app_test.go new file mode 100644 index 0000000000..d596f321ed --- /dev/null +++ b/internal/providers/pluginfw/products/app/resource_app_test.go @@ -0,0 +1,83 @@ +package app + +import ( + "context" + "testing" + + "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" +) + +func TestWaitForAppDeleted_AppDoesNotExist(t *testing.T) { + ctx := context.Background() + mockClient := mocks.NewMockWorkspaceClient(t) + mockAppsAPI := mockClient.GetMockAppsAPI() + + mockAppsAPI.EXPECT().GetByName(mock.Anything, "test-app").Return(nil, &apierr.APIError{ + StatusCode: 404, + Message: "App not found", + }) + + err := waitForAppDeleted(ctx, mockClient.WorkspaceClient, "test-app") + assert.NoError(t, err) +} + +func TestWaitForAppDeleted_AppInDeletingState(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + mockClient := mocks.NewMockWorkspaceClient(t) + mockAppsAPI := mockClient.GetMockAppsAPI() + + mockAppsAPI.EXPECT().GetByName(mock.Anything, "test-app").Return(&apps.App{ + Name: "test-app", + ComputeStatus: &apps.ComputeStatus{ + State: apps.ComputeStateDeleting, + Message: "App is being deleted", + }, + }, nil).Once() + + mockAppsAPI.EXPECT().GetByName(mock.Anything, "test-app").Return(nil, &apierr.APIError{ + StatusCode: 404, + Message: "App not found", + }).Run(func(_ context.Context, _ string) { + cancel() + }).Once() + + err := waitForAppDeleted(ctx, mockClient.WorkspaceClient, "test-app") + assert.NoError(t, err) +} + +func TestWaitForAppDeleted_AppNotInDeletingState_ReturnsImmediately(t *testing.T) { + ctx := context.Background() + mockClient := mocks.NewMockWorkspaceClient(t) + mockAppsAPI := mockClient.GetMockAppsAPI() + + // If app exists but is not in DELETING state, return immediately. + // The subsequent Create() call will fail with a proper API error. + mockAppsAPI.EXPECT().GetByName(mock.Anything, "test-app").Return(&apps.App{ + Name: "test-app", + ComputeStatus: &apps.ComputeStatus{ + State: apps.ComputeStateActive, + Message: "App is active", + }, + }, nil).Once() + + err := waitForAppDeleted(ctx, mockClient.WorkspaceClient, "test-app") + assert.NoError(t, err) +} + +func TestWaitForAppDeleted_APIError(t *testing.T) { + ctx := context.Background() + mockClient := mocks.NewMockWorkspaceClient(t) + mockAppsAPI := mockClient.GetMockAppsAPI() + mockAppsAPI.EXPECT().GetByName(mock.Anything, "test-app").Return(nil, &apierr.APIError{ + StatusCode: 500, + Message: "Internal server error", + }) + + err := waitForAppDeleted(ctx, mockClient.WorkspaceClient, "test-app") + assert.Error(t, err) + assert.Contains(t, err.Error(), "Internal server error") +} From a778056f4c9583c3e222855a48ae6a1548b09c18 Mon Sep 17 00:00:00 2001 From: Varun Deep Saini Date: Tue, 9 Dec 2025 17:45:43 +0530 Subject: [PATCH 2/3] changed the waiting to happen in the delete function --- .../pluginfw/products/app/resource_app.go | 27 ++++++++++++------- .../products/app/resource_app_test.go | 15 +++++------ 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/internal/providers/pluginfw/products/app/resource_app.go b/internal/providers/pluginfw/products/app/resource_app.go index eb7facbc42..0b9529fc3a 100644 --- a/internal/providers/pluginfw/products/app/resource_app.go +++ b/internal/providers/pluginfw/products/app/resource_app.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "slices" + "time" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/apierr" @@ -27,6 +28,7 @@ import ( const ( resourceName = "app" resourceNamePlural = "apps" + appDeletionTimeout = 10 * time.Minute ) type AppResource struct { @@ -127,11 +129,6 @@ func (a *resourceApp) Create(ctx context.Context, req resource.CreateRequest, re return } - if err := waitForAppDeleted(ctx, w, appGoSdk.Name); err != nil { - resp.Diagnostics.AddError("failed to wait for app deletion", err.Error()) - return - } - // Create the app var forceSendFields []string if !app.NoCompute.IsNull() { @@ -217,7 +214,7 @@ func (a *resourceApp) waitForApp(ctx context.Context, w *databricks.WorkspaceCli } func waitForAppDeleted(ctx context.Context, w *databricks.WorkspaceClient, name string) error { - retrier := retries.New[struct{}](retries.WithTimeout(-1), retries.WithRetryFunc(shouldRetry)) + retrier := retries.New[struct{}](retries.WithTimeout(appDeletionTimeout), retries.WithRetryFunc(shouldRetry)) _, err := retrier.Run(ctx, func(ctx context.Context) (*struct{}, error) { app, err := w.Apps.GetByName(ctx, name) if apierr.IsMissing(err) { @@ -226,10 +223,17 @@ func waitForAppDeleted(ctx context.Context, w *databricks.WorkspaceClient, name if err != nil { return nil, retries.Halt(err) } - if app.ComputeStatus.State == apps.ComputeStateDeleting { - return nil, retries.Continues(fmt.Sprintf("app %s is still deleting", name)) + 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: + return nil, retries.Halt(fmt.Errorf("app %s was not deleted, current state: %s", name, app.ComputeStatus.State)) + default: + return nil, retries.Continues(fmt.Sprintf("app is in %s state", app.ComputeStatus.State)) } - return &struct{}{}, nil }) return err } @@ -349,6 +353,11 @@ func (a *resourceApp) Delete(ctx context.Context, req resource.DeleteRequest, re resp.Diagnostics.AddError("failed to delete app", err.Error()) return } + + if err := waitForAppDeleted(ctx, w, app.Name.ValueString()); err != nil { + resp.Diagnostics.AddError("failed to wait for app deletion", err.Error()) + return + } } func (a *resourceApp) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { diff --git a/internal/providers/pluginfw/products/app/resource_app_test.go b/internal/providers/pluginfw/products/app/resource_app_test.go index d596f321ed..eefcba13f4 100644 --- a/internal/providers/pluginfw/products/app/resource_app_test.go +++ b/internal/providers/pluginfw/products/app/resource_app_test.go @@ -25,7 +25,7 @@ func TestWaitForAppDeleted_AppDoesNotExist(t *testing.T) { assert.NoError(t, err) } -func TestWaitForAppDeleted_AppInDeletingState(t *testing.T) { +func TestWaitForAppDeleted_WaitsUntilDeleted(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) mockClient := mocks.NewMockWorkspaceClient(t) mockAppsAPI := mockClient.GetMockAppsAPI() @@ -33,8 +33,7 @@ func TestWaitForAppDeleted_AppInDeletingState(t *testing.T) { mockAppsAPI.EXPECT().GetByName(mock.Anything, "test-app").Return(&apps.App{ Name: "test-app", ComputeStatus: &apps.ComputeStatus{ - State: apps.ComputeStateDeleting, - Message: "App is being deleted", + State: apps.ComputeStateDeleting, }, }, nil).Once() @@ -49,23 +48,21 @@ func TestWaitForAppDeleted_AppInDeletingState(t *testing.T) { assert.NoError(t, err) } -func TestWaitForAppDeleted_AppNotInDeletingState_ReturnsImmediately(t *testing.T) { +func TestWaitForAppDeleted_AppNotDeleted_Halts(t *testing.T) { ctx := context.Background() mockClient := mocks.NewMockWorkspaceClient(t) mockAppsAPI := mockClient.GetMockAppsAPI() - // If app exists but is not in DELETING state, return immediately. - // The subsequent Create() call will fail with a proper API error. mockAppsAPI.EXPECT().GetByName(mock.Anything, "test-app").Return(&apps.App{ Name: "test-app", ComputeStatus: &apps.ComputeStatus{ - State: apps.ComputeStateActive, - Message: "App is active", + State: apps.ComputeStateActive, }, }, nil).Once() err := waitForAppDeleted(ctx, mockClient.WorkspaceClient, "test-app") - assert.NoError(t, err) + assert.Error(t, err) + assert.Contains(t, err.Error(), "was not deleted") } func TestWaitForAppDeleted_APIError(t *testing.T) { From 10b6e91d2b3e7853b721b75d6ee7eaaf22c03657 Mon Sep 17 00:00:00 2001 From: Varun Deep Saini Date: Mon, 29 Dec 2025 19:20:49 +0530 Subject: [PATCH 3/3] changed back to waiting in Creation --- .../pluginfw/products/app/resource_app.go | 50 ++++-------- .../products/app/resource_app_test.go | 80 ------------------- 2 files changed, 16 insertions(+), 114 deletions(-) delete mode 100644 internal/providers/pluginfw/products/app/resource_app_test.go diff --git a/internal/providers/pluginfw/products/app/resource_app.go b/internal/providers/pluginfw/products/app/resource_app.go index 0b9529fc3a..b4fcf1f2c8 100644 --- a/internal/providers/pluginfw/products/app/resource_app.go +++ b/internal/providers/pluginfw/products/app/resource_app.go @@ -2,6 +2,7 @@ package app import ( "context" + "errors" "fmt" "reflect" "slices" @@ -28,7 +29,7 @@ import ( const ( resourceName = "app" resourceNamePlural = "apps" - appDeletionTimeout = 10 * time.Minute + appCreateTimeout = 1 * time.Minute ) type AppResource struct { @@ -129,15 +130,26 @@ func (a *resourceApp) Create(ctx context.Context, req resource.CreateRequest, re return } - // Create the app var forceSendFields []string if !app.NoCompute.IsNull() { forceSendFields = append(forceSendFields, "NoCompute") } - waiter, err := w.Apps.Create(ctx, apps.CreateAppRequest{ + createReq := apps.CreateAppRequest{ App: appGoSdk, NoCompute: app.NoCompute.ValueBool(), ForceSendFields: forceSendFields, + } + + retrier := retries.New[apps.App](retries.WithTimeout(appCreateTimeout), retries.WithRetryFunc(shouldRetry)) + createdApp, err := retrier.Run(ctx, func(ctx context.Context) (*apps.App, error) { + waiter, err := w.Apps.Create(ctx, createReq) + 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 { resp.Diagnostics.AddError("failed to create app", err.Error()) @@ -146,7 +158,7 @@ func (a *resourceApp) Create(ctx context.Context, req resource.CreateRequest, re // Store the initial version of the app in state var newApp AppResource - resp.Diagnostics.Append(converters.GoSdkToTfSdkStruct(ctx, waiter.Response, &newApp)...) + resp.Diagnostics.Append(converters.GoSdkToTfSdkStruct(ctx, createdApp, &newApp)...) if resp.Diagnostics.HasError() { return } @@ -213,31 +225,6 @@ func (a *resourceApp) waitForApp(ctx context.Context, w *databricks.WorkspaceCli }) } -func waitForAppDeleted(ctx context.Context, w *databricks.WorkspaceClient, name string) error { - retrier := retries.New[struct{}](retries.WithTimeout(appDeletionTimeout), retries.WithRetryFunc(shouldRetry)) - _, err := retrier.Run(ctx, func(ctx context.Context) (*struct{}, error) { - app, err := w.Apps.GetByName(ctx, name) - if apierr.IsMissing(err) { - return &struct{}{}, nil - } - if err != 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: - return nil, retries.Halt(fmt.Errorf("app %s was not deleted, current state: %s", name, app.ComputeStatus.State)) - default: - return nil, retries.Continues(fmt.Sprintf("app is in %s state", app.ComputeStatus.State)) - } - }) - return err -} - func (a *resourceApp) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { ctx = pluginfwcontext.SetUserAgentInResourceContext(ctx, resourceName) @@ -353,11 +340,6 @@ func (a *resourceApp) Delete(ctx context.Context, req resource.DeleteRequest, re resp.Diagnostics.AddError("failed to delete app", err.Error()) return } - - if err := waitForAppDeleted(ctx, w, app.Name.ValueString()); err != nil { - resp.Diagnostics.AddError("failed to wait for app deletion", err.Error()) - return - } } func (a *resourceApp) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { diff --git a/internal/providers/pluginfw/products/app/resource_app_test.go b/internal/providers/pluginfw/products/app/resource_app_test.go deleted file mode 100644 index eefcba13f4..0000000000 --- a/internal/providers/pluginfw/products/app/resource_app_test.go +++ /dev/null @@ -1,80 +0,0 @@ -package app - -import ( - "context" - "testing" - - "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" -) - -func TestWaitForAppDeleted_AppDoesNotExist(t *testing.T) { - ctx := context.Background() - mockClient := mocks.NewMockWorkspaceClient(t) - mockAppsAPI := mockClient.GetMockAppsAPI() - - mockAppsAPI.EXPECT().GetByName(mock.Anything, "test-app").Return(nil, &apierr.APIError{ - StatusCode: 404, - Message: "App not found", - }) - - err := waitForAppDeleted(ctx, mockClient.WorkspaceClient, "test-app") - assert.NoError(t, err) -} - -func TestWaitForAppDeleted_WaitsUntilDeleted(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - mockClient := mocks.NewMockWorkspaceClient(t) - mockAppsAPI := mockClient.GetMockAppsAPI() - - mockAppsAPI.EXPECT().GetByName(mock.Anything, "test-app").Return(&apps.App{ - Name: "test-app", - ComputeStatus: &apps.ComputeStatus{ - State: apps.ComputeStateDeleting, - }, - }, nil).Once() - - mockAppsAPI.EXPECT().GetByName(mock.Anything, "test-app").Return(nil, &apierr.APIError{ - StatusCode: 404, - Message: "App not found", - }).Run(func(_ context.Context, _ string) { - cancel() - }).Once() - - err := waitForAppDeleted(ctx, mockClient.WorkspaceClient, "test-app") - assert.NoError(t, err) -} - -func TestWaitForAppDeleted_AppNotDeleted_Halts(t *testing.T) { - ctx := context.Background() - mockClient := mocks.NewMockWorkspaceClient(t) - mockAppsAPI := mockClient.GetMockAppsAPI() - - mockAppsAPI.EXPECT().GetByName(mock.Anything, "test-app").Return(&apps.App{ - Name: "test-app", - ComputeStatus: &apps.ComputeStatus{ - State: apps.ComputeStateActive, - }, - }, nil).Once() - - err := waitForAppDeleted(ctx, mockClient.WorkspaceClient, "test-app") - assert.Error(t, err) - assert.Contains(t, err.Error(), "was not deleted") -} - -func TestWaitForAppDeleted_APIError(t *testing.T) { - ctx := context.Background() - mockClient := mocks.NewMockWorkspaceClient(t) - mockAppsAPI := mockClient.GetMockAppsAPI() - mockAppsAPI.EXPECT().GetByName(mock.Anything, "test-app").Return(nil, &apierr.APIError{ - StatusCode: 500, - Message: "Internal server error", - }) - - err := waitForAppDeleted(ctx, mockClient.WorkspaceClient, "test-app") - assert.Error(t, err) - assert.Contains(t, err.Error(), "Internal server error") -}