AzureRM version 3.69.0 - AKS temporary_name_for_rotation

hgebrael commented 1 year ago

Is there an existing issue for this?

[X] I have searched the existing issues

Community Note

Please vote on this issue by adding a :thumbsup: reaction to the original issue to help the community and maintainers prioritize this request
Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
If you are interested in working on this issue or have submitted a pull request, please leave a comment and review the contribution guide to help.

Terraform Version

1.5.5

AzureRM Provider Version

3.69.0

Affected Resource(s)/Data Source(s)

azurerm_kubernetes_cluster

Terraform Configuration Files

default_node_pool {
    name                        = "default"
    type                        = "VirtualMachineScaleSets"
    vnet_subnet_id              = azurerm_subnet.private.id
    vm_size                     = var.aks.vm_size
    node_count                  = var.aks.node_count
    zones                       = var.aks.availability_zones
    orchestrator_version        = var.aks.version
    os_disk_type                = var.aks.os_disk_type
    max_pods                    = 110
    enable_host_encryption      = var.enable_host_encryption
    temporary_name_for_rotation = var.enable_host_encryption ? "tempnodepool" : ""
}

Debug Output/Panic Output

I'm trying to change the enable_host_encryption on the node pool from False/default to True. 
As mentioned in your documentation, changing this value requires temporary_name_for_rotation to be specified, however performing the changes through terraform results in destructive actions since the recycling of the node doesn't perform a cordon and drained as written in your documentation. 

To perform a safe recycling, I added the new temporary node pool manually with encryption_at_host enabed and performed cordon and drain manually. 

After recycling the node, I tried to run terraform apply and the  enable_host_encryption did not change as the value has been changed manually to true. but terraform still prompting to add the temporary_name_for_rotation and when adding this feature, it performed again an unsafe nodepool recycling. 

│ Error: `temporary_name_for_rotation` must be specified when updating any of the following properties ["default_node_pool.0.name" "default_node_pool.0.enable_host_encryption" "default_no
de_pool.0.enable_node_public_ip" "default_node_pool.0.kubelet_config" "default_node_pool.0.linux_os_config" "default_node_pool.0.max_pods" "default_node_pool.0.node_taints" "default_node_
pool.0.only_critical_addons_enabled" "default_node_pool.0.os_disk_size_gb" "default_node_pool.0.os_disk_type" "default_node_pool.0.os_sku" "default_node_pool.0.pod_subnet_id" "default_nod
e_pool.0.snapshot_id" "default_node_pool.0.ultra_ssd_enabled" "default_node_pool.0.vnet_subnet_id" "default_node_pool.0.vm_size" "default_node_pool.0.zones"]

Expected Behaviour

Terraform should not request for temporary_name_for_rotation when adding the enable_host_encryption to the configuration because its value did not change.

The logic should be: if enable_host_encryption value changed, specify temporary_name_for_rotation otherwise do not specify this attribute.

Actual Behaviour

terraform always asks for adding the temporary_name_for_rotation and performing a recycling on nodepool even if the attribute enable_host_encryption did not change.

Steps to Reproduce

Terraform apply

Important Factoids

No response

References

No response

ms-henglu commented 1 year ago

Hi @hgebrael ,

Thank you for taking time to report this issue!

I tried to reproduce it but failed, here's what I did:

Create cluster with default node pool enable_host_encryption = false in Terraform.
Create a temp system node pool with azure cli
Delete the default node pool with azure cli
Create the default node pool which has enable_host_encryption = true with azure cli
Update the terraform config to have enable_host_encryption = true, terraform shows no changes.

fabio-s-franco commented 1 year ago

@ms-henglu I also hit this problem when changing availability zones. Before: nodes = 2 syspool = zones 1, 2

after nodes = 3 syspool = zones 1,2,3 (west europe)

nrv-96 commented 1 year ago

Hi @ms-henglu, the same issue occurred with my Terraform code when I changed from 'default_node_pool' with 'os_sku' set to 'Ubuntu' to 'AzureLinux'.

My code is as below:

resource "azurerm_kubernetes_cluster" "aks_cluster" { name = "${local.env}-${local.aks_name}" resource_group_name = azurerm_resource_group.rg.name location = azurerm_resource_group.rg.location kubernetes_version = local.aks_version sku_tier = "Free" oidc_issuer_enabled = true workload_identity_enabled = true automatic_channel_upgrade = "patch" private_cluster_enabled = false dns_prefix = "devaks1" node_resource_group = "${local.resource_group_name}-${local.env}-${local.aks_name}" network_profile { network_plugin = "azure" dns_service_ip = "10.0.64.10" service_cidr = "10.0.64.0/19" } default_node_pool { name = "agentpool" vm_size = "standard_b2s" vnet_subnet_id = azurerm_subnet.private.id orchestrator_version = local.aks_version type = "VirtualMachineScaleSets" enable_auto_scaling = true node_count = 1 min_count = 1 max_count = 10 os_sku = "AzureLinux"

node_labels = {
  role = "agentpool"
}

} identity { type = "UserAssigned" identity_ids = [azurerm_user_assigned_identity.base.id] }

tags = { env = local.env }

lifecycle { ignore_changes = [default_node_pool[0].node_count] }

depends_on = [ azurerm_role_assignment.base ] }

Error: Error: temporary_name_for_rotation must be specified when updating any of the following properties ["default_node_pool.0.name" "default_node_pool.0.enable_host_encryption" "default_node_pool.0.enable_node_public_ip" "default_node_pool.0.kubelet_config" "default_node_pool.0.linux_os_config" "default_node_pool.0.max_pods" "default_node_pool.0.node_taints" "default_node_pool.0.only_critical_addons_enabled" "default_node_pool.0.os_disk_size_gb" "default_node_pool.0.os_disk_type" "default_node_pool.0.os_sku" "default_node_pool.0.pod_subnet_id" "default_node_pool.0.snapshot_id" "default_node_pool.0.ultra_ssd_enabled" "default_node_pool.0.vnet_subnet_id" "default_node_pool.0.vm_size" "default_node_pool.0.zones"]

nrv-96 commented 1 year ago

@ms-henglu Please set the temporary_name_for_rotation property should be required inside default_node_pool instead of optional.

@fabio-s-franco & @hgebrael issue is resolved when I have added temporary_name_for_rotation into default_node_pool.

Refer link: https://stackoverflow.com/questions/77094453/temporary-name-for-rotation-must-be-specified-when-changing-any-of-the-following

hashicorp / terraform-provider-azurerm