[ContainerType] Amount should be optional

[johnsudaar]

Currently we mark the amount parameter of the container type as optional. However if it's not specified the request will send a value of 0 instead of not using it at all.

This is due to this line: https://github.com/Scalingo/terraform-provider-scalingo/blob/master/scalingo/resource_scalingo_container_type.go#L62 (and this one: https://github.com/Scalingo/terraform-provider-scalingo/blob/master/scalingo/resource_scalingo_container_type.go#L113).

This behavior is dangerous. Because if a client wants to just scale vertically the containers will be scaled to 0. This is especially dangerous with autoscaler where we might be tempted to write:

resource "scalingo_autoscaler" "web_autoscaler" {
  app            = "[APP_ID]"
  min_containers = 5
  max_containers = 10
  metric         = "rpm_per_container"
  target         = 500
  container_type = "web"
}

resource "scalingo_container_type" "web" {
  app    = "[APP_ID]"
  name   = "web"
  size   = "2XL"
}

We might expect that this creates an autoscaler on the "web" container type that uses 5 to 10 2XL when in fact this will scale the web containers to 0.

[Frzk]

@johnsudaar : If I understand well, I guess the issue's title should read "should not be optional", doesn't it?

I also have a comment/question. When an autoscaler is configured and enabled on an app:

• If I scale the app via the CLI, I get a warning telling me that the operation will disable the autoscaler (and it let me chose to go on or not).
• If I scale the app via an API call, it scales and disables the autoscaler.

For our Terraform provider, I think we should stick with the API behavior, so using:

resource "scalingo_container_type" "web" {
  app    = "[APP_ID]"
  name   = "web"
  size   = "2XL"
  amount = 1
}

would actually disable the autoscaler and scale the app to web:1:2XL.

In your example it would be a bit weird to define an autoscaler and then disable it right afterwards, wouldn't it? Is that OK?

[johnsudaar]

Sorry I missed your comment here.

No the issue title is right. But i might've not explained the issue correctly. Currently the field is indeed marked as Optional and indeed i think it should be. Because the autoscaler only let users do horizontal scaling where the scalingo_container_type resource permit users to do both horizontal and vertical scaling.

So if we apply the changes you propose, we wont be able to change the vertical scalingo of containers managed by an autoscaler by using terraform.

IMHO, we should ensure that if the amount parameter is not passed to the resource, it should also not be sent to the API. This is not the current behavior: if we do not pass the amount variable we will scale it to 0 containers.