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

for delete type LRO, the poller does not really check lro result #2696

Open
ziyeqf opened this issue Jun 27, 2023 · 7 comments
Open

for delete type LRO, the poller does not really check lro result #2696

ziyeqf opened this issue Jun 27, 2023 · 7 comments

Comments

@ziyeqf
Copy link
Contributor

ziyeqf commented Jun 27, 2023

For some resources, the delete lro may not finish even if GET on the resource is returned with 404. Can we take use of the LRO on deletion too?

For example:
1.azurerm_backup_protected_vm: Though it's not using new base layer poller, it shows even GET on the resource is returned 404, the delete call may not finish.
https://github.com/hashicorp/terraform-provider-azurerm/blob/1ed1fed382151d7e7ea559ca6d8deb4a0ebe5c02/internal/services/recoveryservices/backup_protected_vm_resource.go#L333

  1. FrontendsInterface of servicenetworking Swagger (this RP is not included in pandora config now, I just generated sdk code to develop locally)

poller code: https://github.com/hashicorp/terraform-provider-azurerm/blob/99e4874b4f0c8b91101ec2ecb30d2e97be5c1a8b/vendor/github.com/hashicorp/go-azure-sdk/sdk/client/resourcemanager/poller_delete.go#L27

@tombuildsstuff
Copy link
Contributor

@ziyeqf would you be able to clarify this/provide Requests/Responses so that we can determine the issue for this one?

@ziyeqf
Copy link
Contributor Author

ziyeqf commented Jul 27, 2023

for the FrontendsInterface of servicenetworking

DELETE

DELETE https://management.azure.com/subscriptions/{removed}/resourceGroups/acct-sn-230727133524047550/providers/Microsoft.ServiceNetworking/trafficControllers/acct-230727133524047550/frontends/acct-frnt-230727133524047550?api-version=2023-05-01-preview HTTP/2.0
content-type: application/json
user-agent: HashiCorp/go-azure-sdk (Go-http-Client/1.1 frontendsinterface/2023-05-01-preview) HashiCorp Terraform/1.5.3 (+https://www.terraform.io) Terraform Plugin SDK/2.10.1 terraform-provider-azurerm/dev pid-222c6c49-1b0a-5959-a213-6608f9eb8820
authorization: {removed} 
x-ms-correlation-request-id: {removed} 
accept-encoding: gzip
content-length: 0



HTTP/2.0 202 
cache-control: no-cache
pragma: no-cache
content-length: 4
content-type: application/json; charset=utf-8
expires: -1
location: https://management.azure.com/subscriptions/{removed}/providers/Microsoft.ServiceNetworking/locations/northeurope/operationResults/xxxxxxxx-0000-0000-0000-000000000000?api-version=2023-05-01-preview
retry-after: 10
x-ms-request-id: 00000000-0000-0000-0000-000000000000
x-ms-request-id: 00000000-0000-0000-0000-000000000000
x-ms-correlation-request-id: 00000000-0000-0000-0000-000000000000
azure-asyncoperation: https://management.azure.com/subscriptions/{removed}/providers/Microsoft.ServiceNetworking/locations/northeurope/operations/xxxxxxxx-0000-0000-0000-000000000000?api-version=2023-05-01-preview
x-ms-ratelimit-remaining-subscription-deletes: 14999
server: Microsoft-HTTPAPI/2.0
x-ms-routing-request-id: JAPANEAST:20230727T062611Z:15cff89e-aaa3-407e-8f78-c6c5965bd02c
strict-transport-security: max-age=31536000; includeSubDomains
x-content-type-options: nosniff
date: Thu, 27 Jul 2023 06:26:11 GMT

null

operation

GET https://management.azure.com/subscriptions/{removed}/providers/Microsoft.ServiceNetworking/locations/northeurope/operations/615460de-bb8c-422a-a126-d14de7360db7?api-version=2023-05-01-preview HTTP/2.0
content-type: application/json
user-agent: HashiCorp/go-azure-sdk (Go-http-Client/1.1 frontendsinterface/2023-05-01-preview) HashiCorp Terraform/1.5.3 (+https://www.terraform.io) Terraform Plugin SDK/2.10.1 terraform-provider-azurerm/dev pid-222c6c49-1b0a-5959-a213-6608f9eb8820
authorization: {removed} 
x-ms-correlation-request-id: e9dfa725-e284-8ccf-5fd7-60ffc648750c
accept-encoding: gzip
content-length: 0



HTTP/2.0 200 
cache-control: no-cache
pragma: no-cache
content-type: application/json; charset=utf-8
expires: -1
vary: Accept-Encoding
x-ms-request-id: 00000000-0000-0000-0000-000000000000
x-ms-request-id: 00000000-0000-0000-0000-000000000000
x-ms-correlation-request-id: 00000000-0000-0000-0000-000000000000
x-ms-ratelimit-remaining-subscription-reads: 11999
server: Microsoft-HTTPAPI/2.0
x-ms-routing-request-id: JAPANEAST:20230727T062635Z:5f21159a-3d8e-4e73-b831-ac8f42903955
strict-transport-security: max-age=31536000; includeSubDomains
x-content-type-options: nosniff
date: Thu, 27 Jul 2023 06:26:34 GMT
content-length: 29

{
  "status": "Succeeded"
}

operation result

GET https://management.azure.com/subscriptions/{removed}/providers/Microsoft.ServiceNetworking/locations/northeurope/operationResults/612b326e-405e-40ae-bdac-550e7c4d14b6?api-version=2023-05-01-preview HTTP/2.0
user-agent: HashiCorp/go-azure-sdk (Go-http-Client/1.1 frontendsinterface/2023-05-01-preview) HashiCorp Terraform/1.5.3 (+https://www.terraform.io) Terraform Plugin SDK/2.10.1 terraform-provider-azurerm/dev pid-222c6c49-1b0a-5959-a213-6608f9eb8820
authorization: {removed} 
x-ms-correlation-request-id: 00000000-0000-0000-0000-000000000000
content-type: application/json
accept-encoding: gzip
content-length: 0



HTTP/2.0 200 
cache-control: no-cache
pragma: no-cache
content-type: application/json; charset=utf-8
expires: -1
vary: Accept-Encoding
x-ms-request-id: 00000000-0000-0000-0000-000000000000
x-ms-request-id: 00000000-0000-0000-0000-000000000000
x-ms-correlation-request-id: 00000000-0000-0000-0000-000000000000
x-ms-ratelimit-remaining-subscription-reads: 11985
server: Microsoft-HTTPAPI/2.0
x-ms-routing-request-id: JAPANEAST:20230727T063720Z:ecfed528-b07a-49de-95fd-3652e8bce3ef
strict-transport-security: max-age=31536000; includeSubDomains
x-content-type-options: nosniff
date: Thu, 27 Jul 2023 06:37:19 GMT
content-length: 4

null

@tombuildsstuff
Copy link
Contributor

@ziyeqf I think this would be solved by making those operations LROs, in which case this'd be a duplicate of #2345?

@ziyeqf
Copy link
Contributor Author

ziyeqf commented Aug 14, 2023

Hey @tombuildsstuff, I may used wrong examples. For the azurerm_backup_protected_vm one, it does belong to the issue #2345, while the another one, servicenetworking one, there is already a LRO mark in the swagger, while the poller only check if GET on the resource is 404 but ignoring the operation status. https://github.com/Azure/azure-rest-api-specs/blob/main/specification/servicenetworking/resource-manager/Microsoft.ServiceNetworking/preview/2023-05-01-preview/TrafficController.json#L1039

@ziyeqf
Copy link
Contributor Author

ziyeqf commented Sep 28, 2023

Hey @tombuildsstuff, to bubble this up. these operations have been marked as LRO, while the poller only checks if GET on the resource return 404, but not check the operation results.

swagger: https://github.com/Azure/azure-rest-api-specs/blob/6a2e3c7617314fe4ea7e5706da5437214e8a602b/specification/servicenetworking/resource-manager/Microsoft.ServiceNetworking/preview/2023-05-01-preview/TrafficController.json#L1039

@tombuildsstuff
Copy link
Contributor

Checking through here, I suspect this'll be fixed by/as a part of hashicorp/go-azure-sdk#307?

@ziyeqf
Copy link
Contributor Author

ziyeqf commented Jan 26, 2024

Checking through here, I suspect this'll be fixed by/as a part of hashicorp/go-azure-sdk#307?

gave it a try by running TestAccApplicationLoadBalancerFrontend_basic and capture the request, the url in azure-asyncoperation header of delete lro has not been called.
But it does check the provisioningState by calling GET on the resource.

For this issue, we might need to check the LRO result and then checking the provisiongState.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants