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

Upgrade Purview from 2020-12-01-preview to stable version 2021-07-01 #13785

Merged
merged 1 commit into from
Nov 2, 2021

Conversation

myc2h6o
Copy link
Contributor

@myc2h6o myc2h6o commented Oct 19, 2021

  • Update the import path for Azure Purview to use the latest stable version
  • Update name of the constants which are changed in the stable version
  • Update go mods in the vendor folder

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for this @myc2h6o

@tombuildsstuff tombuildsstuff added this to the v2.82.0 milestone Oct 20, 2021
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.

Looks like we have some test failures:

------- Stdout: -------
=== RUN   TestAccPurviewAccount_basic
=== PAUSE TestAccPurviewAccount_basic
=== CONT  TestAccPurviewAccount_basic
    testcase.go:108: Step 1/2 error: After applying this test step, the plan was not empty.
        stdout:
        
        
        Terraform used the selected providers to generate the following execution
        plan. Resource actions are indicated with the following symbols:
          ~ update in-place
        
        Terraform will perform the following actions:
        
          # azurerm_purview_account.test will be updated in-place
          ~ resource "azurerm_purview_account" "test" {
                id                                               = "/subscriptions/*******/resourceGroups/acctestRG-purview-211020215537433833/providers/Microsoft.Purview/accounts/acctestsw211020215537433833"
                name                                             = "acctestsw211020215537433833"
              ~ sku_name                                         = "Standard_1" -> "Standard_4"
                # (9 unchanged attributes hidden)
            }
        
        Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccPurviewAccount_basic (445.63s)
FAIL

@myc2h6o
Copy link
Contributor Author

myc2h6o commented Oct 21, 2021

The tests for Purview are failing due to below change in sdk, which changes the sku property to user-specified to read-only.
Azure/azure-rest-api-specs@b75f081
This actually happens in both preview and stable version, thus the current main branch already has this issue.
I'm working on another change to fix the sku property first, will validate again on this change after that one is done

@katbyte katbyte modified the milestones: v2.82.0, v2.83.0 Oct 21, 2021
@myc2h6o
Copy link
Contributor Author

myc2h6o commented Oct 22, 2021

Fix for the acc tests: #13839

@katbyte
Copy link
Collaborator

katbyte commented Oct 25, 2021

still failing as of today:

------- Stdout: -------
=== RUN   TestAccPurviewAccount_basic
=== PAUSE TestAccPurviewAccount_basic
=== CONT  TestAccPurviewAccount_basic
    testcase.go:108: Step 1/2 error: After applying this test step, the plan was not empty.
        stdout:
        
        
        Terraform used the selected providers to generate the following execution
        plan. Resource actions are indicated with the following symbols:
          ~ update in-place
        
        Terraform will perform the following actions:
        
          # azurerm_purview_account.test will be updated in-place
          ~ resource "azurerm_purview_account" "test" {
                id                                               = "/subscriptions/*******/resourceGroups/acctestRG-purview-211025171004515337/providers/Microsoft.Purview/accounts/acctestsw211025171004515337"
                name                                             = "acctestsw211025171004515337"
              ~ sku_name                                         = "Standard_1" -> "Standard_4"
                # (9 unchanged attributes hidden)
            }
        
        Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccPurviewAccount_basic (445.38s)
FAIL

@hashicorp hashicorp deleted a comment from github-actions bot Oct 25, 2021
@myc2h6o
Copy link
Contributor Author

myc2h6o commented Oct 26, 2021

@katbyte There is an issue opened by @tombuildsstuff on Azure side Azure/azure-rest-api-specs#15675, discussing the breaking change of the sku property.
I discussed with @shaileshsk94 internally about the server status. Purview accounts with sku Standard_4 has been updated to Standard_1, and Standard_16 has been updated to Standard_2 at server end.
Furthermore, this property no longer supports an update through api. Instead, it can only be updated by creating an support ticket at Azure, which means it is supposed be modified outside terraform, so I'm thinking of deprecating it, I've raised a new pr for this purpose, please let me know if this approach looks good or not: #13897

@katbyte katbyte modified the milestones: v2.83.0, v2.84.0 Oct 29, 2021
@myc2h6o
Copy link
Contributor Author

myc2h6o commented Nov 1, 2021

Have rebased to latest main. Test results after the unit test fix in #13817 with the package upgrade:
$ TF_ACC=1 go test -v ./internal/services/purview -run=Test -timeout 60m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN TestAccPurviewAccount_basic
=== PAUSE TestAccPurviewAccount_basic
=== RUN TestAccPurviewAccount_requiresImport
=== PAUSE TestAccPurviewAccount_requiresImport
=== CONT TestAccPurviewAccount_basic
=== CONT TestAccPurviewAccount_requiresImport

--- PASS: TestAccPurviewAccount_requiresImport (672.66s)
--- PASS: TestAccPurviewAccount_basic (676.43s)
PASS
ok github.com/hashicorp/terraform-provider-azurerm/internal/services/purview 677.079s

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 @myc2h6o - LGTM 👍

@katbyte katbyte merged commit 9f46402 into hashicorp:main Nov 2, 2021
katbyte added a commit that referenced this pull request Nov 2, 2021
@myc2h6o myc2h6o deleted the purview_to_stable branch November 2, 2021 04:14
katbyte added a commit that referenced this pull request Nov 5, 2021
@github-actions
Copy link

github-actions bot commented Nov 5, 2021

This functionality has been released in v2.84.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

github-actions bot commented Dec 6, 2021

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 Dec 6, 2021
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