-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 web_app_routing.dns_zone_ids
property
#26117
Conversation
…one_ids` property
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All tests are failing:
------- Stdout: -------
=== RUN TestAccKubernetesCluster_serviceMeshProfile
=== PAUSE TestAccKubernetesCluster_serviceMeshProfile
=== CONT TestAccKubernetesCluster_serviceMeshProfile
testcase.go:113: Step 3/4 error: Error running apply: exit status 1
Error: updating Kubernetes Cluster (Subscription: "*******"
Resource Group Name: "acctestRG-aks-240530033930946571"
Kubernetes Cluster Name: "acctestaks240530033930946571"): performing CreateOrUpdate: unexpected status 400 (400 Bad Request) with response: {
"code": "BadRequest",
"details": null,
"message": "Requested change in revisions is not allowed. Reason: ServiceMeshProfile revision field must not be empty.",
"subcode": ""
}
with azurerm_kubernetes_cluster.test,
on terraform_plugin_test.tf line 44, in resource "azurerm_kubernetes_cluster" "test":
44: resource "azurerm_kubernetes_cluster" "test" {
--- FAIL: TestAccKubernetesCluster_serviceMeshProfile (1267.51s)
FAIL
Hi @katbyte , this failed test is not related with this change and this test is already failing. I'll work on fixing it, but I think it won't block this PR, right? |
@@ -928,7 +952,7 @@ func TestAccKubernetesCluster_webAppRoutingPrivateDNS(t *testing.T) { | |||
check.That(data.ResourceName).Key("web_app_routing.0.web_app_routing_identity.#").HasValue("1"), | |||
), | |||
}, | |||
data.ImportStep(), | |||
data.ImportStep("web_app_routing.0.dns_zone_id", "web_app_routing.0.dns_zone_ids.#", "web_app_routing.0.dns_zone_ids.0"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to add these here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because in this test config, this feature is specified with dns_zone_id
. But in the import step, by default it will be imported as dns_zone_ids
, so there're some differences between them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test will fail in 4.0 mode, so can we add a t.Skip
for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ms-henglu, this test as well as TestAccKubernetesCluster_webAppRouting
will fail in 4.0 mode, can you please add the skip for these.
@@ -891,7 +891,7 @@ A `sysctl_config` block supports the following: | |||
|
|||
A `web_app_routing` block supports the following: | |||
|
|||
* `dns_zone_id` - (Required) Specifies the ID of the DNS Zone in which DNS entries are created for applications deployed to the cluster when Web App Routing is enabled. For Bring-Your-Own DNS zones this property should be set to an empty string `""`. | |||
* `dns_zone_ids` - (Required) Specifies the list of the DNS Zone IDs in which DNS entries are created for applications deployed to the cluster when Web App Routing is enabled. For Bring-Your-Own DNS zones this property should be set to an empty string `""`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Bring-Your-Own DNS zones this property should be set to an empty string
""
Is that still valid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I double checked with service team, and it should be an empty list now. Actually, user should set it to empty string/list if NOT using the Bring-Your-Own DNS zones feature. I've fixed it in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ms-henglu. Can you please also verify that the tests pass in 4.0 mode?
Sure. I tried, but the tests are failing in the 4.0 mode and unrelated with this change. I'll open another PR to fix them. |
@ms-henglu #26231 should resolve most of the 4.0 issues in the resource. |
Thanks @stephybun , the tests are passing in 4.0 mode now. |
@@ -928,7 +952,7 @@ func TestAccKubernetesCluster_webAppRoutingPrivateDNS(t *testing.T) { | |||
check.That(data.ResourceName).Key("web_app_routing.0.web_app_routing_identity.#").HasValue("1"), | |||
), | |||
}, | |||
data.ImportStep(), | |||
data.ImportStep("web_app_routing.0.dns_zone_id", "web_app_routing.0.dns_zone_ids.#", "web_app_routing.0.dns_zone_ids.0"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ms-henglu, this test as well as TestAccKubernetesCluster_webAppRouting
will fail in 4.0 mode, can you please add the skip for these.
Thanks @stephybun , I added the skip for those tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the FourPointOhBeta flag flip was accidentally committed, can you please revert that? This should be good to go then.
internal/features/four_point_oh.go
Outdated
@@ -38,5 +38,5 @@ func FourPointOh() bool { | |||
// This exists to allow breaking changes to be piped through the provider | |||
// during the development of 3.x until 4.0 is ready. | |||
func FourPointOhBeta() bool { | |||
return FourPointOh() || false | |||
return true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to revert this
My bad, I committed the test code by mistake. Now it's reverted. |
There was a problem hiding this 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 👍
<Actions> <action id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>changes detected:
	"hashicorp/azurerm" updated from "3.104.2" to "3.107.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.107.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.107.0
FEATURES:

* **New Resource:** `azurerm_data_protection_backup_policy_postgresql_flexible_server` ([#26024](https://github.com/hashicorp/terraform-provider-azurerm/issues/26024))

ENHANCEMENTS:

* dependencies: updating to `v0.20240604.1114748` of `github.com/hashicorp/go-azure-sdk` ([#26216](hashicorp/terraform-provider-azurerm#26216 `advisor`: update API version to `2023-01-01` ([#26205](hashicorp/terraform-provider-azurerm#26205 `keyvault`: handling the Resources API returning Key Vaults that have been deleted when populating the cache ([#26199](hashicorp/terraform-provider-azurerm#26199 `machinelearning`: update API version to `2024-04-01` ([#26168](hashicorp/terraform-provider-azurerm#26168 `network/privatelinkservices` - update to use `hashicorp/go-azure-sdk` ([#26212](hashicorp/terraform-provider-azurerm#26212 `network/serviceendpointpolicies` - update to use `hashicorp/go-azure-sdk` ([#26196](hashicorp/terraform-provider-azurerm#26196 `network/virtualnetworks` - update to use `hashicorp/go-azure-sdk` ([#26217](hashicorp/terraform-provider-azurerm#26217 `network/virtualwans`: update route resources to use `hashicorp/go-azure-sdk` ([#26189](hashicorp/terraform-provider-azurerm#26189 `azurerm_container_app_job` - support for the `key_vault_secret_id` and `identity` properties in the `secret` block ([#25969](hashicorp/terraform-provider-azurerm#25969 `azurerm_kubernetes_cluster` - support forthe `dns_zone_ids` popperty in the `web_app_routing` block ([#26117](hashicorp/terraform-provider-azurerm#26117 `azurerm_notification_hub_authorization_rule` - support for the `primary_connection_string` and `secondary_connection_string` properties ([#26188](hashicorp/terraform-provider-azurerm#26188 `azurerm_subnet` - support for the `default_outbound_access_enabled` property ([#25259](https://github.com/hashicorp/terraform-provider-azurerm/issues/25259))

BUG FIXES:

* `azurerm_api_management_named_value` - will now enforce setting the `secret` property when setting the `value_from_key_vault` property ([#26150](hashicorp/terraform-provider-azurerm#26150 `azurerm_storage_sync_server_endpoint` - improve pooling to work around api inconsistencies ([#26204](hashicorp/terraform-provider-azurerm#26204 `azurerm_virtual_network` - split create and update function to fix lifecycle - ignore ([#26246](hashicorp/terraform-provider-azurerm#26246 `azurerm_vpn_server_configuration` - split create and update function to fix lifecycle - ignore ([#26175](hashicorp/terraform-provider-azurerm#26175 `azurerm_vpn_server_configuration_policy_group` - split create and update function to fix lifecycle - ignore ([#26207](hashicorp/terraform-provider-azurerm#26207 `azurerm_vpn_site` - split create and update function to fix lifecycle - ignore changes ([#26163](https://github.com/hashicorp/terraform-provider-azurerm/issues/26163))

DEPRECATIONS:

* `azurerm_kubernetes_cluster` - the property `dns_zone_id` has been superseded by the property `dns_zone_ids` in the `web_app_routing` block ([#26117](hashicorp/terraform-provider-azurerm#26117 `azurerm_nginx_deployment` - the block `configuration` has been deprecated and superseded by the resource `azurerm_nginx_configuration` ([#25773](https://github.com/hashicorp/terraform-provider-azurerm/issues/25773))


</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/229/">Jenkins pipeline link</a> </action> </Actions> --- <table> <tr> <td width="77"> <img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli logo" width="50" height="50"> </td> <td> <p> Created automatically by <a href="https://www.updatecli.io/">Updatecli</a> </p> <details><summary>Options:</summary> <br /> <p>Most of Updatecli configuration is done via <a href="https://www.updatecli.io/docs/prologue/quick-start/">its manifest(s)</a>.</p> <ul> <li>If you close this pull request, Updatecli will automatically reopen it, the next time it runs.</li> <li>If you close this pull request and delete the base branch, Updatecli will automatically recreate it, erasing all previous commits made.</li> </ul> <p> Feel free to report any issues at <a href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br /> If you find this tool useful, do not hesitate to star <a href="https://github.com/updatecli/updatecli/stargazers">our GitHub repository</a> as a sign of appreciation, and/or to tell us directly on our <a href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>! </p> </details> </td> </tr> </table> --------- Signed-off-by: Damien Duportal <[email protected]> Co-authored-by: Jenkins Infra Bot (updatecli) <[email protected]> Co-authored-by: Damien Duportal <[email protected]>
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
fixes #26110
Community Note
Description
PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_resource
- support for thething1
property [GH-00000]This is a (please select all that apply):
Related Issue(s)
Fixes #0000
Note
If this PR changes meaningfully during the course of review please update the title and description as required.