Node pool names can be defined by user and don't get appended a random suffix

[oteichmann]

Is there an existing issue for this?

• I have searched the existing issues

Description

Currently the additional node pools get assigned a random MD5 hash suffix. This behaviour isn't document in irrupting to the user in my opinion.

See https://github.com/Azure/terraform-azurerm-aks/blob/main/main.tf#L581

I'd prefer the names are check for uniqueness via TF and in case of conflicts get rejected.

New or Affected Resource(s)/Data Source(s)

variable "node_pools"

Potential Terraform Configuration

// Validation inside "node_pools" variable definition
validation {
  condition     = length(values(var.node_pools)[*].name) == length(distinct(values(var.node_pools)[*].name))
  error_message = "Err: each of the node pool names must be unique."
}

References

No response

[zioproto]

@oteichmann the random suffix is needed during nodepool upgrades, to have a "create before destroy" lifecycle of the nodepools.

If I understand correctly your assumption was that the random suffix was needed to avoid multiple nodepools with the same name. As explained above this is not the goal, so the validation you propose does not meet the goals of the module.

[lonegunmanb]

@oteichmann thanks for opening this issue. As @zioproto explained, if we don't add this suffix, when users want to recreate the pool, they would have to delete the existing pool first, then create a new one since the pool's name could be the same, and that could lead to downtime, so we have no choice but adding this random suffix, so we can create a new pool before we destroy the old one (since now they have different names), and this procedure gives the cluster's operator a chance to evict all workloads from the old pool to the new one to avoid downtime.

Do you have any further questions or suggestions?

[oteichmann]

Thanks for the explanation @zioproto and @lonegunmanb . No further questions. This vital, operational arguments do of course outweigh my striving for order and beauty ;)

I might come back on this once the "azurerm_kubernetes_cluster_node_pool" resource does also support some rotation logic as it is already the case for the default pool.