New Log Analytics Sku for azurerm_log_analytics_workspace (#1078)

[MattMencel]

Fixes #1078

[MattMencel]

azure-sdk-for-go PR

[tombuildsstuff]

👋 hey @MattMencel

Thanks for this PR :)

As you've seen for us to add support for this this would need to be added to the Azure SDK for Go - which is in turn generated from these Swagger files (in this case the Enum value needs to be added here), which will then trigger a PR to the Go SDK repository, which can then be released and vendored in.

In the interim it might make sense just to hard-code this value for the moment and add a TODO; once this is added to the Go SDK we'll switch it out for the constant/enum value from the SDK, which should allow us to ship this now (and then fix it later) - what do you think?

Taking a look at this PR - if we can revert the CHANGELOG.md file, which we edit manually once PR's are merged (given this repo's moves fairly quickly it helps to avoid merge conflicts) - but this otherwise looks pretty good 👍 If we can hard-code the Constant/Enum value for the moment and add an Acceptance Test (example verifying this new behaviour we should be good to merge :)

Thanks!

[MattMencel]

Hi @tombuildsstuff

I added the new Sku as a PR to azure-rest-api-specs. I also pulled my comments out of the changelog.

I'm not sure how to handle the Acceptance Test. Just add a new...

resource "azurerm_log_analytics_workspace" "test" {
...
}

...at the bottom with the new Sku?

[tombuildsstuff]

hey @MattMencel

Sorry - I'd missed we'd not replied to this!

I added the new Sku as a PR to azure-rest-api-specs. I also pulled my comments out of the changelog.

👍 cool thanks, it looks like that change hasn't made it into v15.2 of the Azure SDK for Go - but I'd assume this is available from v16 which should be out at the end of this/the start of next month; once it is we can upgrade to that and this PR should build & work as expected.

I'm not sure how to handle the Acceptance Test. Just add a new...

Indeed, there's two parts to this: firstly adding a method which returns the Terraform Configuration for the acceptance test - (in this case that'll be provisioning a Log Analytics Workspace with the new SKU) - once that's done we then need to create a Test which calls that configuration (here's an example of that). At which point this test can be run using the following command:

TF_ACC=1 go test ./azurerm -v -timeout 120m -run=SomeTestNameToRun

(We'll also run the tests at this end, so as long as the config is present we should be good :)

Thanks!

[tombuildsstuff]

Tagged upstream-microsoft since this is blocked on a future release of the Azure SDK (presumably v16)

[MattMencel]

I borked my rebase. Sorry....

I'm going to try to clean this up....but if I can't figure it out I'll just delete the PR and try again.

[MattMencel]

Wow I think I maybe got a clean PR again. If you want something else on the tests let me know. I'm not sure I know what I'm doing there. :)

[tombuildsstuff]

@MattMencel thanks for pushing those updates, this now LGTM. Once this is available in the next Azure SDK (and we've upgraded to it) - we'll merge this in :)

Thanks!

[tombuildsstuff]

LGTM - once the next version of the Azure SDK is released & we've upgraded to it we should be able to run this test and merge this 👍

[tombuildsstuff]

Just checked, this value isn't present in v16.2.1 of the Azure SDK for Go - I guess it'll be released with v17

[tombuildsstuff]

hey @MattMencel

Sorry for the delay here, just to let you know that I've taken another look at the SDK and this is now available - as such I've rebased this PR and we can now merge it :)

Thanks!

[katbyte]

LGTM 👍

[tombuildsstuff]

Tests pass:

$ acctests azurerm TestAccAzureRMLogAnalyticsWorkspace_

=== RUN   TestAccAzureRMLogAnalyticsWorkspace_importRequiredOnly
--- PASS: TestAccAzureRMLogAnalyticsWorkspace_importRequiredOnly (136.16s)
=== RUN   TestAccAzureRMLogAnalyticsWorkspace_importRetentionInDaysComplete
--- PASS: TestAccAzureRMLogAnalyticsWorkspace_importRetentionInDaysComplete (74.59s)
=== RUN   TestAccAzureRMLogAnalyticsWorkspace_requiredOnly
--- PASS: TestAccAzureRMLogAnalyticsWorkspace_requiredOnly (129.72s)
=== RUN   TestAccAzureRMLogAnalyticsWorkspace_retentionInDaysComplete
--- PASS: TestAccAzureRMLogAnalyticsWorkspace_retentionInDaysComplete (131.16s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	471.669s

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!