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

automanage: refactoring to use hashicorp/go-azure-sdk #25293

Merged
merged 9 commits into from
Mar 19, 2024

Conversation

tombuildsstuff
Copy link
Contributor

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 "+1" or "me too" comments, they generate extra noise for PR followers and do not help prioritize for review

Description

This PR updates the automanage service package to use hashicorp/go-azure-sdk rather than tombuildsstuff/kermit.

During this migration I've discovered that the AzureStackHCI Cluster resource is unexpectedly managing the association between the AzureStackHCI Cluster and the associated AutoManage Configuration, rather than exposing this association as separate resource (as this should be) - as such I've added TODOs for us to fix this in 4.0.

Unfortunately it appears that despite the Service Team committing that there would be no breaking changes to the payload (specifically that any changes would be additive) - there's been a breaking change to the payload since the original pull request was merged - given that the TestAccAutoManageConfigurationProfile_complete and TestAccAutoManageConfigurationProfile_update tests haven't passed for some time.

As such - @liuwuliuyun / @mybayern1974 can you please reach out to the Service Team to find out what's happened here?

@liuwuliuyun
Copy link
Contributor

liuwuliuyun commented Mar 19, 2024

Hi @tombuildsstuff the tests failure are probably due to the lack of permission from test subscription. You could enable this feature flag by following steps in #22799.

Local test on the original code:

GOROOT=C:\Program Files\Go #gosetup
GOPATH=C:\Users\yunliu1\go #gosetup
"C:\Program Files\Go\bin\go.exe" test -c -o C:\Users\yunliu1\AppData\Local\JetBrains\GoLand2023.3\tmp\GoLand\___TestAccAutoManageConfigurationProfile_complete_in_github_com_hashicorp_terraform_provider_azurerm_internal_services_automanage.test.exe github.com/hashicorp/terraform-provider-azurerm/internal/services/automanage #gosetup
"C:\Program Files\Go\bin\go.exe" tool test2json -t C:\Users\yunliu1\AppData\Local\JetBrains\GoLand2023.3\tmp\GoLand\___TestAccAutoManageConfigurationProfile_complete_in_github_com_hashicorp_terraform_provider_azurerm_internal_services_automanage.test.exe -test.v -test.paniconexit0 -test.run ^\QTestAccAutoManageConfigurationProfile_complete\E$ #gosetup
=== RUN   TestAccAutoManageConfigurationProfile_complete
=== PAUSE TestAccAutoManageConfigurationProfile_complete
=== CONT  TestAccAutoManageConfigurationProfile_complete
--- PASS: TestAccAutoManageConfigurationProfile_complete (138.23s)
PASS


Process finished with the exit code 0

@tombuildsstuff
Copy link
Contributor Author

@liuwuliuyun in our case those features aren't registered, so that makes sense - however the API returns this rather cryptic when the feature isn't enabled:

 Error: creating Configuration Profile (Subscription: "XXX"
         Resource Group Name: "acctest-rg-240319093707573983"
         Configuration Profile Name: "acctest-amcp-240319093707573983"): unexpected status 400 with error: > InvalidConfigurationName: The following invalid configuration names were found:  <Alerts/AutomanageStatusChanges/Enable>

Whereas previously (from #22799) it returned the much more helpful:

Error: creating Automanage Configuration: (Configuration Profile Name "Example" / Resource Group "Example"): automanage.ConfigurationProfilesClient#CreateOrUpdate: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="InvalidConfigurationValue" Message="The feature flag <Microsoft.Automanage/AutomanageAlertsEnabled> must enabled on subscription: <subscription>, in order to use properties: <Alerts/AutomanageStatusChanges/Enable>"

As such given the documentation doesn't show this - would you mind adding documentation to highlight that this is needed?

Thanks!

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

There are still MaxItems attributes in the schema for the state migration but otherwise LGTM 👍

@tombuildsstuff tombuildsstuff force-pushed the refactor/automanage-to-go-azure-sdk branch from d780ae3 to bb0ea73 Compare March 19, 2024 17:24
@tombuildsstuff tombuildsstuff force-pushed the refactor/automanage-to-go-azure-sdk branch from bb0ea73 to 3e77d2f Compare March 19, 2024 17:43
@tombuildsstuff tombuildsstuff merged commit 9fd2258 into main Mar 19, 2024
29 checks passed
@tombuildsstuff tombuildsstuff deleted the refactor/automanage-to-go-azure-sdk branch March 19, 2024 17:58
tombuildsstuff added a commit that referenced this pull request Mar 19, 2024
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 Apr 19, 2024
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.

3 participants