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

azurerm_kubernetes_cluster - fix tests #25200

Merged
merged 11 commits into from
Apr 15, 2024
Merged

azurerm_kubernetes_cluster - fix tests #25200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1c29aa0
`azurerm_kubernetes_cluster` - fix tests
ms-henglu Mar 11, 2024
15e5271
fix more
ms-henglu Mar 12, 2024
862b86f
add deprecated message for Azure AD Integration (legacy)
ms-henglu Mar 19, 2024
810ad36
add default value for max_surge
ms-henglu Mar 19, 2024
2b0d7ca
update
ms-henglu Mar 20, 2024
1586c1b
update
ms-henglu Mar 20, 2024
ff1dd66
remove the block in 4.0
ms-henglu Mar 21, 2024
b0fbe33
deprecate managed
ms-henglu Apr 1, 2024
89db74d
update more tests
ms-henglu Apr 8, 2024
8c10e6c
update more tests
ms-henglu Apr 9, 2024
303bbfe
Merge branch 'main' into issue-24020
ms-henglu Apr 15, 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
45 changes: 45 additions & 0 deletions internal/services/containers/kubernetes_cluster_addons_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,9 @@ resource "azurerm_kubernetes_cluster" "test" {
node_count = 1
vm_size = "Standard_DS2_v2"
vnet_subnet_id = azurerm_subnet.test.id
upgrade_settings {
max_surge = "10%%"
}
}

aci_connector_linux {
Expand Down Expand Up @@ -412,6 +415,9 @@ resource "azurerm_kubernetes_cluster" "test" {
node_count = 1
vm_size = "Standard_DS2_v2"
vnet_subnet_id = azurerm_subnet.test.id
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -454,6 +460,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

azure_policy_enabled = %t
Expand Down Expand Up @@ -514,6 +523,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

oms_agent {
Expand Down Expand Up @@ -576,6 +588,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

oms_agent {
Expand Down Expand Up @@ -619,6 +634,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -657,6 +675,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

http_application_routing_enabled = %t
Expand Down Expand Up @@ -778,6 +799,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

ingress_application_gateway {
Expand Down Expand Up @@ -824,6 +848,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

ingress_application_gateway {
Expand Down Expand Up @@ -867,6 +894,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -919,6 +949,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

ingress_application_gateway {
Expand Down Expand Up @@ -966,6 +999,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

open_service_mesh_enabled = %t
Expand Down Expand Up @@ -1006,6 +1042,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

key_vault_secrets_provider {
Expand Down Expand Up @@ -1049,6 +1088,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

confidential_computing {
Expand Down Expand Up @@ -1091,6 +1133,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down
48 changes: 48 additions & 0 deletions internal/services/containers/kubernetes_cluster_auth_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ func TestAccKubernetesCluster_roleBasedAccessControlDisabled(t *testing.T) {
}

func TestAccKubernetesCluster_roleBasedAccessControlAAD(t *testing.T) {
t.Skip("Azure AD Integration (legacy) (https://aka.ms/aks/aad-legacy) is deprecated, the cluster could not be created with the Azure AD integration (legacy) enabled.")
Copy link
Member

Choose a reason for hiding this comment

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

Should we also add a deprecation message to the affected properties?

Copy link
Contributor Author

@ms-henglu ms-henglu Mar 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The deprecated use case is specifying managed = false, so I think we couldn't add the deprecated message to the field.

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

Copy link
Member

Choose a reason for hiding this comment

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

If AKS doesn't support clusters where managed = false i.e. where client_app_id, server_app_id, server_app_secret can be specified for that use case, then those properties (including managed) should have deprecation messages and be removed in v4.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it! I'll fix it in the next commit.

data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster", "test")
r := KubernetesClusterResource{}
clientData := data.Client()
Expand All @@ -167,6 +168,7 @@ func TestAccKubernetesCluster_roleBasedAccessControlAAD(t *testing.T) {
}

func TestAccKubernetesCluster_roleBasedAccessControlAADUpdate(t *testing.T) {
t.Skip("Azure AD Integration (legacy) (https://aka.ms/aks/aad-legacy) is deprecated, the cluster could not be created with the Azure AD integration (legacy) enabled.")
data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster", "test")
r := KubernetesClusterResource{}

Expand All @@ -193,6 +195,7 @@ func TestAccKubernetesCluster_roleBasedAccessControlAADUpdate(t *testing.T) {
}

func TestAccKubernetesCluster_roleBasedAccessControlAADUpdateToManaged(t *testing.T) {
t.Skip("Azure AD Integration (legacy) (https://aka.ms/aks/aad-legacy) is deprecated, the cluster could not be created with the Azure AD integration (legacy) enabled.")
data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster", "test")
r := KubernetesClusterResource{}
clientData := data.Client()
Expand Down Expand Up @@ -458,6 +461,9 @@ resource "azurerm_kubernetes_cluster" "test" {
node_count = 1
vm_size = "Standard_DS2_v2"
vnet_subnet_id = azurerm_subnet.test.id
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -501,6 +507,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -537,6 +546,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -575,6 +587,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -630,6 +645,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -675,6 +693,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -715,6 +736,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -755,6 +779,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -800,6 +827,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -845,6 +875,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
Copy link
Member

Choose a reason for hiding this comment

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

Can you also update the default value for this property for 4.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The max_surge = 10% is only for the clusters created with k8s version = 1.28+, so I think we can't add the default value for this field.

Copy link
Member

Choose a reason for hiding this comment

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

The window for supported versions will continue to move so at some point clusters can only be created with k8s version 1.28+. I don't see what is preventing us from having a default value in place for that field for v4.0.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it correct 10%% with double %% ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this syntax is required for % values in the test config

}
}

identity {
Expand Down Expand Up @@ -890,6 +923,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -934,6 +970,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 2
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -977,6 +1016,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -1022,6 +1064,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down Expand Up @@ -1082,6 +1127,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

service_principal {
Expand Down
1 change: 1 addition & 0 deletions internal/services/containers/kubernetes_cluster_data_source_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ func TestAccDataSourceKubernetesCluster_roleBasedAccessControlAAD_OlderKubernete
}

func TestAccDataSourceKubernetesCluster_roleBasedAccessControlAAD(t *testing.T) {
t.Skip("Azure AD Integration (legacy) (https://aka.ms/aks/aad-legacy) is deprecated, the cluster could not be created with the Azure AD integration (legacy) enabled.")
data := acceptance.BuildTestData(t, "data.azurerm_kubernetes_cluster", "test")
r := KubernetesClusterDataSource{}
clientId := os.Getenv("ARM_CLIENT_ID")
Expand Down
3 changes: 3 additions & 0 deletions internal/services/containers/kubernetes_cluster_extension_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ resource "azurerm_kubernetes_cluster" "test" {
name = "default"
node_count = 1
vm_size = "Standard_DS2_v2"
upgrade_settings {
max_surge = "10%%"
}
}

identity {
Expand Down
Loading
Loading