feat(w-m): fast-track azure vm deletion when resources are cascade-deleted - #8790
Conversation
…leted Since Azure supports fast resource removal with the VM (`deleteOption: 'Delete'`) and our ARM templates are enforcing this, we could save some time by not trying to deprovision each resource manually as before. We also cannot rely/control what template is used, so we have to check the actual VM model and NIC response from Azure when worker registers. Only after checking that and making sure all resources are "cascadable" we could short-circuit deprovision loop. Deprovision now becomes: 1. delete VM. 2. if cascade exists, mark all done. This would skip NIC/IP/Disks steps Additionally we log new metric and log type to see how it performs. This should reduce total number of STOPPING workers that are usuallly blocking worker-scanner-azure.
| if (!vmDeleted || worker.providerData.vm.id) { | ||
| return; | ||
| } | ||
| const nicDeleted = await this.deprovisionResource({ |
There was a problem hiding this comment.
this part stays as is, it is just moved under if .. else { } block
petemoore
left a comment
There was a problem hiding this comment.
Thanks for adding all the tests! This looks pretty solid, the only thing I'm not sure about is how clean up works if deletes fail...
| const teardownMode = useFastPath ? 'fast' : 'slow'; | ||
|
|
||
| if (useFastPath) { | ||
| // Mark the cascaded resource records deleted as we trust cascade delete option |
There was a problem hiding this comment.
thanks yarik! what happens if the deletes fail? is this a fire-and-forget or is there something that would later drop those resources if the deletes fail?
There was a problem hiding this comment.
so we do this check specifically to make sure that resources got "deleteOption: Delete"
And if all do, then it's on the Azure's resource manager to clean them up
And indeed, once we confirmed that vm is deleted, it might be the case that resources are still "present" on azure side, but I don't think we should worry about this (not at least here)
We had a long standing - #8113 to cover that gap
petemoore
left a comment
There was a problem hiding this comment.
Thanks Yarik. This looks pretty robust, thanks for the extra tests, great job. :-)
Since Azure supports fast resource removal with the VM (
deleteOption: 'Delete') and our ARM templates are enforcing this, we could save some time by not trying to deprovision each resource manually as before.We also cannot rely/control what template is used, so we have to check the actual VM model and NIC response from Azure when worker registers. Only after checking that and making sure all resources are "cascadable" we could short-circuit deprovision loop.
Deprovision now becomes: 1. delete VM. 2. if cascade exists, mark all done.
This would skip NIC/IP/Disks steps
Additionally we log new metric and log type to see how it performs.
This should reduce total number of STOPPING workers that are usuallly blocking worker-scanner-azure.
Fixes #8161 - instead of excluding stopping capacity from estimator, we just do this