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

Unable to create an instance with `cloudamqp_rabbitmq_configuration` with heartbeat value set to 0 #214

Open thar opened 1 year ago

thar commented 1 year ago

Hi

When creating a cloudamqp_rabbitmq_configuration resource and applying it I'm always getting errors.

I'm creating it the following way:

resource "cloudamqp_rabbitmq_configuration" "rabbitmq_config" {
  instance_id = cloudamqp_instance.rabbitmq.id
  heartbeat   = var.instance.heartbeat
}

If the heartbeat value is greater than 0 I get the following error: Error: Put "https://customer.cloudamqp.com/api/instances/220352/config": EOF

The debug log shows:

2023-07-13T10:32:21.163Z [DEBUG] provider.terraform-provider-cloudamqp_v1.27.0: 2023/07/13 10:32:21 [DEBUG] go-api::rabbitmq-configuration::updateWithRetry Timeout talking to backend, attempt: 1, until timeout: 3540
2023-07-13T10:33:44.249Z [DEBUG] provider.terraform-provider-cloudamqp_v1.27.0: 2023/07/13 10:33:44 [DEBUG] go-api::rabbitmq-configuration::updateWithRetry Timeout talking to backend, attempt: 2, until timeout: 3480
2023-07-13T10:34:44.251Z [ERROR] vertex "module.rabbitmq.cloudamqp_rabbitmq_configuration.rabbitmq_config[0]" error: Put "https://customer.cloudamqp.com/api/instances/220352/config": EOF
2023-07-13T10:34:44.252Z [DEBUG] cloud/state: state read serial is: 793; serial is: 793
2023-07-13T10:34:44.252Z [DEBUG] cloud/state: state read lineage is: 8ffccdda-ea1e-fbbf-fae2-2cda7640a0c9; lineage is: 8ffccdda-ea1e-fbbf-fae2-2cda7640a0c9
2023-07-13T10:34:45.791Z [DEBUG] provider.stdio: received EOF, stopping recv loop: err="rpc error: code = Unavailable desc = error reading from server: EOF"
2023-07-13T10:34:45.791Z [DEBUG] provider: plugin process exited: path=.terraform/providers/registry.terraform.io/cloudamqp/cloudamqp/1.27.0/linux_amd64/terraform-provider-cloudamqp_v1.27.0 pid=215
2023-07-13T10:34:45.791Z [DEBUG] provider: plugin exited

I've also detected that when setting heartbeat value to 0 I receive the following error: Error: Update RabbitMQ configuration failed, status: 400, message: map[error:No settings to validate]

Do you know this issue?

Thanks in advance

dentarg commented 1 year ago

I could not reproduce these issues, tried with >0 and 0

Are you still having issues @thar? The problems could be related to the load of your cluster. Please reach out to https://www.cloudamqp.com/support.html (feel free to reference this issue) and we will help you.

dentarg commented 1 year ago

I could not reproduce these issues, tried with >0 and 0

That was against RabbitMQ 3.12.1, where I in my first run used heartbeat = 60 and heartbeat = 0 after that.

Doing a run with heartbeat = 0 against a freshly created RabbitMQ 3.11.10 do cause the 400 Bad Request

cloudamqp_rabbitmq_configuration.rabbitmq_config: Creating...
╷
│ Error: Update RabbitMQ configuration failed, status: 400, message: map[error:No settings to validate]
│
│   with cloudamqp_rabbitmq_configuration.rabbitmq_config,
│   on legacy_plan_test.tf line 18, in resource "cloudamqp_rabbitmq_configuration" "rabbitmq_config":
│   18: resource "cloudamqp_rabbitmq_configuration" "rabbitmq_config" {
│
╵
dentarg commented 1 year ago

heartbeat = 0 works against RabbitMQ 3.11.10 after first doing heartbeat = 60

resource "cloudamqp_rabbitmq_configuration" "rabbitmq_config" {
  instance_id = cloudamqp_instance.cluster.id
  heartbeat   = 0
}

Interesting thing is that I can see rabbit.connection_max being set to infinity and rabbit.queue_index_embed_msgs_below being set to 4096 in the same request.

dentarg commented 1 year ago

I can see that the PUT request to /config from the provider has empty body.

dentarg commented 1 year ago

I think you can workaround the heartbeat = 0 issue if you import the configuration before trying to change it (the default heartbeat value is 120).

...but we should address this as a run like this fails

resource "cloudamqp_instance" "cluster" {
  name   = "terraform"
  plan   = "bunny-1"
  region = "amazon-web-services::eu-north-1"
}

resource "cloudamqp_rabbitmq_configuration" "rabbitmq_config" {
  instance_id = cloudamqp_instance.cluster.id
  heartbeat   = 0
}
tete17 commented 1 year ago

I have a strong suspicious that the provider is not allowing 0 & 0.0 in creating a resource even though they are valid states for some of the configuration like it states in doc for heartbeat & channel_max

https://github.com/cloudamqp/terraform-provider-cloudamqp/blob/00f4368b715ee3080bd36681dfc05e9ecb68aecd/cloudamqp/resource_cloudamqp_rabbitmq_configuration.go#L157-L164

dentarg commented 1 year ago

Thanks, that indeed looks like the issue. It comes from #166. Looks like some more elaborate handling is needed. Let's wait until @tbroden84 is back from vacation.

tbroden84 commented 1 year ago

We are using an old Terraform Plugin SDK version (not latest Framework). The older versions don't support unset values to be represented as null. Instead they will get the default data types values (e.g. string = "", int = 0). During the initial resource create run, it's not possible to determine the difference between missing schema argument or schema argument set to a default value.

So when using this the first time, there is no difference between heartbeat or any other schema.TypeInt argument. They will all be read as 0. Even running d.HasChange(key) will not detect the change for heartbeat the first time.

resource "cloudamqp_rabbitmq_configuration" "rabbitmq_config" {
  instance_id = cloudamqp_instance.cluster.id
  heartbeat   = 0
}

During the second run, there will be a diff from read values against your configuration. It's then possible to detect the heartbeat value being 0 and update this. Not an optimal workflow.

Tried to clean up the code a bit in #215 but will unfortunately not solve this. We would need to upgrade to Terraform Plugin Framework, which will be a huge task. Not currently planed but will also come with more benefits in other use cases too.

dentarg commented 1 year ago

Ah, so we can't see the difference between user doing heartbeat = 0 (and wanting us to change it to 0) and user not changing heartbeat (wants us to do nothing with it)?

tbroden84 commented 1 year ago

Correct, so during the initial create run they will both be 0 without any changes detected. During the second run or a run after an import there will be a difference. State file says heartbeat = 120, while configuration file heartbeat = 0. This 0 value we can detect.

dentarg commented 1 year ago

Should we address this with improved documentation for now @tbroden84?

tbroden84 commented 1 year ago

Yes good point, added it as a task.