Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check for Resource Already Exist Error before retry #243

Merged
merged 3 commits into from
Jul 9, 2024
Merged

Check for Resource Already Exist Error before retry #243

merged 3 commits into from
Jul 9, 2024

Conversation

uzaxirr
Copy link
Member

@uzaxirr uzaxirr commented Jun 24, 2024 •
edited
Loading

Fixes: #234

@uzaxirr uzaxirr self-assigned this Jun 24, 2024
@uzaxirr uzaxirr mentioned this pull request Jun 24, 2024
Closed
vishalanarase
vishalanarase reviewed Jun 24, 2024
View reviewed changes
civo/instances/resource_instance.go
if err != nil {
return diag.Errorf("[ERR] failed to create instance after multiple attempts: %s", err)
if errors.Is(err, civogo.DatabaseInstanceDuplicateNameError) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the only error we want to verify? There are multiple errors are present, What will happen to those?

vishalanarase
vishalanarase reviewed Jun 24, 2024
View reviewed changes
civo/network/resource_network.go
if err != nil {

// Check for the resource already exists error
if errors.Is(err, civogo.DatabaseNetworkExistsError) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above

@uzaxirr
Copy link
Member Author

uzaxirr commented Jun 24, 2024

@vishalanarase there are a ton of errors in the SDK not feasible to check for all of them. Duplicate name is the most common one hence checking for that only to improve the experience. for other errors tf will error out after all the retry. But for Duplicate Resource it will error out immediately without retry.

fernando-villalba
fernando-villalba approved these changes Jul 8, 2024
View reviewed changes
Copy link
Contributor

@fernando-villalba fernando-villalba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good, but there may be other resources aside from network and instances, may be worth checking kubernetes, and others as well.

@uzaxirr uzaxirr merged commit c225818 into master Jul 9, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vishalanarase vishalanarase vishalanarase left review comments

@fernando-villalba fernando-villalba fernando-villalba approved these changes

@alejandrojnm alejandrojnm Awaiting requested review from alejandrojnm

@alessandroargentieri alessandroargentieri Awaiting requested review from alessandroargentieri alessandroargentieri is a code owner

Assignees

@uzaxirr uzaxirr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] When creating a resource with Terraform that has already been created somewhere else it hangs...
3 participants
@uzaxirr @fernando-villalba @vishalanarase