cloudamqp / terraform-provider-cloudamqp

Terraform Provider for CloudAMQP
https://registry.terraform.io/providers/cloudamqp/cloudamqp
Mozilla Public License 2.0
35 stars 39 forks source link

Fail during destroying vpc #238

Closed ovitente closed 10 months ago

ovitente commented 11 months ago

I have simple module

resource "cloudamqp_vpc" "vpc" {
  region = var.region
  subnet = var.subnet
  name   = "cloudamqp"  # Replace with the actual required name.
  tags   = var.tags
}

resource "cloudamqp_instance" "instance" {
  depends_on  = [cloudamqp_vpc.vpc]
  name        = var.instance_name
  plan        = var.plan
  region      = cloudamqp_vpc.vpc.region
  vpc_id      = cloudamqp_vpc.vpc.id
  nodes       = var.nodes
  tags        = var.tags
  rmq_version = var.rmq_version
}

Which allows create vpc and cloudamqp instance under it, but it fails when I try to delete these resources.

It actually deletes them on CloudAMQP but returning error in terraform.

It seems it is some bug in cloudamqp provider.

Destroy log

Terraform will perform the following actions:

  # cloudamqp_instance.instance will be destroyed
  - resource "cloudamqp_instance" "instance" {
      - apikey              = (sensitive value)
      - backend             = "rabbitmq" -> null
      - dedicated           = true -> null
      - host                = "juicy-teal-fox.rmq4.cloudamqp.com" -> null
      - host_internal       = "juicy-teal-fox.in.rmq4.cloudamqp.com" -> null
      - id                  = "255441" -> null
      - keep_associated_vpc = false -> null
      - name                = "test-staging" -> null
      - nodes               = 1 -> null
      - plan                = "squirrel-1" -> null
      - ready               = true -> null
      - region              = "google-compute-engine::us-central1" -> null
      - rmq_version         = "3.12.2" -> null
      - tags                = [] -> null
      - url                 = (sensitive value)
      - vhost               = "ujopxmus" -> null
      - vpc_id              = 255440 -> null
    }

  # cloudamqp_vpc.vpc will be destroyed
  - resource "cloudamqp_vpc" "vpc" {
      - id       = "255440" -> null
      - name     = "cloudamqp" -> null
      - region   = "google-compute-engine::us-central1" -> null
      - subnet   = "10.20.0.0/24" -> null
      - tags     = [] -> null
      - vpc_name = "vpc-qfdj9hrs" -> null
    }

Plan: 0 to add, 0 to change, 2 to destroy.

Changes to Outputs:
  - cloudamqp_instance_id = "255441" -> null
  - cloudamqp_vpc_id      = "255440" -> null

Do you really want to destroy all resources?
  Terraform will destroy all your managed infrastructure, as shown above.
  There is no undo. Only 'yes' will be accepted to confirm.

  Enter a value: yes

cloudamqp_instance.instance: Destroying... [id=255441]
cloudamqp_instance.instance: Still destroying... [id=255441, 10s elapsed]
cloudamqp_instance.instance: Destruction complete after 11s
cloudamqp_vpc.vpc: Destroying... [id=255440]
╷
│ Error: DeleteVpcInstance failed, status: 404, message: map[error:No VPC found with ID: 255440]
│
│
╵
Releasing state lock. This may take a few moments...
ERRO[0023] Terraform invocation failed in /myproject-terraform/live/cloudamqp/staging/gcp/us-central1/test/.terragrunt-cache/wBTxO4K52zWBVwWntUm91zYH6iw/MkyfmcqbOW_OS3CipDZZgYCVbL4/modules/cloudamqp  prefix=[/myproject-terraform/live/cloudamqp/staging/gcp/us-central1/test]
ERRO[0023] 1 error occurred:
        * exit status 1
tbroden84 commented 11 months ago

@ovitente Hi, you will need to add keep_associated_vpc to the cloudamqp_instance resource. https://registry.terraform.io/providers/cloudamqp/cloudamqp/latest/docs/resources/instance#keep_associated_vpc

Otherwise the VPC will be destroyed when the cloudamqp_instance being destroyed. Remnant from older versions, in between when managed standalone VPCs was introduced.

dentarg commented 11 months ago

@tbroden84 I wonder if we could make the provider smarter: not sure it makes sense for cloudamqp_vpc to raise an error when the resource has already been deleted

If we make our backend respond with 410 Gone instead of 404 Not Found, we could make it happen and still raise error when the ID is actually incorrect

ovitente commented 11 months ago

@tbroden84 Thank you, it solved the issue for now.

hanssto commented 11 months ago

I'll add that this issue can also get you into an inconsistent state which blocks planning completely if you target the instance for destruction, but nothing else:

$ terraform destroy --target=module.cloudamqp.cloudamqp_instance.instance
...
Destroy complete! Resources: 28 destroyed.
$ terraform plan
module.cloudamqp.random_password.password_items["service_user"]: Refreshing state... [id=none]
module.cloudamqp.random_password.password_items["second_service_user"]: Refreshing state... [id=none]
data.google_secret_manager_secret_version.cloudamqp_full_access_key: Reading...
data.google_secret_manager_secret_version.cloudamqp_full_access_key: Read complete after 0s [id=projects/501495631697/secrets/cloudamqp-full-access-key/versions/1]
cloudamqp_vpc.vpc: Refreshing state... [id=258088]

Planning failed. Terraform encountered an error while generating this plan.

╷
│ Error: ReadVpcInstance failed, status: 404, message: map[error:No VPC found with ID: 258088]
│ 
│   with cloudamqp_vpc.vpc,
│   on main.tf line 4, in resource "cloudamqp_vpc" "vpc":
│    4: resource "cloudamqp_vpc" "vpc" {
│ 
╵

Here, I'd expect the provider to plan the creation of the instance. Note that the vpc itself did not show up in the list of resources to be destroyed.

I understand and will apply the workaround, but I do believe the default behavior should be to not destroy a VPC if you have managed it in TF through a specific resource. I might want to both create and remove an instance in the same apply, example. If I have declarative code for the VPC, I shouldn't have to think.

Furthermore, the dependency graph should be resolved such that the VPC is also scheduled for destruction if I destroy the instance, if the default is kept as it is. I should be able to see in the TF destruction plan what the provider is gonna do.

tbroden84 commented 10 months ago

The managed VPC resource will no longer throw and error in the latest version v1.29.0. Instead log a warning and continue if it has been deleted by other resource.

The reason it doesn't show up in the dependancy graph, is when destroyed through the instance it's done in our backend. No such dependency is known by the provider. This is unfortunately a rest from when the managed standalone VPC resource was introduced.

dentarg commented 10 months ago

I think we can close this issue now?

We will still have https://github.com/cloudamqp/terraform-provider-cloudamqp/issues/245 open to track that gotcha.