search

ionos-cloud / terraform-provider-profitbricks

Terraform ProfitBricks provider
https://www.terraform.io/docs/providers/profitbricks/
Mozilla Public License 2.0
3 stars 16 forks source link

public_ips not removed from nodepool #86

Closed arteonprifti closed 3 years ago

arteonprifti commented 3 years ago

Terraform Version

Terraform v0.13.3

Affected Resource(s)

Please list the resources as a list, for example:

Terraform Configuration Files

  name        =  "example"
  k8s_version = "1.18.5"
  auto_scaling {
    min_node_count = 1
    max_node_count = 2
  }
  datacenter_id     = "1ab26503-ca71-4b5e-898b-ff7dc81492c4"
  k8s_cluster_id    = "ca16889c-321f-466f-9342-f4ca692867b0"
  cpu_family        = "INTEL_XEON"
  availability_zone = "AUTO"
  storage_type      = "SSD"
  node_count        = 1
  cores_count       = 2
  ram_size          = 2048
  storage_size      = 40      
  public_ips      = ["217.160.200.189", "217.160.200.190", "217.160.200.191"]
}

Expected Behavior

public_ips should get removed

Actual Behavior

terraform returns changed, but the public_ips are not removed

Steps to Reproduce

  1. Start with 2 nodes (no autoscaling) and 3 public_ips as base setup (works of course)

  2. Change 2->1 nodes and 3->2 public_ips works (if you don’t change the ip_block size, you will see one free CRIP in DCD)

  3. Change to empty list for public_ips fails during plan phase: “Error: public_ips: attribute supports 2 item as a minimum, config has 0 declared” This is OK: https://github.com/ionos-cloud/terraform-provider-profitbricks/blob/master/profitbricks/resource_profitbricks_k8s_node_pool.go (line 126 ff) specifies that if public_ips is provided, it must have at least 2 entries. Thus you have to provide min 2 IPS or completely omit the public_ips property. Which leads to …

  4. Remove public_ips completely. This shows the described wrong behavior, i.e. silently doing nothing. This is caused by a missing else block in the code (line 319ff): The public_ips are only updated if they are provided (‘if newPublicIps != nil’). If they are omitted, newPublicIps is nil and this is not covered by the code.

LiviusP commented 3 years ago

Work in progress. Issue was identified in the Terraform code: wrong validation of number of elements in the publicIps array.

LiviusP commented 3 years ago

Solved in version 1.5.12.

https://registry.terraform.io/providers/ionos-cloud/profitbricks/latest

mflorin commented 3 years ago

@arteonprifti - please note that removing public_ips can now be done by simply providing an empty public_ips list i.e.

    public_ips = []

Omitting the field altogether, will NOT remove it, it'll leave it unchanged, this behavior being consistent with every other update operation.