terraform, azure and utoronto: an upgrade, misc to support it, and misc opportunistic details

terraform/azure/main.tf

@@ -93,22 +93,33 @@ resource "azurerm_kubernetes_cluster" "jupyterhub" {
     }
   }
 
-  # Core node-pool
+  # default_node_pool must be set, and it must be a node pool of system type
+  # that can't scale to zero. Due to that we are forced to use it, and have
+  # decided to use it as our core node pool.
+  #
+  # Most changes to this node pool forces a replace operation on the entire
+  # cluster. This can be avoided with v3.47.0+ of this provider by declaring
+  # temporary_name_for_rotation = "coreb".
+  #
+  # ref: https://github.com/hashicorp/terraform-provider-azurerm/pull/20628
+  # ref: https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/kubernetes_cluster#temporary_name_for_rotation.
+  #
   default_node_pool {
-    # Unfortunately, changing anything about VM type / size recreates *whole cluster
-    name                = "core"
-    vm_size             = var.core_node_vm_size
-    os_disk_size_gb     = 40
-    enable_auto_scaling = true
-    min_count           = 1
-    max_count           = 10
+    name                = var.core_node_pool.name
+    vm_size             = var.core_node_pool.vm_size
+    os_disk_size_gb     = var.core_node_pool.os_disk_size_gb
+    enable_auto_scaling = var.core_node_pool.enable_auto_scaling
+    min_count           = var.core_node_pool.enable_auto_scaling ? var.core_node_pool.min : null
+    max_count           = var.core_node_pool.enable_auto_scaling ? var.core_node_pool.max : null
+    node_count          = var.core_node_pool.node_count
+    kubelet_disk_type   = var.core_node_pool.kubelet_disk_type
     vnet_subnet_id      = azurerm_subnet.node_subnet.id
-    node_labels = {
+    node_labels = merge({
       "hub.jupyter.org/node-purpose" = "core",
       "k8s.dask.org/node-purpose"    = "core"
-    }
+    }, var.core_node_pool.labels)
 
-    orchestrator_version = var.kubernetes_version
+    orchestrator_version = coalesce(var.core_node_pool.kubernetes_version, var.kubernetes_version)
   }
 
   auto_scaler_profile {
@@ -138,21 +149,21 @@ resource "azurerm_kubernetes_cluster" "jupyterhub" {
 
 
 resource "azurerm_kubernetes_cluster_node_pool" "user_pool" {
-  for_each = var.notebook_nodes
+  for_each = var.user_node_pools
 
-  name                  = "nb${each.key}"
+  name                  = coalesce(each.value.name, each.key)
   kubernetes_cluster_id = azurerm_kubernetes_cluster.jupyterhub.id
   enable_auto_scaling   = true
-  os_disk_size_gb       = 200
+  os_disk_size_gb       = each.value.os_disk_size_gb
   vnet_subnet_id        = azurerm_subnet.node_subnet.id
+  kubelet_disk_type     = each.value.kubelet_disk_type
 
   orchestrator_version = each.value.kubernetes_version == "" ? var.kubernetes_version : each.value.kubernetes_version
 
   vm_size = each.value.vm_size
   node_labels = merge({
     "hub.jupyter.org/node-purpose" = "user",
     "k8s.dask.org/node-purpose"    = "scheduler"
-    "hub.jupyter.org/node-size"    = each.value.vm_size
   }, each.value.labels)
 
   node_taints = concat([
@@ -165,22 +176,21 @@ resource "azurerm_kubernetes_cluster_node_pool" "user_pool" {
 }
 
 resource "azurerm_kubernetes_cluster_node_pool" "dask_pool" {
-  # If dask_nodes is set, we use that. If it isn't, we use notebook_nodes.
-  # This lets us set dask_nodes to an empty array to get no dask nodes
-  for_each = var.dask_nodes
+  # If dask_node_pools is set, we use that. If it isn't, we use user_node_pools.
+  # This lets us set dask_node_pools to an empty array to get no dask nodes
+  for_each = var.dask_node_pools
 
   name                  = "dask${each.key}"
   kubernetes_cluster_id = azurerm_kubernetes_cluster.jupyterhub.id
   enable_auto_scaling   = true
-  os_disk_size_gb       = 200
+  os_disk_size_gb       = each.value.os_disk_size_gb
   vnet_subnet_id        = azurerm_subnet.node_subnet.id
 
   orchestrator_version = each.value.kubernetes_version == "" ? var.kubernetes_version : each.value.kubernetes_version
 
   vm_size = each.value.vm_size
   node_labels = merge({
     "k8s.dask.org/node-purpose" = "worker",
-    "hub.jupyter.org/node-size" = each.value.vm_size
   }, each.value.labels)
 
   node_taints = concat([

terraform/azure/projects/utoronto.tfvars

@@ -8,21 +8,39 @@ location                       = "canadacentral"
 storage_size = 8192
 ssh_pub_key  = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDQJ4h39UYNi1wybxAH+jCFkNK2aqRcuhDkQSMx0Hak5xkbt3KnT3cOwAgUP1Vt/SjhltSTuxpOHxiAKCRnjwRk60SxKhUNzPHih2nkfYTmBBjmLfdepDPSke/E0VWvTDIEXz/L8vW8aI0QGPXnXyqzEDO9+U1buheBlxB0diFAD3vEp2SqBOw+z7UgrGxXPdP+2b3AV+X6sOtd6uSzpV8Qvdh+QAkd4r7h9JrkFvkrUzNFAGMjlTb0Lz7qAlo4ynjEwzVN2I1i7cVDKgsGz9ZG/8yZfXXx+INr9jYtYogNZ63ajKR/dfjNPovydhuz5zQvQyxpokJNsTqt1CiWEUNj georgiana@georgiana"
 
-# FIXME: upgrade to 1.27.7, and then 1.28.3, based on the latest versions
-#        available via: az aks get-versions --location westus2 -o table
-#
-kubernetes_version = "1.26.3"
-
-# FIXME: upgrade core_node_vm_size to Standard_E4s_v5
-core_node_vm_size = "Standard_E4s_v3"
-
-notebook_nodes = {
-  "default" : {
-    # NOTE: min-max below was set to 0-86 retroactively to align with
-    #       observed state without understanding on why 0-86 was picked.
+# List available versions via: az aks get-versions --location westus2 -o table
+kubernetes_version = "1.28.3"
+
+core_node_pool = {
+  name : "core",
+  kubernetes_version : "1.28.3",
+
+  # FIXME: transition to "Standard_E2s_v5" nodes as they are large enough and
+  #        can more cheaply handle being forced to have 2-3 replicas for silly
+  #        reasons like three calico-typha pods. See
+  #        https://github.com/2i2c-org/infrastructure/issues/3592#issuecomment-1883269632.
+  #
+  vm_size : "Standard_E4s_v3",
+
+  # FIXME: stop using persistent disks for the nodes, use the variable default
+  #        "Temporary" instead
+  kubelet_disk_type : "OS",
+
+  # FIXME: use a larger os_disk_size_gb than 40, like the default of 100, to
+  #        avoid running low when few replicas are used
+  os_disk_size_gb : 40,
+
+  # FIXME: its nice to use autoscaling, but we end up with three replicas due to
+  #        https://github.com/2i2c-org/infrastructure/issues/3592#issuecomment-1883269632
+  #        and its a waste at least using Standard_E4s_v3 machines.
+  enable_auto_scaling : false,
+  node_count : 2,
+}
+
+user_node_pools = {
+  "usere8sv5" : {
     min : 0,
-    max : 86,
-    # FIXME: upgrade user nodes vm_size to Standard_E8s_v5
-    vm_size : "Standard_E8s_v3",
-  }
+    max : 100,
+    vm_size : "Standard_E8s_v5",
+  },
 }

terraform/azure/variables.tf

@@ -48,20 +48,6 @@ variable "kubernetes_version" {
 }
 
 
-variable "core_node_vm_size" {
-  type        = string
-  description = <<-EOT
-  VM Size to use for core nodes
-
-  Core nodes will always be on, and count as 'base cost'
-  for a cluster. We should try to run with as few of them
-  as possible.
-
-  WARNING: CHANGING THIS WILL DESTROY AND RECREATE THE CLUSTER!
-  EOT
-}
-
-
 variable "global_container_registry_name" {
   type        = string
   description = <<-EOT
@@ -92,27 +78,47 @@ variable "ssh_pub_key" {
   EOT
 }
 
-variable "notebook_nodes" {
+variable "core_node_pool" {
+  type = object({
+    name : optional(string, ""),
+    enable_auto_scaling = optional(bool, true),
+    min : optional(number, 1),
+    max : optional(number, 10),
+    node_count : optional(number),
+    vm_size : string,
+    labels : optional(map(string), {}),
+    taints : optional(list(string), []),
+    os_disk_size_gb : optional(number, 100),
+    kubernetes_version : optional(string, ""),
+    kubelet_disk_type : optional(string, "Temporary"),
+  })
+  description = "Core node pool"
+}
+
+variable "user_node_pools" {
   type = map(object({
+    name : optional(string, ""),
     min : number,
     max : number,
     vm_size : string,
     labels : optional(map(string), {}),
     taints : optional(list(string), []),
-    kubernetes_version : optional(string, "")
+    os_disk_size_gb : optional(number, 200),
+    kubernetes_version : optional(string, ""),
+    kubelet_disk_type : optional(string, "Temporary"),
   }))
-  description = "Notebook node pools to create"
+  description = "User node pools to create"
   default     = {}
 }
 
-variable "dask_nodes" {
+variable "dask_node_pools" {
   type = map(object({
     min : number,
     max : number,
     vm_size : string,
     labels : optional(map(string), {}),
     taints : optional(list(string), []),
-    kubernetes_version : optional(string, "")
+    kubernetes_version : optional(string, ""),
   }))
   description = "Dask node pools to create"
   default     = {}