Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Let the users decide whether adding a random suffix in cluster and pool's name or not. #496

Merged
merged 6 commits into from
Feb 20, 2024
Merged

Let the users decide whether adding a random suffix in cluster and pool's name or not. #496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c8781fd
add new cluster_name_random_suffix variable so the users could decide…
lonegunmanb Jan 11, 2024
0f4ac24
fix incorrect type
lonegunmanb Jan 11, 2024
f5d6af1
add retry when ingress is not ready
lonegunmanb Jan 12, 2024
c9f8820
revert change to example
lonegunmanb Feb 19, 2024
984612f
update extra node pool's name suffix, use uuid() as random seed
lonegunmanb Feb 19, 2024
4a20a4e
fix format issue
lonegunmanb Feb 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions README.md
View file
Open in desktop

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions examples/multiple_node_pools/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ resource "azurerm_subnet" "test" {
locals {
nodes = {
for i in range(3) : "worker${i}" => {
name = substr("worker${i}${random_id.prefix.hex}", 0, 8)
vm_size = "Standard_D2s_v3"
node_count = 1
vnet_subnet_id = azurerm_subnet.test.id
name = substr("worker${i}${random_id.prefix.hex}", 0, 8)
vm_size = "Standard_D2s_v3"
node_count = 1
vnet_subnet_id = azurerm_subnet.test.id
create_before_destroy = i % 2 == 0
}
}
}
Expand Down
170 changes: 164 additions & 6 deletions extra_node_pool.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
resource "azurerm_kubernetes_cluster_node_pool" "node_pool" {
for_each = var.node_pools
moved {
from = azurerm_kubernetes_cluster_node_pool.node_pool
to = azurerm_kubernetes_cluster_node_pool.node_pool_create_before_destroy
}

resource "azurerm_kubernetes_cluster_node_pool" "node_pool_create_before_destroy" {
for_each = local.node_pools_create_before_destroy

kubernetes_cluster_id = azurerm_kubernetes_cluster.main.id
name = "${each.value.name}${substr(md5(jsonencode(each.value)), 0, 4)}"
name = "${each.value.name}${substr(md5(uuid()), 0, 4)}"
vm_size = each.value.vm_size
capacity_reservation_group_id = each.value.capacity_reservation_group_id
custom_ca_trust_enabled = each.value.custom_ca_trust_enabled
Expand Down Expand Up @@ -34,14 +39,13 @@ resource "azurerm_kubernetes_cluster_node_pool" "node_pool" {
snapshot_id = each.value.snapshot_id
spot_max_price = each.value.spot_max_price
tags = merge(each.value.tags, (/*<box>*/ (var.tracing_tags_enabled ? { for k, v in /*</box>*/ {
avm_yor_name = "node_pool"
avm_git_commit = "bc0c9fab9ee53296a64c7a682d2ed7e0726c6547"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change ? It is just changing the order in the tag list, correct ?

avm_git_file = "main.tf"
avm_git_last_modified_at = "2023-05-04 05:02:32"
avm_git_org = "Azure"
avm_git_repo = "terraform-azurerm-aks"
avm_yor_trace = "6dcb4922-45aa-469d-ab2e-1945e2735ac5"
} /*<box>*/ : replace(k, "avm_", var.tracing_tags_prefix) => v } : {}) /*</box>*/), (/*<box>*/ (var.tracing_tags_enabled ? { for k, v in /*</box>*/ {
avm_yor_name = "node_pool"
avm_yor_trace = "2aa46696-a713-4c12-95c0-550c5ff7f8ec"
} /*<box>*/ : replace(k, "avm_", var.tracing_tags_prefix) => v } : {}) /*</box>*/))
ultra_ssd_enabled = each.value.ultra_ssd_enabled
vnet_subnet_id = each.value.vnet_subnet_id
Expand Down Expand Up @@ -161,6 +165,160 @@ resource "azurerm_kubernetes_cluster_node_pool" "node_pool" {
}
}

resource "azurerm_kubernetes_cluster_node_pool" "node_pool_create_after_destroy" {
for_each = local.node_pools_create_after_destroy

kubernetes_cluster_id = azurerm_kubernetes_cluster.main.id
name = each.value.name
vm_size = each.value.vm_size
capacity_reservation_group_id = each.value.capacity_reservation_group_id
custom_ca_trust_enabled = each.value.custom_ca_trust_enabled
enable_auto_scaling = each.value.enable_auto_scaling
enable_host_encryption = each.value.enable_host_encryption
enable_node_public_ip = each.value.enable_node_public_ip
eviction_policy = each.value.eviction_policy
fips_enabled = each.value.fips_enabled
host_group_id = each.value.host_group_id
kubelet_disk_type = each.value.kubelet_disk_type
max_count = each.value.max_count
max_pods = each.value.max_pods
message_of_the_day = each.value.message_of_the_day
min_count = each.value.min_count
mode = each.value.mode
node_count = each.value.node_count
node_labels = each.value.node_labels
node_public_ip_prefix_id = each.value.node_public_ip_prefix_id
node_taints = each.value.node_taints
orchestrator_version = each.value.orchestrator_version
os_disk_size_gb = each.value.os_disk_size_gb
os_disk_type = each.value.os_disk_type
os_sku = each.value.os_sku
os_type = each.value.os_type
pod_subnet_id = each.value.pod_subnet_id
priority = each.value.priority
proximity_placement_group_id = each.value.proximity_placement_group_id
scale_down_mode = each.value.scale_down_mode
snapshot_id = each.value.snapshot_id
spot_max_price = each.value.spot_max_price
tags = merge(each.value.tags, (/*<box>*/ (var.tracing_tags_enabled ? { for k, v in /*</box>*/ {
avm_yor_name = "node_pool"
avm_git_commit = "bc0c9fab9ee53296a64c7a682d2ed7e0726c6547"
avm_git_file = "main.tf"
avm_git_last_modified_at = "2023-05-04 05:02:32"
avm_git_org = "Azure"
avm_git_repo = "terraform-azurerm-aks"
avm_yor_trace = "2aa46696-a713-4c12-95c0-550c5ff7f8ec"
} /*<box>*/ : replace(k, "avm_", var.tracing_tags_prefix) => v } : {}) /*</box>*/))
ultra_ssd_enabled = each.value.ultra_ssd_enabled
vnet_subnet_id = each.value.vnet_subnet_id
workload_runtime = each.value.workload_runtime
zones = each.value.zones

dynamic "kubelet_config" {
for_each = each.value.kubelet_config == null ? [] : ["kubelet_config"]

content {
allowed_unsafe_sysctls = each.value.kubelet_config.allowed_unsafe_sysctls
container_log_max_line = each.value.kubelet_config.container_log_max_files
container_log_max_size_mb = each.value.kubelet_config.container_log_max_size_mb
cpu_cfs_quota_enabled = each.value.kubelet_config.cpu_cfs_quota_enabled
cpu_cfs_quota_period = each.value.kubelet_config.cpu_cfs_quota_period
cpu_manager_policy = each.value.kubelet_config.cpu_manager_policy
image_gc_high_threshold = each.value.kubelet_config.image_gc_high_threshold
image_gc_low_threshold = each.value.kubelet_config.image_gc_low_threshold
pod_max_pid = each.value.kubelet_config.pod_max_pid
topology_manager_policy = each.value.kubelet_config.topology_manager_policy
}
}
dynamic "linux_os_config" {
for_each = each.value.linux_os_config == null ? [] : ["linux_os_config"]

content {
swap_file_size_mb = each.value.linux_os_config.swap_file_size_mb
transparent_huge_page_defrag = each.value.linux_os_config.transparent_huge_page_defrag
transparent_huge_page_enabled = each.value.linux_os_config.transparent_huge_page_enabled

dynamic "sysctl_config" {
for_each = each.value.linux_os_config.sysctl_config == null ? [] : ["sysctl_config"]

content {
fs_aio_max_nr = each.value.linux_os_config.sysctl_config.fs_aio_max_nr
fs_file_max = each.value.linux_os_config.sysctl_config.fs_file_max
fs_inotify_max_user_watches = each.value.linux_os_config.sysctl_config.fs_inotify_max_user_watches
fs_nr_open = each.value.linux_os_config.sysctl_config.fs_nr_open
kernel_threads_max = each.value.linux_os_config.sysctl_config.kernel_threads_max
net_core_netdev_max_backlog = each.value.linux_os_config.sysctl_config.net_core_netdev_max_backlog
net_core_optmem_max = each.value.linux_os_config.sysctl_config.net_core_optmem_max
net_core_rmem_default = each.value.linux_os_config.sysctl_config.net_core_rmem_default
net_core_rmem_max = each.value.linux_os_config.sysctl_config.net_core_rmem_max
net_core_somaxconn = each.value.linux_os_config.sysctl_config.net_core_somaxconn
net_core_wmem_default = each.value.linux_os_config.sysctl_config.net_core_wmem_default
net_core_wmem_max = each.value.linux_os_config.sysctl_config.net_core_wmem_max
net_ipv4_ip_local_port_range_max = each.value.linux_os_config.sysctl_config.net_ipv4_ip_local_port_range_max
net_ipv4_ip_local_port_range_min = each.value.linux_os_config.sysctl_config.net_ipv4_ip_local_port_range_min
net_ipv4_neigh_default_gc_thresh1 = each.value.linux_os_config.sysctl_config.net_ipv4_neigh_default_gc_thresh1
net_ipv4_neigh_default_gc_thresh2 = each.value.linux_os_config.sysctl_config.net_ipv4_neigh_default_gc_thresh2
net_ipv4_neigh_default_gc_thresh3 = each.value.linux_os_config.sysctl_config.net_ipv4_neigh_default_gc_thresh3
net_ipv4_tcp_fin_timeout = each.value.linux_os_config.sysctl_config.net_ipv4_tcp_fin_timeout
net_ipv4_tcp_keepalive_intvl = each.value.linux_os_config.sysctl_config.net_ipv4_tcp_keepalive_intvl
net_ipv4_tcp_keepalive_probes = each.value.linux_os_config.sysctl_config.net_ipv4_tcp_keepalive_probes
net_ipv4_tcp_keepalive_time = each.value.linux_os_config.sysctl_config.net_ipv4_tcp_keepalive_time
net_ipv4_tcp_max_syn_backlog = each.value.linux_os_config.sysctl_config.net_ipv4_tcp_max_syn_backlog
net_ipv4_tcp_max_tw_buckets = each.value.linux_os_config.sysctl_config.net_ipv4_tcp_max_tw_buckets
net_ipv4_tcp_tw_reuse = each.value.linux_os_config.sysctl_config.net_ipv4_tcp_tw_reuse
net_netfilter_nf_conntrack_buckets = each.value.linux_os_config.sysctl_config.net_netfilter_nf_conntrack_buckets
net_netfilter_nf_conntrack_max = each.value.linux_os_config.sysctl_config.net_netfilter_nf_conntrack_max
vm_max_map_count = each.value.linux_os_config.sysctl_config.vm_max_map_count
vm_swappiness = each.value.linux_os_config.sysctl_config.vm_swappiness
vm_vfs_cache_pressure = each.value.linux_os_config.sysctl_config.vm_vfs_cache_pressure
}
}
}
}
dynamic "node_network_profile" {
for_each = each.value.node_network_profile == null ? [] : ["node_network_profile"]

content {
node_public_ip_tags = each.value.node_network_profile.node_public_ip_tags
}
}
dynamic "upgrade_settings" {
for_each = each.value.upgrade_settings == null ? [] : ["upgrade_settings"]

content {
max_surge = each.value.upgrade_settings.max_surge
}
}
dynamic "windows_profile" {
for_each = each.value.windows_profile == null ? [] : ["windows_profile"]

content {
outbound_nat_enabled = each.value.windows_profile.outbound_nat_enabled
}
}

depends_on = [azapi_update_resource.aks_cluster_post_create]

lifecycle {
precondition {
condition = can(regex("[a-z0-9]{1,8}", each.value.name))
error_message = "A Node Pools name must consist of alphanumeric characters and have a maximum lenght of 8 characters (4 random chars added)"
}
precondition {
condition = var.network_plugin_mode != "overlay" || each.value.os_type != "Windows"
error_message = "Windows Server 2019 node pools are not supported for Overlay and Windows support is still in preview"
}
precondition {
condition = var.network_plugin_mode != "overlay" || !can(regex("^Standard_DC[0-9]+s?_v2$", each.value.vm_size))
error_message = "With with Azure CNI Overlay you can't use DCsv2-series virtual machines in node pools. "
}
precondition {
condition = var.agents_type == "VirtualMachineScaleSets"
error_message = "Multiple Node Pools are only supported when the Kubernetes Cluster is using Virtual Machine Scale Sets."
}
}
}

resource "null_resource" "pool_name_keeper" {
for_each = var.node_pools

Expand Down
3 changes: 3 additions & 0 deletions locals.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ locals {
(contains(["patch"], var.automatic_channel_upgrade) && can(regex("^[0-9]{1,}\\.[0-9]{1,}$", var.kubernetes_version)) && (can(regex("^[0-9]{1,}\\.[0-9]{1,}$", var.orchestrator_version)) || var.orchestrator_version == null)) ||
(contains(["rapid", "stable", "node-image"], var.automatic_channel_upgrade) && var.kubernetes_version == null && var.orchestrator_version == null)
)
cluster_name = coalesce(var.cluster_name, trim("${var.prefix}-aks", "-"))
# Abstract the decision whether to create an Analytics Workspace or not.
create_analytics_solution = var.log_analytics_workspace_enabled && var.log_analytics_solution == null
create_analytics_workspace = var.log_analytics_workspace_enabled && var.log_analytics_workspace == null
Expand Down Expand Up @@ -45,6 +46,8 @@ locals {
resource_group_name = split("/", var.log_analytics_workspace.id)[4]
}
) : null # Finally, the Log Analytics Workspace should be disabled.
node_pools_create_after_destroy = { for k, p in var.node_pools : k => p if p.create_before_destroy != true }
node_pools_create_before_destroy = { for k, p in var.node_pools : k => p if p.create_before_destroy == true }
potential_subnet_ids = flatten(concat([
for pool in var.node_pools : [
pool.vnet_subnet_id,
Expand Down
13 changes: 12 additions & 1 deletion main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ resource "tls_private_key" "ssh" {

resource "azurerm_kubernetes_cluster" "main" {
location = coalesce(var.location, data.azurerm_resource_group.main.location)
name = coalesce(var.cluster_name, trim("${var.prefix}-aks", "-"))
name = "${local.cluster_name}${var.cluster_name_random_suffix ? substr(md5(uuid()), 0, 4) : ""}"
resource_group_name = data.azurerm_resource_group.main.name
automatic_channel_upgrade = var.automatic_channel_upgrade
azure_policy_enabled = var.azure_policy_enabled
Expand Down Expand Up @@ -524,6 +524,11 @@ resource "azurerm_kubernetes_cluster" "main" {
http_proxy_config[0].no_proxy,
kubernetes_version,
public_network_access_enabled,
# we might have a random suffix in cluster's name so we have to ignore it here, but we've traced user supplied cluster name by `null_resource.kubernetes_cluster_name_keeper` so when the name is changed we'll recreate this resource.
name,
]
replace_triggered_by = [
null_resource.kubernetes_cluster_name_keeper.id
]

precondition {
Expand Down Expand Up @@ -595,6 +600,12 @@ resource "azurerm_kubernetes_cluster" "main" {
}
}

resource "null_resource" "kubernetes_cluster_name_keeper" {
triggers = {
name = local.cluster_name
}
}

resource "null_resource" "kubernetes_version_keeper" {
triggers = {
version = var.kubernetes_version
Expand Down
13 changes: 11 additions & 2 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,13 @@ variable "cluster_name" {
description = "(Optional) The name for the AKS resources created in the specified Azure Resource Group. This variable overwrites the 'prefix' var (The 'prefix' var will still be applied to the dns_prefix if it is set)"
}

variable "cluster_name_random_suffix" {
type = bool
default = false
description = "Whether to add a random suffix on Aks cluster's name or not. `azurerm_kubernetes_cluster` resource defined in this module is `create_before_destroy = true` implicity now(described [here](https://github.com/Azure/terraform-azurerm-aks/issues/389)), without this random suffix we'll not be able to recreate this cluster directly due to the naming conflict."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neat: Aks should be all capital. AKS. Add a space in now(described. Should be now (described.

nullable = false
}

variable "confidential_computing" {
type = object({
sgx_quote_helper_enabled = bool
Expand Down Expand Up @@ -926,8 +933,9 @@ variable "node_pools" {
windows_profile = optional(object({
outbound_nat_enabled = optional(bool, true)
}))
workload_runtime = optional(string)
zones = optional(set(string))
workload_runtime = optional(string)
zones = optional(set(string))
create_before_destroy = optional(bool, true)
}))
default = {}
description = <<-EOT
Expand Down Expand Up @@ -1026,6 +1034,7 @@ variable "node_pools" {
}))
workload_runtime = (Optional) Used to specify the workload runtime. Allowed values are `OCIContainer` and `WasmWasi`. WebAssembly System Interface node pools are in Public Preview - more information and details on how to opt into the preview can be found in [this article](https://docs.microsoft.com/azure/aks/use-wasi-node-pools)
zones = (Optional) Specifies a list of Availability Zones in which this Kubernetes Cluster Node Pool should be located. Changing this forces a new Kubernetes Cluster Node Pool to be created.
create_before_destroy = (Optional) Create a new node pool before destroy the old one when Terraform must update an argument that cannot be updated in-place. Set this argument to `true` will add add a random suffix to pool's name to avoid conflict. Default to `true`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand now why @the-technat commented that this is error prone.

The create_before_destroy value is defined per node pool inside the map. Then we cannot enforce this is a boolean value.

We also need to test the mixed scenario where some nodepools have create_before_destroy = true and others have create_before_destroy = false.

}))
EOT
nullable = false
Expand Down
Loading