Adding retry functionality to role assignment #1647
Conversation
There was a problem hiding this comment.
hey @mb290
Thanks for this PR :)
This mostly LGTM - I'm going to push a commit to bump the timeout to 5m (just to account for Azure being slower than intended) - once that's done we should be able to run the tests 👍
I think there's a separate issue which (also) needs to be fixed within the azurerm_azuread_application and azurerm_azuread_service_principal resources which is the root cause of #1635 - as such I'm going to suggest we leave that open.
Thanks!
| @@ -105,7 +107,7 @@ func resourceArmRoleAssignmentCreate(d *schema.ResourceData, meta interface{}) e | |||
| }, | |||
| } | |||
|
|
|||
| _, err := roleAssignmentsClient.Create(ctx, scope, name, properties) | |||
| err := resource.Retry(180*time.Second, retryRoleAssignmentsClient(scope, name, properties, meta)) | |||
There was a problem hiding this comment.
I think we should probably give this a little lee-way (whilst it's 3m in normal circumstances, sometimes things can be slower) - as such I'm going to push a commit to bump this up to 5m (300s)
|
@tombuildsstuff thanks for the feedback. The reason I chose 180 was to mimic the functionality of the az cli which was referenced in https://github.com/Azure/azure-cli/blob/master/src/command_modules/azure-cli-role/azure/cli/command_modules/role/custom.py#L959 |
|
@tombuildsstuff is there a reason you're holding off on the merge? Just wanted to see if there was something you needed changing or done differently by me. |
|
@mb290 there's no issue blocking this from a code perspective - unfortunately our acceptance test account filled up, so we've needed to clear that out before we could run the tests; I've done that this morning and we should be able to merge this once the tests pass; apologies for the delay! |
|
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
locked and limited conversation to collaborators
This is a fix for #1635. It adds the retry functionality to the azurerm_role_assignment resource to take into account the time it takes for Azure to replicate the Principal ID to be usable by roles. Very similar to #1644