Azure / terraform-azurerm-aks

Terraform Module for deploying an AKS cluster
MIT License
360 stars 468 forks source link

If you add "kubelet_config" to the "azurerm_kubernetes_cluster_node_pool" entry, you will try to replace the node. #563

Closed yuslee80 closed 3 months ago

yuslee80 commented 5 months ago

Is there an existing issue for this?

Description

If you add "kubelet_config" to the "azurerm_kubernetes_cluster_node_pool" entry, you will try to replace the node.

resource "azurerm_kubernetes_cluster_node_pool" "usernodepool" {
  for_each = var.usernodepoo_vm

  name                  = each.value.user_agents_name
  kubernetes_cluster_id = azurerm_kubernetes_cluster.aks.id
  vm_size               = each.value.user_agents_size
  os_disk_size_gb       = each.value.user_agents_os_disk_size
  node_count            = each.value.user_agents_count
  vnet_subnet_id        = data.azurerm_subnet.subnet.id
  zones                 = [1, 2, 3]
  mode                  = "User"
  kubelet_disk_type     = "OS"
  os_sku                = "Ubuntu"
  os_disk_type          = "Managed"
  ultra_ssd_enabled     = "false"
  max_pods              = each.value.max_pods
  orchestrator_version  = each.value.orchestrator_version
  node_labels           = each.value.node_labels
  kubelet_config {
      container_log_max_line = each.value.container_log_max_line
      container_log_max_size_mb = each.value.container_log_max_size_mb
        }
  upgrade_settings {
      max_surge = each.value.max_surge
    }
}

If I add the code as above and do the terraform plan, I would like to replace the node as below.

  # azurerm_kubernetes_cluster_node_pool.usernodepool["vm3"] must be replaced
-/+ resource "azurerm_kubernetes_cluster_node_pool" "usernodepool" {
      - custom_ca_trust_enabled       = false -> null
      - enable_auto_scaling           = false -> null
      - enable_host_encryption        = false -> null
      - enable_node_public_ip         = false -> null
      - fips_enabled                  = false -> null
      ~ id                            = "/subscriptions/00000-000000-000000-0000000/resourceGroups/rg-d-search-aks02-aks/providers/Microsoft.ContainerService/managedClusters/testaks/agentPools/upool03" -> (known after apply)
      - max_count                     = 0 -> null
      - min_count                     = 0 -> null
        name                          = "upool03"
      - node_taints                   = [] -> null
      - tags                          = {} -> null
        # (25 unchanged attributes hidden)

      + kubelet_config { # forces replacement
          + container_log_max_line    = 10 # forces replacement
          + container_log_max_size_mb = 1 # forces replacement
          + cpu_cfs_quota_enabled     = false # forces replacement
        }

      ~ upgrade_settings {
          - drain_timeout_in_minutes      = 0 -> null
          - node_soak_duration_in_minutes = 0 -> null
            # (1 unchanged attribute hidden)
        }
    }

However, in "default_node_pool", it works as "change".

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

azurerm_v3.107.0

Potential Terraform Configuration

It should act as "change" instead of "force replace".

References

No response

zioproto commented 5 months ago

@yuslee80 thanks for opening this issue

I see you are using the azurerm_kubernetes_cluster_node_pool resource directly. Can you confirm you are not using the module from this repository to create the node pool with the variable var.node_pools ?

The correct place to open this bug is at the Hashicorp Terraform provider azurerm repository: https://github.com/hashicorp/terraform-provider-azurerm/issues

zioproto commented 4 months ago

This issue should be solved my the patch in PR #564

Please reopen the issue if you need further help.

yuslee80 commented 4 months ago

Is this "PR" not integrated with AzureRM yet? My code still says "Replace" as below.

  # azurerm_kubernetes_cluster_node_pool.usernodepool["vm1"] must be replaced
-/+ resource "azurerm_kubernetes_cluster_node_pool" "usernodepool" {
      - custom_ca_trust_enabled       = false -> null
      - enable_auto_scaling           = false -> null
      - enable_host_encryption        = false -> null
      - enable_node_public_ip         = false -> null
      - fips_enabled                  = false -> null
      ~ id                            = "/subscriptions/00000-000000-0000000-0000000/resourceGroups/rg-p-yuslee-aks/providers/Microsoft.ContainerService/managedClusters/aks-test/agentPools/upool01" -> (known after apply)
      - max_count                     = 0 -> null
      - min_count                     = 0 -> null
        name                          = "upool01"
      - tags                          = {} -> null
        # (26 unchanged attributes hidden)

      + kubelet_config { # forces replacement
          + container_log_max_line    = 2 # forces replacement
          + container_log_max_size_mb = 10 # forces replacement
          + cpu_cfs_quota_enabled     = false # forces replacement
        }

      ~ upgrade_settings {
          - drain_timeout_in_minutes      = 0 -> null
          - node_soak_duration_in_minutes = 0 -> null
            # (1 unchanged attribute hidden)
        }
    }

AzureRM : v3.113.0

zioproto commented 4 months ago

Hello @yuslee80

Sorry I made a mistake. PR #564 is about the upgrade_settings, not the kubelet_config.

It seems that adding kubelet_config forces the replacement of the nodepool, according to the documentation this seems to be the expected behaviour.

https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/kubernetes_cluster#kubelet_config

You should use the temporary_name_for_rotation as explained in the docs.

Could you confirm you are having this problem using the provider directly and not this module ?

thanks

lonegunmanb commented 3 months ago

Hi @yuslee80 , I assume that temporary_name_for_rotation could solve this issue and I'm closing it now, please feel free to reopen it if you have any further questions.