Skip to content

Commit 5ce6557

Browse files
Check app state before retrying on RESOURCE_ALREADY_EXISTS
1 parent bed765e commit 5ce6557

File tree

3 files changed

+173
-78
lines changed

3 files changed

+173
-78
lines changed

bundle/direct/dresources/all_test.go

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,3 +719,161 @@ func jsonDump(obj any) string {
719719
}
720720
return string(bytes)
721721
}
722+
723+
// TestAppDoCreate_RetriesWhenAppIsDeleting verifies that DoCreate retries when
724+
// an app already exists but is in DELETING state.
725+
func TestAppDoCreate_RetriesWhenAppIsDeleting(t *testing.T) {
726+
server := testserver.New(t)
727+
728+
createCallCount := 0
729+
getCallCount := 0
730+
731+
server.Handle("POST", "/api/2.0/apps", func(req testserver.Request) any {
732+
createCallCount++
733+
if createCallCount == 1 {
734+
return testserver.Response{
735+
StatusCode: 409,
736+
Body: map[string]string{
737+
"error_code": "RESOURCE_ALREADY_EXISTS",
738+
"message": "An app with the same name already exists.",
739+
},
740+
}
741+
}
742+
return apps.App{
743+
Name: "test-app",
744+
ComputeStatus: &apps.ComputeStatus{
745+
State: apps.ComputeStateActive,
746+
},
747+
}
748+
})
749+
750+
server.Handle("GET", "/api/2.0/apps/{name}", func(req testserver.Request) any {
751+
getCallCount++
752+
return apps.App{
753+
Name: req.Vars["name"],
754+
ComputeStatus: &apps.ComputeStatus{
755+
State: apps.ComputeStateDeleting,
756+
},
757+
}
758+
})
759+
760+
testserver.AddDefaultHandlers(server)
761+
762+
client, err := databricks.NewWorkspaceClient(&databricks.Config{
763+
Host: server.URL,
764+
Token: "testtoken",
765+
})
766+
require.NoError(t, err)
767+
768+
r := (&ResourceApp{}).New(client)
769+
ctx := context.Background()
770+
name, _, err := r.DoCreate(ctx, &apps.App{Name: "test-app"})
771+
772+
require.NoError(t, err)
773+
assert.Equal(t, "test-app", name)
774+
assert.Equal(t, 2, createCallCount, "expected Create to be called twice (1 retry)")
775+
assert.Equal(t, 1, getCallCount, "expected Get to be called once to check app state")
776+
}
777+
778+
// TestAppDoCreate_RetriesWhenGetReturnsNotFound verifies that DoCreate retries
779+
// when the app was just deleted between the create call and the get call.
780+
func TestAppDoCreate_RetriesWhenGetReturnsNotFound(t *testing.T) {
781+
server := testserver.New(t)
782+
783+
createCallCount := 0
784+
getCallCount := 0
785+
786+
server.Handle("POST", "/api/2.0/apps", func(req testserver.Request) any {
787+
createCallCount++
788+
if createCallCount == 1 {
789+
return testserver.Response{
790+
StatusCode: 409,
791+
Body: map[string]string{
792+
"error_code": "RESOURCE_ALREADY_EXISTS",
793+
"message": "An app with the same name already exists.",
794+
},
795+
}
796+
}
797+
return apps.App{
798+
Name: "test-app",
799+
ComputeStatus: &apps.ComputeStatus{
800+
State: apps.ComputeStateActive,
801+
},
802+
}
803+
})
804+
805+
server.Handle("GET", "/api/2.0/apps/{name}", func(req testserver.Request) any {
806+
getCallCount++
807+
return testserver.Response{
808+
StatusCode: 404,
809+
Body: map[string]string{
810+
"error_code": "RESOURCE_DOES_NOT_EXIST",
811+
"message": "App not found.",
812+
},
813+
}
814+
})
815+
816+
testserver.AddDefaultHandlers(server)
817+
818+
client, err := databricks.NewWorkspaceClient(&databricks.Config{
819+
Host: server.URL,
820+
Token: "testtoken",
821+
})
822+
require.NoError(t, err)
823+
824+
r := (&ResourceApp{}).New(client)
825+
ctx := context.Background()
826+
name, _, err := r.DoCreate(ctx, &apps.App{Name: "test-app"})
827+
828+
require.NoError(t, err)
829+
assert.Equal(t, "test-app", name)
830+
assert.Equal(t, 2, createCallCount, "expected Create to be called twice")
831+
assert.Equal(t, 1, getCallCount, "expected Get to be called once to check app state")
832+
}
833+
834+
// TestAppDoCreate_FailsWhenAppExistsAndNotDeleting verifies that DoCreate returns
835+
// a hard error when an app already exists but is NOT in DELETING state.
836+
func TestAppDoCreate_FailsWhenAppExistsAndNotDeleting(t *testing.T) {
837+
server := testserver.New(t)
838+
839+
createCallCount := 0
840+
getCallCount := 0
841+
842+
server.Handle("POST", "/api/2.0/apps", func(req testserver.Request) any {
843+
createCallCount++
844+
return testserver.Response{
845+
StatusCode: 409,
846+
Body: map[string]string{
847+
"error_code": "RESOURCE_ALREADY_EXISTS",
848+
"message": "An app with the same name already exists.",
849+
},
850+
}
851+
})
852+
853+
server.Handle("GET", "/api/2.0/apps/{name}", func(req testserver.Request) any {
854+
getCallCount++
855+
return apps.App{
856+
Name: req.Vars["name"],
857+
ComputeStatus: &apps.ComputeStatus{
858+
State: apps.ComputeStateActive,
859+
},
860+
}
861+
})
862+
863+
testserver.AddDefaultHandlers(server)
864+
865+
client, err := databricks.NewWorkspaceClient(&databricks.Config{
866+
Host: server.URL,
867+
Token: "testtoken",
868+
})
869+
require.NoError(t, err)
870+
871+
r := (&ResourceApp{}).New(client)
872+
ctx := context.Background()
873+
_, _, err = r.DoCreate(ctx, &apps.App{Name: "test-app"})
874+
875+
require.Error(t, err)
876+
assert.Contains(t, err.Error(), "already exists")
877+
assert.Equal(t, 1, createCallCount, "expected Create to be called only once")
878+
assert.Equal(t, 1, getCallCount, "expected Get to be called once to check app state")
879+
}

bundle/direct/dresources/app.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,25 @@ func (r *ResourceApp) DoCreate(ctx context.Context, config *apps.App) (string, *
3838
ForceSendFields: nil,
3939
}
4040

41-
retrier := retries.New[apps.App](retries.WithTimeout(time.Minute), retries.WithRetryFunc(shouldRetry))
41+
retrier := retries.New[apps.App](retries.WithTimeout(15*time.Minute), retries.WithRetryFunc(shouldRetry))
4242
app, err := retrier.Run(ctx, func(ctx context.Context) (*apps.App, error) {
4343
waiter, err := r.client.Apps.Create(ctx, request)
4444
if err != nil {
4545
if errors.Is(err, apierr.ErrResourceAlreadyExists) {
46-
return nil, retries.Continues("app already exists, retrying")
46+
// Check if the app is in DELETING state - only then should we retry
47+
existingApp, getErr := r.client.Apps.GetByName(ctx, config.Name)
48+
if getErr != nil {
49+
// If we can't get the app (e.g., it was just deleted), retry the create
50+
if apierr.IsMissing(getErr) {
51+
return nil, retries.Continues("app was deleted, retrying create")
52+
}
53+
return nil, retries.Halt(err)
54+
}
55+
if existingApp.ComputeStatus != nil && existingApp.ComputeStatus.State == apps.ComputeStateDeleting {
56+
return nil, retries.Continues("app is deleting, retrying create")
57+
}
58+
// App exists and is not being deleted - this is a hard error
59+
return nil, retries.Halt(err)
4760
}
4861
return nil, retries.Halt(err)
4962
}

bundle/direct/dresources/app_test.go

Lines changed: 0 additions & 76 deletions
This file was deleted.

0 commit comments

Comments
 (0)