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.

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

enable_partitioning not supported servicebus Premium Sku. #1391

Merged
merged 3 commits into from
Jul 2, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
7 changes: 3 additions & 4 deletions azurerm/resource_arm_servicebus_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,9 @@ func resourceArmServiceBusQueueCreateUpdate(d *schema.ResourceData, meta interfa
return nsErr
}

// Enforce Premium namespace to have partitioning enabled in Terraform. It is always enabled in Azure for
// Premium SKU.
if namespace.Sku.Name == servicebus.Premium && !d.Get("enable_partitioning").(bool) {
return fmt.Errorf("ServiceBus Queue (%s) must have Partitioning enabled for Premium SKU", name)
// In a Premium tier namespace, partitioning entities is not supported.
if namespace.Sku.Name == servicebus.Premium && d.Get("enable_partitioning").(bool) {
return fmt.Errorf("ServiceBus Queue (%s) does not support Partitioning enabled for Premium SKU", name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we are changing the behavior can we update the documentation too with this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

gonna push a commit to remove this, since existing Namespaces should continue working

}

// Enforce Premium namespace to have Express Entities disabled in Terraform since they are not supported for
Expand Down
4 changes: 2 additions & 2 deletions azurerm/resource_arm_servicebus_queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestAccAzureRMServiceBusQueue_defaultEnablePartitioningPremium(t *testing.T
Config: config,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMServiceBusQueueExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "enable_partitioning", "true"),
resource.TestCheckResourceAttr(resourceName, "enable_partitioning", "false"),
resource.TestCheckResourceAttr(resourceName, "enable_express", "false"),
),
},
Expand Down Expand Up @@ -331,7 +331,7 @@ resource "azurerm_servicebus_queue" "test" {
name = "acctestservicebusqueue-%d"
resource_group_name = "${azurerm_resource_group.test.name}"
namespace_name = "${azurerm_servicebus_namespace.test.name}"
enable_partitioning = true
enable_partitioning = false
enable_express = false
}
`, rInt, location, rInt, rInt)
Expand Down
13 changes: 13 additions & 0 deletions azurerm/resource_arm_servicebus_topic.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,19 @@ func resourceArmServiceBusTopicCreate(d *schema.ResourceData, meta interface{})
parameters.SBTopicProperties.DuplicateDetectionHistoryTimeWindow = utils.String(duplicateWindow)
}

// We need to retrieve the namespace because Premium namespace works differently from Basic and Standard,
// so it needs different rules applied to it.
namespacesClient := meta.(*ArmClient).serviceBusNamespacesClient
namespace, nsErr := namespacesClient.Get(ctx, resourceGroup, namespaceName)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Super minor, but can we make nsErr just err to keep it consistent in the codebase?

if nsErr != nil {
return nsErr
}

// In a Premium tier namespace, partitioning entities is not supported.
if namespace.Sku.Name == servicebus.Premium && d.Get("enable_partitioning").(bool) {
return fmt.Errorf("ServiceBus Queue (%s) does not support Partitioning enabled for Premium SKU", name)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

gonna push a commit to remove this, since existing Namespaces should continue working


_, err := client.CreateOrUpdate(ctx, resourceGroup, namespaceName, name, parameters)
if err != nil {
return err
Expand Down
6 changes: 3 additions & 3 deletions azurerm/resource_arm_servicebus_topic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func TestAccAzureRMServiceBusTopic_enablePartitioningPremium(t *testing.T) {
{
Config: postConfig,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "enable_partitioning", "true"),
resource.TestCheckResourceAttr(resourceName, "enable_partitioning", "false"),

This comment was marked as outdated.

Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't possible due to the API behaviour change/limitation

resource.TestCheckResourceAttr(resourceName, "max_size_in_megabytes", "81920"),
),
},
Expand Down Expand Up @@ -349,7 +349,7 @@ resource "azurerm_servicebus_topic" "test" {
name = "acctestservicebustopic-%d"
namespace_name = "${azurerm_servicebus_namespace.test.name}"
resource_group_name = "${azurerm_resource_group.test.name}"
enable_partitioning = true
enable_partitioning = false
}
`, rInt, location, rInt, rInt)
}
Expand Down Expand Up @@ -397,7 +397,7 @@ resource "azurerm_servicebus_topic" "test" {
name = "acctestservicebustopic-%d"
namespace_name = "${azurerm_servicebus_namespace.test.name}"
resource_group_name = "${azurerm_resource_group.test.name}"
enable_partitioning = true
enable_partitioning = false
max_size_in_megabytes = 81920
}
`, rInt, location, rInt, rInt)
Expand Down