Azure / terraform-azurerm-aks

Terraform Module for deploying an AKS cluster
MIT License
346 stars 459 forks source link

The os_disk_type default value is not consistent with AKS default behaviour #261

Open zioproto opened 1 year ago

zioproto commented 1 year ago

Is there an existing issue for this?

Greenfield/Brownfield provisioning

greenfield

Terraform Version

1.2.8

Module Version

6.0.0

AzureRM Provider Version

3.0.0

Affected Resource(s)/Data Source(s)

azurerm_kubernetes_cluster

Terraform Configuration Files

variable "os_disk_type" {
  type        = string
  description = "The type of disk which should be used for the Operating System. Possible values are `Ephemeral` and `Managed`. Defaults to `Managed`. Changing this forces a new resource to be created."
  default     = "Managed"
  nullable    = false
}

tfvars variables values

N/A

Debug Output/Panic Output

N/A

Expected Behaviour

The default value for os_disk_type should be Ephemeral.

https://learn.microsoft.com/en-us/azure/aks/cluster-configuration#ephemeral-os

Screenshot 2022-09-28 at 09 14 03

Actual Behaviour

The current default value is Managed https://github.com/Azure/terraform-azurerm-aks/blob/master/variables.tf#L363

Steps to Reproduce

No response

Important Factoids

n/a

References

It is all about this variable https://github.com/Azure/terraform-azurerm-aks/blob/master/variables.tf#L360

zioproto commented 1 year ago

@lonegunmanb this is a trivial patch, but it is also a change of default behaviour that triggers the cluster to be destroyed and recreated. It is very dangerous. I believe we should target this for 7.0.0 with a very visible release note. What is your opinion on this ?

Thanks to @paolosalvatori that noticed this discrepancy in default behaviour between the AKS product and the Terraform Module.

lonegunmanb commented 1 year ago

Thanks @zioproto for opening this issue. According to azurerm_kubernetes_cluster's document, the os_disk_type's default value is Managed in Terraform AzureRM Provider, so I think it's ok to keep it in this module. As the downstream of Terraform Provider I think it's better to keep consist with Terraform Provider, because the Provider has low tolerance of breaking change, whilst API and portal are tend to change more frequently.