civo / terraform-provider-civo

Terraform Civo provider
https://www.civo.com
Mozilla Public License 2.0
66 stars 51 forks source link

[BUG] When creating a resource with a public ip address in a different region, quota increases without resources being created #249

Closed fernando-villalba closed 4 days ago

fernando-villalba commented 2 weeks ago

Issue

By accident I forgot to set the default region to LON1 (default was NYC1) while specifying the region of the instance to LON1 and this is what happened when I applied this code:

resource "civo_reserved_ip" "www" {
    name = "test-ip" 
}

resource "civo_instance" "foo" {
    hostname = "amazingthingy20"
    tags = ["python", "nginx"]
    notes = "This is a test-issue"
    size = "g3.xsmall"
    region = "LON1"
    reserved_ipv4 = civo_reserved_ip.www.id
    firewall_id = "default-default"
    disk_image = "debian-10"
}

The following happens when applying the code above:

  1. the reserved ip gets created
  2. The civo instance resource does NOT get created
  3. Instead it keeps trying to create the resource while the quota for CPU, Disk, public ip addresses, Volumes and instances keeps increasing. After cancelling the action takes some time until the quota clears.

Applying:

image

If you go into the quota page as you do that and keep refreshing, you will see it progressively balloon until all is filled:

image

Acceptance Criteria

Praveen005 commented 2 weeks ago

Hi @uzaxirr Can I work on this issue?

uzaxirr commented 2 weeks ago

Hey @Praveen005 can you outline your approach here.

Praveen005 commented 2 weeks ago

@uzaxirr, Here's my assessment of the problem:

  1. It first creates the civo_reserved_ip resource with the default region, NYC1
  2. It then tries to create the instance resource with region LON1, if unsuccessful, retries.
  3. The likely reason for resource ballooning is, each retry will make an API call for instance creation, and for each such retry, some ancillary resources quota gets reserved.

Now, I feel, to solve this issue, we should fail at the very first instance when there is a region mismatch(add checks in resourceInstanceCreate()) and don't retry.

While this may likely address the issue at hand, but I feel this ghost resource creation should be handled in a better way on the API side.

I'd appreciate your input on this approach.

Thank you.

Praveen005 commented 1 week ago

Hi @uzaxirr,

Whenever you have a moment, could you please review my approach?

Thank you! Praveen

Praveen005 commented 4 days ago

Hi @uzaxirr,

275 fixes this issue it seems.

I tried recreating it, but rightly got an error:

main.tf:

Screenshot 2024-07-24 214353

Error correctly getting logged:

Screenshot 2024-07-24 214054
uzaxirr commented 4 days ago

closing wrt: https://github.com/civo/terraform-provider-civo/pull/275