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

azurerm_storage_account - support for the multichannel_enabled property #17999

Merged
merged 1 commit into from
Sep 28, 2022

Conversation

alexwilcox9
Copy link
Contributor

@alexwilcox9 alexwilcox9 commented Aug 15, 2022

Hi,

I'd like to make use of multichannel SMB shares so thought I'd have a go at adding this functionality to the storage account resource.

#13715

==> Checking that code complies with gofmt requirements... ==> Checking that Custom Timeouts are used... ==> Checking that acceptance test packages are used... TF_ACC=1 go test -v ./internal/services/storage -run=TestAccStorageAccount_smbMultichannel -timeout 60m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc" === RUN TestAccStorageAccount_smbMultichannel === PAUSE TestAccStorageAccount_smbMultichannel === CONT TestAccStorageAccount_smbMultichannel --- PASS: TestAccStorageAccount_smbMultichannel (348.15s) PASS ok github.com/hashicorp/terraform-provider-azurerm/internal/services/storage 348.644s

@alexwilcox9 alexwilcox9 marked this pull request as ready for review August 19, 2022 23:41
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks @alexwilcox9 - seems like we need to enable a feature on the storage account?

------- Stdout: -------
=== RUN   TestAccAzureRMStorageAccount_shareProperties
=== PAUSE TestAccAzureRMStorageAccount_shareProperties
=== CONT  TestAccAzureRMStorageAccount_shareProperties
    testcase.go:110: Step 1/2 error: Error running apply: exit status 1
        
        Error: updating Azure Storage Account `share_properties` "unlikely23exst2acct1rvfl": storage.FileServicesClient#SetServiceProperties: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="FeatureNotSupportedOnStorageAccount" Message="Feature is not supported on this storage account.\nRequestId:b45ca9e4-501a-001b-7d40-c17878000000\nTime:2022-09-05T15:59:49.6466540Z"
        
          with azurerm_storage_account.test,
          on terraform_plugin_test.tf line 11, in resource "azurerm_storage_account" "test":
          11: resource "azurerm_storage_account" "test" {
        
--- FAIL: TestAccAzureRMStorageAccount_shareProperties (96.13s)
FAIL
  • could you document how to do this in the docs?

@alexwilcox9
Copy link
Contributor Author

Hi @katbyte
This feature is only for premium storage accounts, I've added that to the documentation now.

What's confusing me now is why it complained for that test in particular as multichannel isn't enabled in that test. From a quick test it seems if you send a storage.Multichannel block on a Standard storage account it complains. Even if enabled is set to False
To try and address this I've changed the code to only send the block if it's explicitly enabled however this has caused a new problem where multichannel can't be disabled if it's already enabled (as we only send the block if it's enabled)

Have you got any tips on how to get around this?

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

we'll need to handle the non premium account situation, so likely what needs to happen is the correct handling if the API return value in that situation

@alexwilcox9
Copy link
Contributor Author

Hi @katbyte
I think I've got it this time, both tests are passing for me. Could you please also test and let me know if the approach I've taken to deal with Standard accounts is appropriate?
Thanks!

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

THanks @alexwilcox9! LGTM now 🍄

@katbyte katbyte changed the title Storage Account add multichannel support azurerm_storage_account - support for the multichannel_enabled property Sep 15, 2022
@katbyte katbyte merged commit ea1087e into hashicorp:main Sep 28, 2022
katbyte added a commit that referenced this pull request Sep 28, 2022
@github-actions github-actions bot added this to the v3.25.0 milestone Sep 28, 2022
@github-actions
Copy link

This functionality has been released in v3.25.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants