azurerm_kubernetes_cluster_node_pool: max_surge and max_unavailable should not be required for spot nodes pools

internal/services/containers/kubernetes_cluster_node_pool_resource.go

@@ -523,6 +523,11 @@ func resourceKubernetesClusterNodePoolCreate(d *pluginsdk.ResourceData, meta int
 	spotMaxPrice := d.Get("spot_max_price").(float64)
 	t := d.Get("tags").(map[string]interface{})
 
+	upgradeSettings, err := expandAgentPoolUpgradeSettings(d.Get("upgrade_settings").([]interface{}), priority)
+	if err != nil {
+		return err
+	}
+
 	profile := agentpools.ManagedClusterAgentPoolProfileProperties{
 		OsType:                 pointer.To(agentpools.OSType(osType)),
 		EnableAutoScaling:      pointer.To(enableAutoScaling),
@@ -536,7 +541,7 @@ func resourceKubernetesClusterNodePoolCreate(d *pluginsdk.ResourceData, meta int
 		Tags:                   tags.Expand(t),
 		Type:                   pointer.To(agentpools.AgentPoolTypeVirtualMachineScaleSets),
 		VMSize:                 pointer.To(d.Get("vm_size").(string)),
-		UpgradeSettings:        expandAgentPoolUpgradeSettings(d.Get("upgrade_settings").([]interface{})),
+		UpgradeSettings:        upgradeSettings,
 		WindowsProfile:         expandAgentPoolWindowsProfile(d.Get("windows_profile").([]interface{})),
 
 		// this must always be sent during creation, but is optional for auto-scaled clusters during update
@@ -879,7 +884,12 @@ func resourceKubernetesClusterNodePoolUpdate(d *pluginsdk.ResourceData, meta int
 
 	if d.HasChange("upgrade_settings") {
 		upgradeSettingsRaw := d.Get("upgrade_settings").([]interface{})
-		props.UpgradeSettings = expandAgentPoolUpgradeSettings(upgradeSettingsRaw)
+		priority := d.Get("priority").(string)
+		upgradeSettings, err := expandAgentPoolUpgradeSettings(upgradeSettingsRaw, priority)
+		if err != nil {
+			return err
+		}
+		props.UpgradeSettings = upgradeSettings
 	}
 
 	if d.HasChange("scale_down_mode") {
@@ -1265,21 +1275,23 @@ func upgradeSettingsSchemaNodePoolResource() *pluginsdk.Schema {
 		Elem: &pluginsdk.Resource{
 			Schema: map[string]*pluginsdk.Schema{
 				"max_surge": {
-					Type:         pluginsdk.TypeString,
-					Optional:     true,
-					ValidateFunc: validation.StringIsNotEmpty,
-					ExactlyOneOf: []string{"upgrade_settings.0.max_surge", "upgrade_settings.0.max_unavailable"},
+					Type:          pluginsdk.TypeString,
+					Optional:      true,
+					ValidateFunc:  validation.StringIsNotEmpty,
+					ConflictsWith: []string{"upgrade_settings.0.max_unavailable"},
+					Description:   "The maximum number of nodes that can be created during upgrade. This cannot be set when `priority` is set to `Spot`.",
 				},
 				"drain_timeout_in_minutes": {
 					Type:         pluginsdk.TypeInt,
 					Optional:     true,
 					ValidateFunc: validation.IntAtLeast(0),
 				},
 				"max_unavailable": {
-					Type:         pluginsdk.TypeString,
-					Optional:     true,
-					ValidateFunc: validation.StringIsNotEmpty,
-					ExactlyOneOf: []string{"upgrade_settings.0.max_surge", "upgrade_settings.0.max_unavailable"},
+					Type:          pluginsdk.TypeString,
+					Optional:      true,
+					ValidateFunc:  validation.StringIsNotEmpty,
+					ConflictsWith: []string{"upgrade_settings.0.max_surge"},
+					Description:   "The maximum number of nodes that can be unavailable during upgrade. This cannot be set when `priority` is set to `Spot`.",
 				},
 				"node_soak_duration_in_minutes": {
 					Type:         pluginsdk.TypeInt,
@@ -1368,20 +1380,40 @@ func expandAgentPoolKubeletConfig(input []interface{}) *agentpools.KubeletConfig
 	return result
 }
 
-func expandAgentPoolUpgradeSettings(input []interface{}) *agentpools.AgentPoolUpgradeSettings {
+func expandAgentPoolUpgradeSettings(input []interface{}, priority string) (*agentpools.AgentPoolUpgradeSettings, error) {
 	setting := &agentpools.AgentPoolUpgradeSettings{}
 	if len(input) == 0 || input[0] == nil {
-		return nil
+		return nil, nil
 	}
 
 	v := input[0].(map[string]interface{})
-	if maxSurgeRaw := v["max_surge"].(string); maxSurgeRaw != "" {
-		setting.MaxSurge = pointer.To(maxSurgeRaw)
-		setting.MaxUnavailable = pointer.To("0")
+	isSpot := priority == string(agentpools.ScaleSetPrioritySpot)
+
+	maxSurgeRaw := v["max_surge"].(string)
+	maxUnavailableRaw := v["max_unavailable"].(string)
+
+	if isSpot {
+		if maxSurgeRaw != "" {
+			return nil, fmt.Errorf("`max_surge` cannot be set when `priority` is set to `Spot`. Spot pools do not support `max_surge`")
+		}
+		if maxUnavailableRaw != "" {
+			return nil, fmt.Errorf("`max_unavailable` cannot be set when `priority` is set to `Spot`. Spot pools do not support `max_unavailable`")
+		}
 	}
-	if maxUnavailableRaw := v["max_unavailable"].(string); maxUnavailableRaw != "" {
-		setting.MaxUnavailable = pointer.To(maxUnavailableRaw)
-		setting.MaxSurge = pointer.To("0")
+
+	if !isSpot {
+		if maxSurgeRaw == "" && maxUnavailableRaw == "" {
+			return nil, fmt.Errorf("either `max_surge` or `max_unavailable` must be specified in `upgrade_settings` when `priority` is not `Spot`")
+		}
+
+		if maxSurgeRaw != "" {
+			setting.MaxSurge = pointer.To(maxSurgeRaw)
+			setting.MaxUnavailable = pointer.To("0")
+		}
+		if maxUnavailableRaw != "" {
+			setting.MaxUnavailable = pointer.To(maxUnavailableRaw)
+			setting.MaxSurge = pointer.To("0")
+		}
 	}
 
 	if drainTimeoutInMinutesRaw, ok := v["drain_timeout_in_minutes"].(int); ok {
@@ -1393,7 +1425,7 @@ func expandAgentPoolUpgradeSettings(input []interface{}) *agentpools.AgentPoolUp
 	if undrainableNodeBehaviorRaw, ok := v["undrainable_node_behavior"].(string); ok && undrainableNodeBehaviorRaw != "" {
 		setting.UndrainableNodeBehavior = pointer.To(agentpools.UndrainableNodeBehavior(undrainableNodeBehaviorRaw))
 	}
-	return setting
+	return setting, nil
 }
 
 func flattenAgentPoolUpgradeSettings(input *agentpools.AgentPoolUpgradeSettings) []interface{} {

internal/services/containers/kubernetes_cluster_node_pool_resource_test.go

@@ -6,6 +6,7 @@ package containers_test
 import (
 	"context"
 	"fmt"
+	"regexp"
 	"strings"
 	"testing"
 	"time"
@@ -547,6 +548,57 @@ func TestAccKubernetesClusterNodePool_spot(t *testing.T) {
 	})
 }
 
+func TestAccKubernetesClusterNodePool_spotWithMaxSurge(t *testing.T) {
+	data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster_node_pool", "test")
+	r := KubernetesClusterNodePoolResource{}
+
+	data.ResourceTest(t, r, []acceptance.TestStep{
+		{
+			Config:      r.spotWithMaxSurgeConfig(data),
+			ExpectError: regexp.MustCompile("`max_surge` cannot be set when `priority` is set to `Spot`"),
+		},
+	})
+}
+
+func TestAccKubernetesClusterNodePool_spotWithMaxUnavailable(t *testing.T) {
+	data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster_node_pool", "test")
+	r := KubernetesClusterNodePoolResource{}
+
+	data.ResourceTest(t, r, []acceptance.TestStep{
+		{
+			Config:      r.spotWithMaxUnavailableConfig(data),
+			ExpectError: regexp.MustCompile("`max_unavailable` cannot be set when `priority` is set to `Spot`"),
+		},
+	})
+}
+
+func TestAccKubernetesClusterNodePool_upgradeSettingsWithoutMaxSurgeOrMaxUnavailable(t *testing.T) {
+	data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster_node_pool", "test")
+	r := KubernetesClusterNodePoolResource{}
+
+	data.ResourceTest(t, r, []acceptance.TestStep{
+		{
+			Config:      r.upgradeSettingsWithoutMaxSurgeOrMaxUnavailableConfig(data),
+			ExpectError: regexp.MustCompile("either `max_surge` or `max_unavailable` must be specified in `upgrade_settings` when `priority` is not `Spot`"),
+		},
+	})
+}
+
+func TestAccKubernetesClusterNodePool_spotWithOtherUpgradeSettings(t *testing.T) {
+	data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster_node_pool", "test")
+	r := KubernetesClusterNodePoolResource{}
+
+	data.ResourceTest(t, r, []acceptance.TestStep{
+		{
+			Config: r.spotWithOtherUpgradeSettingsConfig(data),
+			Check: acceptance.ComposeTestCheckFunc(
+				check.That(data.ResourceName).ExistsInAzure(r),
+			),
+		},
+		data.ImportStep(),
+	})
+}
+
 func TestAccKubernetesClusterNodePool_upgradeSettingsMaxSurge(t *testing.T) {
 	data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster_node_pool", "test")
 	r := KubernetesClusterNodePoolResource{}
@@ -2331,6 +2383,115 @@ resource "azurerm_kubernetes_cluster_node_pool" "test" {
 `, r.templateConfig(data))
 }
 
+func (r KubernetesClusterNodePoolResource) spotWithMaxSurgeConfig(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+provider "azurerm" {
+  features {}
+}
+
+%s
+
+resource "azurerm_kubernetes_cluster_node_pool" "test" {
+  name                  = "internal"
+  kubernetes_cluster_id = azurerm_kubernetes_cluster.test.id
+  vm_size               = "Standard_DS2_v2"
+  node_count            = 1
+  priority              = "Spot"
+  eviction_policy       = "Delete"
+  spot_max_price        = 0.5
+  upgrade_settings {
+    max_surge = "10%%"
+  }
+  node_labels = {
+    "kubernetes.azure.com/scalesetpriority" = "spot"
+  }
+  node_taints = [
+    "kubernetes.azure.com/scalesetpriority=spot:NoSchedule"
+  ]
+}
+`, r.templateConfig(data))
+}
+
+func (r KubernetesClusterNodePoolResource) spotWithMaxUnavailableConfig(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+provider "azurerm" {
+  features {}
+}
+
+%s
+
+resource "azurerm_kubernetes_cluster_node_pool" "test" {
+  name                  = "internal"
+  kubernetes_cluster_id = azurerm_kubernetes_cluster.test.id
+  vm_size               = "Standard_DS2_v2"
+  node_count            = 1
+  priority              = "Spot"
+  eviction_policy       = "Delete"
+  spot_max_price        = 0.5
+  upgrade_settings {
+    max_unavailable = "1"
+  }
+  node_labels = {
+    "kubernetes.azure.com/scalesetpriority" = "spot"
+  }
+  node_taints = [
+    "kubernetes.azure.com/scalesetpriority=spot:NoSchedule"
+  ]
+}
+`, r.templateConfig(data))
+}
+
+func (r KubernetesClusterNodePoolResource) upgradeSettingsWithoutMaxSurgeOrMaxUnavailableConfig(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+provider "azurerm" {
+  features {}
+}
+
+%s
+
+resource "azurerm_kubernetes_cluster_node_pool" "test" {
+  name                  = "internal"
+  kubernetes_cluster_id = azurerm_kubernetes_cluster.test.id
+  vm_size               = "Standard_DS2_v2"
+  node_count            = 1
+  upgrade_settings {
+    drain_timeout_in_minutes = 15
+  }
+}
+`, r.templateConfig(data))
+}
+
+func (r KubernetesClusterNodePoolResource) spotWithOtherUpgradeSettingsConfig(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+provider "azurerm" {
+  features {}
+}
+
+%s
+
+resource "azurerm_kubernetes_cluster_node_pool" "test" {
+  name                  = "internal"
+  kubernetes_cluster_id = azurerm_kubernetes_cluster.test.id
+  vm_size               = "Standard_DS2_v2"
+  node_count            = 1
+  priority              = "Spot"
+  eviction_policy       = "Delete"
+  spot_max_price        = 0.5
+  upgrade_settings {
+    drain_timeout_in_minutes      = 15
+    node_soak_duration_in_minutes = 5
+    undrainable_node_behavior     = "Schedule"
+  }
+  node_labels = {
+    "kubernetes.azure.com/scalesetpriority" = "spot"
+  }
+  node_taints = [
+    "kubernetes.azure.com/scalesetpriority=spot:NoSchedule"
+  ]
+}
+`, r.templateConfig(data))
+}
+
 func (r KubernetesClusterNodePoolResource) completeUpgradeSettingsWithMaxSurge(data acceptance.TestData, maxSurge string, drainTimeout int, nodeSoakDuration int, undrainableBehavior string) string {
 	template := r.templateConfig(data)
 	return fmt.Sprintf(`

website/docs/r/kubernetes_cluster_node_pool.html.markdown

@@ -314,7 +314,7 @@ A `upgrade_settings` block supports the following:
 
 * `max_unavailable` - (Optional) The maximum number or percentage of nodes which can be unavailable during the upgrade.
 
-~> **Note:** Exactly one of `max_surge` or `max_unavailable` must be specified.
+~> **Note:** Exactly one of `max_surge` or `max_unavailable` must be specified, unless `priority` is set to `Spot`. Spot node pools do not support `max_surge` or `max_unavailable`.
 
 * `undrainable_node_behavior` - (Optional) Specifies the action when a node is undrainable during upgrade. Possible values are `Cordon` and `Schedule`. Unsetting this after configuring it will force a new resource to be created.