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.

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

azurerm_kubernetes_cluster - support for the default_node_pool.snapshot_id property #22708

Conversation

ms-henglu
Copy link
Contributor

fixes #22656

=== RUN   TestAccKubernetesCluster_snapshotId
=== PAUSE TestAccKubernetesCluster_snapshotId
=== CONT  TestAccKubernetesCluster_snapshotId
--- PASS: TestAccKubernetesCluster_snapshotId (1593.10s)
PASS

@@ -436,6 +436,8 @@ A `default_node_pool` block supports the following:

* `scale_down_mode` - (Optional) Specifies the autoscaling behaviour of the Kubernetes Cluster. Allowed values are `Delete` and `Deallocate`. Defaults to `Delete`.

* `snapshot_id` - (Optional) The ID of the Snapshot which should be used to create this default Node Pool.

Choose a reason for hiding this comment

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

Does it support a rolling update? Or will the node pool be recreated on snapshot_id change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @js-315385995 , yes, it could be updated, but it is done by cycling the system node pool of the cluster, temporary_name_for_rotation must be specified when changing it. I've updated the doc as well.

@zioproto
Copy link
Contributor

zioproto commented Aug 4, 2023

I tested like this:

  1. Take snapshot of system nodepool from an existing AKS cluster:
NODEPOOL_ID=$(az aks nodepool show -g istio-aks --cluster-name istio-aks -n system --query id -o tsv)
az group create --name snapshots --location eastus
az aks nodepool snapshot create --name systemsnap --resource-group snapshots --nodepool-id $NODEPOOL_ID --location eastus
  1. Get the ID of the new snapshot:
az aks nodepool snapshot show --name systemsnap --resource-group snapshots --query id -o tsv
  1. Run the following Terraform:
resource "azurerm_kubernetes_cluster" "aks" {
  name                = "aktest"
  location            = "eastus"
  resource_group_name = "istio-aks"
  dns_prefix          = "aks"
  kubernetes_version  = "1.25.11"
  identity {
    type = "SystemAssigned"
  }
  default_node_pool {
    name                = "default"
    node_count          = 1
    vm_size             = "Standard_DS3_v2"
    type                = "VirtualMachineScaleSets"
    enable_auto_scaling = true
    max_count           = 3
    min_count           = 1
    os_disk_size_gb     = 30
    snapshot_id =  "/subscriptions/<subscription_id_redacted>/resourceGroups/snapshots/providers/Microsoft.ContainerService/snapshots/systemsnap"
  }
}

provider "azurerm" {
  features {}
}

It created the cluster as expected.

@ms-henglu @stephybun is there anything else we can test ? What is missing to get this merged ?

thanks

@@ -1381,6 +1402,11 @@ func FlattenDefaultNodePool(input *[]managedclusters.ManagedClusterAgentPoolProf
scaleDownMode = *agentPool.ScaleDownMode
}

snapshotId := ""
if agentPool.CreationData != nil && agentPool.CreationData.SourceResourceId != nil {
snapshotId = *agentPool.CreationData.SourceResourceId
Copy link
Member

Choose a reason for hiding this comment

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

We should parse this ID insensitively before setting it to state

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I've updated it.

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks @ms-henglu LGTM 💯

@stephybun stephybun merged commit 010c47a into hashicorp:main Aug 9, 2023
22 checks passed
@github-actions github-actions bot added this to the v3.69.0 milestone Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for AKS node pool snapshot in the default_node_pool block in the resource azurerm_kubernetes_cluster
4 participants