`azurerm_eventhub_namespace` / `azurerm_servicebus_namespace` - deprecate the `zone_redundant` field in v4.0 #26611

WodansSon · 2024-07-12T01:04:26Z

Community Note

Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

The zone_redundant field, per the service team, is now going to be 100% controlled by the API. So exposing this as a field to the end user no longer makes sense.

PR Checklist

I have followed the guidelines in our Contributing Documentation.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Changes to existing Resource / Data Source

I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
I have written new tests for my resource or datasource changes & updated any relevent documentation.
I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
(For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

EventHub Test Results:

Note: The one failing test case is an environment issue not related to the code change in this PR.

ServiceBus Test Results:

NOTE: The two failures are v4.0 related.

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

azurerm_eventhub_namespace / azurerm_servicebus_namespace - deprecate the zone_redundant field in v4.0

This is a (please select all that apply):

Bug Fix
New Feature (ie adding a service, resource, or data source)
Enhancement
Breaking Change

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

stephybun

Thanks for this @WodansSon. The breaking schema changes look good! The test needs some slight re-work though.

stephybun · 2024-07-12T07:56:06Z

internal/services/eventhub/eventhub_namespace_resource_test.go

@@ -399,7 +400,7 @@ func TestAccEventHubNamespace_BasicWithSkuUpdate(t *testing.T) {
 			),
 		},
 		{
-			Config: r.premium(data),
+			Config: r.premium(data, features.FourPointOhBeta()),


I appreciate the effort here to reduce the lines of code and repetition, but keeping the clean up effort of all the FourPointOh/FourPointOhBeta code in mind, this is more work to unpick than if the config is just duplicated.

We try and leave the test alone if possible and update the function returning the config to return the right config based on the flag, so in this case

func (EventHubNamespaceResource) premium(data acceptance.TestData) string { if !features.FourPointOhBeta() { return fmt.Sprintf(` provider "azurerm" { features {} } resource "azurerm_resource_group" "test" { name = "acctestRG-eh-%d" location = "%s" } resource "azurerm_eventhub_namespace" "test" { name = "acctesteventhubnamespace-%d" location = azurerm_resource_group.test.location resource_group_name = azurerm_resource_group.test.name sku = "Premium" capacity = "1" zone_redundant = true } `, data.RandomInteger, data.Locations.Primary, data.RandomInteger) } return fmt.Sprintf(` provider "azurerm" { features {} } resource "azurerm_resource_group" "test" { name = "acctestRG-eh-%d" location = "%s" } resource "azurerm_eventhub_namespace" "test" { name = "acctesteventhubnamespace-%d" location = azurerm_resource_group.test.location resource_group_name = azurerm_resource_group.test.name sku = "Premium" capacity = "1" } `, data.RandomInteger, data.Locations.Primary, data.RandomInteger) }

stephybun · 2024-07-12T07:56:27Z

internal/services/servicebus/servicebus_namespace_resource_test.go

@@ -151,6 +152,10 @@ func TestAccAzureRMServiceBusNamespace_premiumMessagingPartition(t *testing.T) {
 }

 func TestAccAzureRMServiceBusNamespace_zoneRedundant(t *testing.T) {
+	if features.FourPointOhBeta() {
+		t.Skipf("Skippped as 'zone_redundant' property is deprecated in 4.0")


Suggested change

t.Skipf("Skippped as 'zone_redundant' property is deprecated in 4.0")

t.Skipf("Skipped as 'zone_redundant' property is deprecated in 4.0")

stephybun · 2024-07-12T07:56:56Z

website/docs/d/eventhub_namespace.html.markdown

@@ -44,6 +44,8 @@ output "eventhub_namespace_id" {

 * `zone_redundant` - Is this EventHub Namespace deployed across Availability Zones?

+~> **Note:** The `zone_redundant` field will be removed in v4.0 of the provider.


Anyone using the field will be notified by the deprecation messages added to the field so no need to add this to the documentation as well

Suggested change

~> **Note:** The `zone_redundant` field will be removed in v4.0 of the provider.

stephybun · 2024-07-12T07:57:03Z

website/docs/d/servicebus_namespace.html.markdown

@@ -41,6 +41,8 @@ output "location" {

 * `zone_redundant` - Whether or not this ServiceBus Namespace is zone redundant.

+~> **Note:** The `zone_redundant` field will be removed in v4.0 of the provider.


Suggested change

~> **Note:** The `zone_redundant` field will be removed in v4.0 of the provider.

stephybun · 2024-07-12T07:57:11Z

website/docs/r/eventhub_namespace.html.markdown

-~> **Note:** For eventhub premium namespace, `zone_redundant` is computed by api based on the availability zone feature in each region. User's input will be overridden. Please explicitly sets the property to `true` when creating the premium namespace in a region that supports availability zone since the default value is `false` in 3.0 provider.
+~> **Note:** For eventhub premium namespace, `zone_redundant` is computed by the api based on the availability zone feature in each region, user's input will be overridden. Please explicitly sets the property to `true` when creating the premium namespace in a region that supports availability zone since the default value is `false` in 3.0 provider.
+
+~> **Note:** The `zone_redundant` field will be removed in v4.0 of the provider.


Suggested change

~> **Note:** The `zone_redundant` field will be removed in v4.0 of the provider.

stephybun · 2024-07-12T07:57:17Z

website/docs/r/servicebus_namespace.html.markdown

@@ -65,6 +65,8 @@ The following arguments are supported:

 * `zone_redundant` - (Optional) Whether or not this resource is zone redundant. `sku` needs to be `Premium`. Changing this forces a new resource to be created.

+~> **Note:** The `zone_redundant` field will be removed in v4.0 of the provider.


Suggested change

~> **Note:** The `zone_redundant` field will be removed in v4.0 of the provider.

katbyte

test failure:

------- Stdout: -------
=== RUN   TestAccAzureRMServiceBusNamespace_basicCapacity
=== PAUSE TestAccAzureRMServiceBusNamespace_basicCapacity
=== CONT  TestAccAzureRMServiceBusNamespace_basicCapacity
    testcase.go:113: Step 1/1, expected an error with pattern, no match on: Error running apply: exit status 1
        
        Error: service bus SKU "Basic" only supports `capacity` of 0
        
          with azurerm_servicebus_namespace.test,
          on terraform_plugin_test.tf line 30, in resource "azurerm_servicebus_namespace" "test":
          30: resource "azurerm_servicebus_namespace" "test" {
        
--- FAIL: TestAccAzureRMServiceBusNamespace_basicCapacity (42.32s)
FAIL

katbyte · 2024-07-19T01:12:58Z

internal/services/eventhub/eventhub_namespace_data_source.go

+		resource.Schema["zone_redundant"] = &pluginsdk.Schema{
+			Type:     pluginsdk.TypeBool,
+			Computed: true,
+		}


we should still add a deprecation message here?

katbyte · 2024-07-19T01:13:09Z

internal/services/servicebus/servicebus_namespace_data_source.go

+		resource.Schema["zone_redundant"] = &pluginsdk.Schema{
+			Type:     pluginsdk.TypeBool,
+			Computed: true,
+		}


katbyte

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.112.0" to "3.113.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.113.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.113.0
ENHANCEMENTS:

* dependencies: updating to `v0.20240715.1100358` of `hashicorp/go-azure-sdk` ([#26638](hashicorp/terraform-provider-azurerm#26638 `storage` - updating to use `hashicorp/go-azure-sdk` ([#26218](https://github.com/hashicorp/terraform-provider-azurerm/issues/26218))

BUG FIXES:

* `azurerm_storage_account` - fix a validation bug when replacing a StorageV2 account with a StorageV1 account ([#26639](hashicorp/terraform-provider-azurerm#26639 `azurerm_storage_account` - resolve an issue refreshing blob or queue properties after recreation ([#26218](hashicorp/terraform-provider-azurerm#26218 `azurerm_storage_account` - resolve an issue setting tags for an existing storage account where a policy mandates them ([#26218](hashicorp/terraform-provider-azurerm#26218 `azurerm_storage_account` - fix a persistent diff with the `customer_managed_key` block ([#26218](hashicorp/terraform-provider-azurerm#26218 `azurerm_storage_account` - resolve several consistency related issues when crreating a new storage account ([#26218](https://github.com/hashicorp/terraform-provider-azurerm/issues/26218))

DEPRECATIONS:

* `azurerm_eventhub_namespace` - deprecate the `zone_redundant` field in v4.0 ([#26611](hashicorp/terraform-provider-azurerm#26611 `azurerm_servicebus_namespace` - deprecate the `zone_redundant` field in v4.0 ([#26611](hashicorp/terraform-provider-azurerm#26611; 

</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/331/">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> Co-authored-by: Jenkins Infra Bot (updatecli) <[email protected]>

github-actions · 2024-08-19T02:10:27Z

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

Initial Check-in...

ef683b0

github-actions bot added documentation service/event-hubs EventHubs service/service-bus size/M labels Jul 12, 2024

stephybun requested changes Jul 12, 2024

View reviewed changes

github-actions bot added the waiting-response label Jul 12, 2024

Address PR comments...

9385808

github-actions bot added waiting-response and removed waiting-response labels Jul 12, 2024

DOH! Missed one...

6a4b764

github-actions bot added waiting-response and removed waiting-response labels Jul 12, 2024

Fix error msg casing lint error...

67c233c

github-actions bot added waiting-response and removed waiting-response labels Jul 12, 2024

terrafmt...

4fadeb7

github-actions bot removed the waiting-response label Jul 12, 2024

WodansSon added this to the v3.113.0 milestone Jul 16, 2024

katbyte requested changes Jul 19, 2024

View reviewed changes

github-actions bot added the waiting-response label Jul 19, 2024

Address PR comments...

35a8372

github-actions bot removed the waiting-response label Jul 19, 2024

Fix error msg casing for test cases...

b5e16d9

github-actions bot added size/L and removed size/M labels Jul 19, 2024

katbyte approved these changes Jul 19, 2024

View reviewed changes

WodansSon merged commit dab553d into main Jul 19, 2024
34 checks passed

WodansSon deleted the d_eventhub_servicebus_dep branch July 19, 2024 04:10

WodansSon added a commit that referenced this pull request Jul 19, 2024

updating including #26611

5ba841d

github-actions bot locked as resolved and limited conversation to collaborators Aug 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_eventhub_namespace` / `azurerm_servicebus_namespace` - deprecate the `zone_redundant` field in v4.0 #26611

`azurerm_eventhub_namespace` / `azurerm_servicebus_namespace` - deprecate the `zone_redundant` field in v4.0 #26611

WodansSon commented Jul 12, 2024 •

edited

Loading

stephybun left a comment

stephybun Jul 12, 2024

WodansSon Jul 12, 2024

stephybun Jul 12, 2024

WodansSon Jul 12, 2024

stephybun Jul 12, 2024

WodansSon Jul 12, 2024

stephybun Jul 12, 2024

WodansSon Jul 12, 2024

stephybun Jul 12, 2024

WodansSon Jul 12, 2024

stephybun Jul 12, 2024

WodansSon Jul 12, 2024

katbyte left a comment

katbyte Jul 19, 2024

WodansSon Jul 19, 2024

katbyte Jul 19, 2024

WodansSon Jul 19, 2024

katbyte left a comment

github-actions bot commented Aug 19, 2024

	t.Skipf("Skippped as 'zone_redundant' property is deprecated in 4.0")
	t.Skipf("Skipped as 'zone_redundant' property is deprecated in 4.0")

		@@ -44,6 +44,8 @@ output "eventhub_namespace_id" {

		* `zone_redundant` - Is this EventHub Namespace deployed across Availability Zones?

		~> Note: The `zone_redundant` field will be removed in v4.0 of the provider.

		@@ -41,6 +41,8 @@ output "location" {

		* `zone_redundant` - Whether or not this ServiceBus Namespace is zone redundant.

		~> Note: The `zone_redundant` field will be removed in v4.0 of the provider.

		@@ -65,6 +65,8 @@ The following arguments are supported:

		* `zone_redundant` - (Optional) Whether or not this resource is zone redundant. `sku` needs to be `Premium`. Changing this forces a new resource to be created.

		~> Note: The `zone_redundant` field will be removed in v4.0 of the provider.

azurerm_eventhub_namespace / azurerm_servicebus_namespace - deprecate the zone_redundant field in v4.0 #26611

azurerm_eventhub_namespace / azurerm_servicebus_namespace - deprecate the zone_redundant field in v4.0 #26611

Conversation

WodansSon commented Jul 12, 2024 • edited Loading

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

EventHub Test Results:

ServiceBus Test Results:

Change Log

stephybun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 19, 2024

`azurerm_eventhub_namespace` / `azurerm_servicebus_namespace` - deprecate the `zone_redundant` field in v4.0 #26611

`azurerm_eventhub_namespace` / `azurerm_servicebus_namespace` - deprecate the `zone_redundant` field in v4.0 #26611

WodansSon commented Jul 12, 2024 •

edited

Loading