-
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
, azurerm_kubernetes_cluster_node_pool
- deprecate preview features
#26863
azurerm_kubernetes_cluster
, azurerm_kubernetes_cluster_node_pool
- deprecate preview features
#26863
Conversation
…- deprecate preview features
@ms-henglu Thank you for submitting this! It mostly LGTM. The only concern is that all the remaining properties are supported by the stable API version. For that reason, I think it might be a good idea to combine the API version changes in this PR as well, so that we can run the test to ensure everything else is working fine? |
Thanks for the suggestion, but actually I think it's difficult to do that, because the stable API version doesn't have the preview features which will make the compilation fail and having both stable and preview API SDK will make it difficult to maintain. |
@@ -554,26 +554,6 @@ func TestAccDataSourceKubernetesCluster_microsoftDefender(t *testing.T) { | |||
}) | |||
} | |||
|
|||
func TestAccDataSourceKubernetesCluster_customCaTrustCerts(t *testing.T) { |
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 shouldn't be removing these tests until after 4.0
- We still need the ability to run these in case there are severe issues with any features that are still available in 3.x
- In case of unforeseen consequences that may require us to roll back anything
Can you please skip the tests using the flag for now. We can worry about cleaning up unused tests and configs post major release.
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.
Got it! I've updated this PR as suggested.
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 update all of the deprecation messages with the suggestion left in-line. In addition can you also fix the panic introduced by these changes?
------- Stdout: -------
=== RUN TestAccKubernetesClusterNodePool_autoScale
=== PAUSE TestAccKubernetesClusterNodePool_autoScale
=== CONT TestAccKubernetesClusterNodePool_autoScale
------- Stderr: -------
panic: Invalid address to set: []string{"custom_ca_trust_enabled"}
goroutine 766 [running]:
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*ResourceData).Set(0xc0001e9180, {0x8628d88, 0x17}, {0x707a560, 0x0})
/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource_data.go:233 +0x2b5
github.com/hashicorp/terraform-provider-azurerm/internal/services/containers.resourceKubernetesClusterNodePoolRead(0xc0001e9180, {0x75851e0?, 0xc0015dc900?})
/opt/teamcity-agent/work/3337027aeff310bf/internal/services/containers/kubernetes_cluster_node_pool_resource.go:994 +0xa1f
github.com/hashicorp/terraform-provider-azurerm/internal/services/containers.resourceKubernetesClusterNodePoolCreate(0x0?, {0x75851e0?, 0xc0015dc900?})
/opt/teamcity-agent/work/3337027aeff310bf/internal/services/containers/kubernetes_cluster_node_pool_resource.go:773 +0x3655
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).create(0x8ec3508?, {0x8ec3508?, 0xc002b9b530?}, 0xd?, {0x75851e0?, 0xc0015dc900?})
------- Stdout: -------
=== RUN TestAccKubernetesClusterNodePoolDataSource_basic
=== PAUSE TestAccKubernetesClusterNodePoolDataSource_basic
=== CONT TestAccKubernetesClusterNodePoolDataSource_basic
------- Stderr: -------
panic: Invalid address to set: []string{"custom_ca_trust_enabled"}
goroutine 735 [running]:
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*ResourceData).Set(0xc003d19f00, {0x8628d88, 0x17}, {0x707a560, 0x0})
/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource_data.go:233 +0x2b5
github.com/hashicorp/terraform-provider-azurerm/internal/services/containers.resourceKubernetesClusterNodePoolRead(0xc003d19f00, {0x75851e0?, 0xc002947200?})
/opt/teamcity-agent/work/3337027aeff310bf/internal/services/containers/kubernetes_cluster_node_pool_resource.go:994 +0xa1f
github.com/hashicorp/terraform-provider-azurerm/internal/services/containers.resourceKubernetesClusterNodePoolCreate(0x0?, {0x75851e0?, 0xc002947200?})
/opt/teamcity-agent/work/3337027aeff310bf/internal/services/containers/kubernetes_cluster_node_pool_resource.go:773 +0x3655
Once that's done this should be good to go!
@@ -745,6 +733,21 @@ func dataSourceKubernetesCluster() *pluginsdk.Resource { | |||
Computed: true, | |||
Deprecated: "This property is deprecated and will be removed in v4.0 of the AzureRM Provider in favour of the `node_public_ip_enabled` property.", | |||
} | |||
resource.Schema["storage_profile"].Elem.(*pluginsdk.Resource).Schema["disk_driver_version"] = &pluginsdk.Schema{ | |||
Deprecated: "This feature is a preview feature and will be removed in version 4.0 of the AzureRM Provider.", |
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.
Please update all of the deprecation messages to the following:
This property is not available in the stable API and will be removed in v4.0 of the Azure Provider. Please see https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/guides/4.0-upgrade-guide#aks-migration-to-stable-api for more details
The link in the message isn't accessible right now, but it will be when the release goes out this week.
Hi @stephybun - I've fixed the panic, and the failed tests are not related to this change. |
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 👍
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. |
Community Note
Description
This PR deprecates the following preview features:
azurerm_kubernetes_cluster
data source:custom_ca_trust_certificates_base64
deprecated and removed in 4.0storage_profile.disk_driver_version
is deprecated and removed in 4.0azurerm_kubernetes_cluster
resource:custom_ca_trust_certificates_base64
is deprecated and removed in 4.0api_server_access_profile.subnet_id
is deprecated and removed in 4.0api_server_access_profile. vnet_integration_enabled
is deprecated and removed in 4.0default_node_pool. custom_ca_trust_enabled
is deprecated and removed in 4.0default_node_pool. message_of_the_day
is deprecated and removed in 4.0default_node_pool. workload_runtime
's allowed valueKataMshvVmIsolation
is deprecated and removed in 4.0storage_profile.disk_driver_version
is deprecated and removed in 4.0azurerm_kubernetes_cluster_node_pool
is deprecated and removed in 4.0custom_ca_trust_enabled
is deprecated and removed in 4.0message_of_the_day
is removedworkload_runtime
's allowed valueKataMshvVmIsolation
is deprecated and removed in 4.0After this PR, I'll open another PR to switch the SDK to stable api-version
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.