azurerm_nginx_certificate: support update key_virtual_path, certificate_virtual_path and key_vault_secret_id fields
#22100
Conversation
There was a problem hiding this comment.
Hi @wuxu92
This change looks mostly OK, but could you update the docs to reflect the removal of ForceNew from the affected properties?
Thanks!
| @@ -156,7 +201,7 @@ func (m CertificateResource) Read() sdk.ResourceFunc { | |||
|
|
|||
| func (m CertificateResource) Delete() sdk.ResourceFunc { | |||
| return sdk.ResourceFunc{ | |||
| Timeout: 10 * time.Minute, | |||
| Timeout: 30 * time.Minute, | |||
There was a problem hiding this comment.
No need to bump this timeout to 30 minutes. We have found 10minutes to suffice and typical range is 1-3 minutes.
There was a problem hiding this comment.
there is a context deadline exceeded log in the origin issue, so I increased the delete timeout here
Error: deleting Certificate (Subscription: "ee920d60-90f3-4a92-b5e7-bb284c3a6ce2"
Resource Group Name: "tf-22bc9-e2e-tf-xzpd4e"
Nginx Deployment Name: "tf-22bc9-e2e-tf-xzpd4e"
Certificate Name: "tf-22bc9-e2e-tf-xzpd4e"): polling after CertificatesDelete: context deadline exceeded
There was a problem hiding this comment.
Should have included additionally that the timeout comes from the delete. The delete should never work as you cannot delete an in use certificate in this service. In the example in the bug my certificate was in use and thus not deletable.
There was a problem hiding this comment.
The delete API should return an error instead of timeout in this case. could you please have a check on this?
There was a problem hiding this comment.
Made an inquiry to the team on expected behavior and potentially applying a fix.
|
@jackofallops document updated |
|
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. |
fixes: #22092 . these fieds should support update.