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

Update segment parsing resource id for network connection monitor V1 #7349

Closed

Conversation

neil-yechenwei
Copy link
Contributor

@neil-yechenwei neil-yechenwei commented Jun 17, 2020

This PR is to fix below two issues:

  1. fixes azurerm_network_connection_monitor Gets Recreated Every Run #3909 , as the resource azurerm_network_connection_monitor cannot be deleted anymore for incorrect resource id segment issue, so we have to update segment of resource id here for network connection monitor using state migration.
  2. fixes azurerm_network_connection_monitor failed to create with properties source and destination in API version 2020-03-01 #7309 , Currently, this resource is using API version 2020-05-01 but this version has deprecated properties source and destination and these properties in this resource are required property. So provisioning this resource is always failed now and this resource is already blocked to all users. After confirmed with service team, they said source and destination are only supported by 2019-06-01. And if we continue to use 2020-05-01, we have to deprecate "source" and "destination" and introduce some new required properties but it would cause breaking-change. So we have to rollback to api version 2019-06-01 first.

Note: Please merge this PR first for connection monitor v1, and release a new azurerm provider version, then merge PR 8640 for connection monitor v2.

    --- PASS: TestAccAzureRMNetworkWatcher/ConnectionMonitor (6266.46s)
    --- PASS: TestAccAzureRMNetworkWatcher/ConnectionMonitor/addressBasic (590.12s)
    --- PASS: TestAccAzureRMNetworkWatcher/ConnectionMonitor/addressComplete (581.33s)
    --- PASS: TestAccAzureRMNetworkWatcher/ConnectionMonitor/vmBasic (646.27s)
    --- PASS: TestAccAzureRMNetworkWatcher/ConnectionMonitor/bothDestinationsInvalid (657.97s)
    --- PASS: TestAccAzureRMNetworkWatcher/ConnectionMonitor/requiresImport (582.93s)
    --- PASS: TestAccAzureRMNetworkWatcher/ConnectionMonitor/addressUpdate (606.79s)
    --- PASS: TestAccAzureRMNetworkWatcher/ConnectionMonitor/vmComplete (570.62s)
    --- PASS: TestAccAzureRMNetworkWatcher/ConnectionMonitor/vmUpdate (639.56s)
    --- PASS: TestAccAzureRMNetworkWatcher/ConnectionMonitor/destinationUpdate (851.76s)
    --- PASS: TestAccAzureRMNetworkWatcher/ConnectionMonitor/missingDestinationInvalid (539.11s)

@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Aug 20, 2020

Just for justification. Because the segment of resource id of the 2018-08-01 version terraform initially used is also updated to "connectionMonitors". So I think this issue would also happen on older version. After tested with old version, this issue also happened. So I assume we don't need to be aware of compatible problem for this case. Based on above conclusion, I assume all users are blocked on all released azurerm provider versions now.

@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Aug 20, 2020

@tombuildsstuff, although it's a breaking change, but I assume all users are blocked now by this issue on all released azurerm provider versions. Could we merge it to unblock users first?

@ghost ghost added size/L and removed size/XS labels Aug 21, 2020
@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Aug 22, 2020

@tombuildsstuff , I assume here the "breaking-change" tag also could be removed since I also updated this PR to use MigrateState. And I also tested the scenario you mentioned in PR 8167. After tested, I found there is no difference using this branch as expect when provisioning a resource using older version.

@ghost ghost added size/XXL dependencies and removed size/L labels Sep 3, 2020
@WodansSon WodansSon added this to the v2.27.0 milestone Sep 4, 2020
@jackofallops jackofallops modified the milestones: v2.27.0, v2.28.0 Sep 10, 2020
@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Sep 11, 2020

Manual test for this PR:

  1. Run tf apply to create this resource with provider.azurerm v2.0.0 (I have to manually update the name segment to "NetworkConnectionMonitors" since the name segment in all api versions has been updated to "connectionMonitors" such as api version 2018-08-01. So we cannot get old value anymore.)
azurerm_network_connection_monitor.test: Creation complete after 9s [id=/subscriptions/xx-xx-x-xx/resourceGroups/acctestRG-watcher-test01/providers/Microsoft.Network/networkWatchers/acctnw-test01/NetworkConnectionMonitors/acctestcm-test01]
  1. Switch provider.azurerm from v2.0.0 to this branch
  2. Run tf plan
azurerm_network_connection_monitor.test: Refreshing state... [id=/subscriptions/xx-x-xx-xx/resourceGroups/acctestRG-watcher-test01/providers/Microsoft.Network/networkWatchers/acctnw-test01/connectionMonitors/acctestcm-test01]
  1. Add new tag to tfconfig.
  2. Run tf apply
azurerm_network_connection_monitor.test: Modifications complete after 9s [id=/subscriptions/x-xx-x-x-x/resourceGroups/acctestRG-watcher-test01/providers/Microsoft.Network/networkWatchers/acctnw-test01/connectionMonitors/acctestcm-test01]
  1. Run tf plan
azurerm_network_connection_monitor.test: Refreshing state... [id=/subscriptions/x-xx-x-x-xx/resourceGroups/acctestRG-watcher-test01/providers/Microsoft.Network/networkWatchers/acctnw-test01/connectionMonitors/acctestcm-test01]
  1. Run tf destroy
azurerm_network_connection_monitor.test: Destroying... [id=/subscriptions/xx-x-x-xxx/resourceGroups/acctestRG-watcher-test01/providers/Microsoft.Network/networkWatchers/acctnw-test01/connectionMonitors/acctestcm-test01]
azurerm_network_connection_monitor.test: Destruction complete after 3s

@jackofallops jackofallops modified the milestones: v2.28.0, v2.29.0 Sep 17, 2020
@tombuildsstuff tombuildsstuff modified the milestones: v2.29.0, v2.30.0 Sep 24, 2020
@katbyte katbyte self-assigned this Sep 24, 2020
@neil-yechenwei neil-yechenwei changed the title Update segment parsing resource id for network connection monitor Update segment parsing resource id for network connection monitor V1 Sep 26, 2020
@tombuildsstuff tombuildsstuff modified the milestones: v2.30.0, v2.31.0 Oct 1, 2020
@tombuildsstuff tombuildsstuff modified the milestones: v2.31.0, v2.32.0 Oct 8, 2020
@jackofallops jackofallops modified the milestones: v2.32.0, v2.33.0 Oct 15, 2020
@tombuildsstuff tombuildsstuff modified the milestones: v2.33.0, v2.34.0 Oct 22, 2020
@jackofallops jackofallops modified the milestones: v2.34.0, v2.35.0 Oct 29, 2020
@neil-yechenwei
Copy link
Contributor Author

We don't need to support connection monitor v1 anymore since service team deprecated it.

@ghost
Copy link

ghost commented Nov 5, 2020

This has been released in version 2.35.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.35.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Dec 5, 2020

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!

@ghost ghost locked as resolved and limited conversation to collaborators Dec 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.