Reduce unnecessary delete calls to ARM for storage accounts

eng/common/scripts/Helpers/Resource-Helpers.ps1

@@ -232,104 +232,10 @@ function Remove-WormStorageAccounts() {
   foreach ($group in $groups) {
     Write-Host "========================================="
     $accounts = Get-AzStorageAccount -ResourceGroupName $group.ResourceGroupName
-    if ($accounts) {
-      foreach ($account in $accounts) {
-        if ($WhatIfPreference) {
-          Write-Host "What if: Removing $($account.StorageAccountName) in $($account.ResourceGroupName)"
-        }
-        else {
-          Write-Host "Removing $($account.StorageAccountName) in $($account.ResourceGroupName)"
-        }
-
-        $hasContainers = ($account.Kind -ne "FileStorage")
-
-        # If it doesn't have containers then we can skip the explicit clean-up of this storage account
-        if (!$hasContainers) { continue }
-
-        $ctx = New-AzStorageContext -StorageAccountName $account.StorageAccountName
-        $containers = $ctx | Get-AzStorageContainer
-        $blobs = $containers | Get-AzStorageBlob
-
-        $immutableBlobs = $containers `
-        | Where-Object { $_.BlobContainerProperties.HasImmutableStorageWithVersioning } `
-        | Get-AzStorageBlob
-        try {
-          foreach ($blob in $immutableBlobs) {
-            # We can't edit blobs with customer encryption without using that key
-            # so just try to delete them fully instead. It is unlikely they
-            # will also have a legal hold enabled.
-            if (($blob | Get-Member 'ListBlobProperties') `
-                -and $blob.ListBlobProperties.Properties.CustomerProvidedKeySha256) {
-              Write-Host "Removing customer encrypted blob: $($blob.Name), account: $($account.StorageAccountName), group: $($group.ResourceGroupName)"
-              $blob | Remove-AzStorageBlob -Force
-              continue
-            }
-
-            if (!($blob | Get-Member 'BlobProperties')) {
-              continue
-            }
-
-            if ($blob.BlobProperties.LeaseState -eq 'Leased') {
-              Write-Host "Breaking blob lease: $($blob.Name), account: $($account.StorageAccountName), group: $($group.ResourceGroupName)"
-              $blob.ICloudBlob.BreakLease()
-            }
+    if (!$accounts) { break }
 
-            if ($blob.BlobProperties.HasLegalHold) {
-              Write-Host "Removing legal hold - blob: $($blob.Name), account: $($account.StorageAccountName), group: $($group.ResourceGroupName)"
-              $blob | Set-AzStorageBlobLegalHold -DisableLegalHold | Out-Null
-            }
-          }
-        } catch {
-          Write-Warning "Ensure user has 'Storage Blob Data Owner' RBAC permission on subscription or resource group"
-          Write-Error $_
-          throw
-        }
-        # Sometimes we get a 404 blob not found but can still delete containers,
-        # and sometimes we must delete the blob if there's a legal hold.
-        # Try to remove the blob, but keep running regardless.
-        $succeeded = $false
-        for ($attempt = 0; $attempt -lt 2; $attempt++) {
-          if ($succeeded) {
-            break
-          }
-
-          try {
-            foreach ($blob in $blobs) {
-              if ($blob.BlobProperties.ImmutabilityPolicy.PolicyMode) {
-                Write-Host "Removing immutability policy - blob: $($blob.Name), account: $($ctx.StorageAccountName), group: $($group.ResourceGroupName)"
-                $null = $blob | Remove-AzStorageBlobImmutabilityPolicy
-              }
-            }
-          }
-          catch {}
-
-          try {
-            foreach ($blob in $blobs) {
-              $blob | Remove-AzStorageBlob -Force
-            }
-            $succeeded = $true
-          }
-          catch {
-            Write-Warning "Failed to remove blobs - account: $($ctx.StorageAccountName), group: $($group.ResourceGroupName)"
-            Write-Warning $_
-          }
-        }
-
-        try {
-          # Use AzRm cmdlet as deletion will only work through ARM with the immutability policies defined on the blobs
-          $containers | ForEach-Object { Remove-AzRmStorageContainer -Name $_.Name -StorageAccountName $ctx.StorageAccountName -ResourceGroupName $group.ResourceGroupName -Force }
-        } catch {
-          Write-Warning "Container removal failed. Ignoring the error and trying to delete the storage account."
-          Write-Warning $_
-        }
-        Remove-AzStorageAccount -StorageAccountName $account.StorageAccountName -ResourceGroupName $account.ResourceGroupName -Force
-      }
-    }
-    if ($WhatIfPreference) {
-      Write-Host "What if: Removing resource group $($group.ResourceGroupName)"
-    }
-    else {
-      Remove-AzResourceGroup -ResourceGroupName $group.ResourceGroupName -Force -AsJob
+    foreach ($account in $accounts) {
+      RemoveStorageAccount -Account $account
     }
   }
 }
@@ -401,6 +307,96 @@ function SetStorageNetworkAccessRules([string]$ResourceGroupName, [array]$AllowI
   }
 }
 
+function RemoveStorageAccount($Account) {
+  Write-Host ($WhatIfPreference ? 'What if: ' : '') + "Readying $($Account.StorageAccountName) in $($Account.ResourceGroupName) for deletion"
+  # If it doesn't have containers then we can skip the explicit clean-up of this storage account
+  if ($Account.Kind -eq "FileStorage") { return }
+
+  $containers = New-AzStorageContext -StorageAccountName $Account.StorageAccountName | Get-AzStorageContainer
+  $blobs = $containers | Get-AzStorageBlob
+  $deleteNow = @()
+
+  try {
+    foreach ($blob in $blobs) {
+      $shouldDelete = EnableBlobDeletion -Blob $blob -StorageAccountName $Account.StorageAccountName -ResourceGroupName $Account.ResourceGroupName
+      if ($shouldDelete) {
+        $deleteNow += $blob
+      }
+    }
+  } catch {
+    Write-Warning "Ensure user has 'Storage Blob Data Owner' RBAC permission on subscription or resource group"
+    Write-Error $_
+    throw
+  }
+
+  # Blobs with legal holds or immutability policies must be deleted individually
+  # before the container/account can be deleted
+  foreach ($blobToDelete in $deleteNow) {
+    try {
+      $blobToDelete | Remove-AzStorageBlob -Force
+    } catch {
+      Write-Host "Blob removal failed: $($Blob.Name), account: $($Account.storageAccountName), group: $($Account.ResourceGroupName)"
+      Write-Warning "Ignoring the error and trying to delete the storage account"
+      Write-Warning $_
+    }
+  }
+
+  foreach ($container in $containers) {
+    if ($container.BlobContainerProperties.HasImmutableStorageWithVersioning) {
+      try {
+        # Use AzRm cmdlet as deletion will only work through ARM with the immutability policies defined on the blobs
+        Remove-AzRmStorageContainer -Name $container.Name -StorageAccountName $Account.StorageAccountName -ResourceGroupName $Account.ResourceGroupName -Force
+        #$container | Remove-AzStorageContainer
+      } catch {
+        Write-Host "Container removal failed: $($container.Name), account: $($Account.storageAccountName), group: $($Account.ResourceGroupName)"
+        Write-Warning "Ignoring the error and trying to delete the storage account"
+        Write-Warning $_
+      }
+    }
+  }
+
+  if ($containers) {
+    Remove-AzStorageAccount -StorageAccountName $Account.StorageAccountName -ResourceGroupName $Account.ResourceGroupName -Force
+  }
+}
+
+function EnableBlobDeletion($Blob, $StorageAccountName, $ResourceGroupName) {
+  # Some properties like immutability policies require the blob to be
+  # deleted before the container can be deleted
+  $forceBlobDeletion = $false
+
+  # We can't edit blobs with customer encryption without using that key
+  # so just try to delete them fully instead. It is unlikely they
+  # will also have a legal hold enabled.
+  if (($Blob | Get-Member 'ListBlobProperties') `
+      -and $Blob.ListBlobProperties.Properties.CustomerProvidedKeySha256) {
+    return $true
+  }
+
+  if (!($Blob | Get-Member 'BlobProperties')) {
+    return $false
+  }
+
+  if ($Blob.BlobProperties.ImmutabilityPolicy.PolicyMode) {
+    Write-Host "Removing immutability policy - blob: $($Blob.Name), account: $StorageAccountName, group: $ResourceGroupName"
+    $null = $Blob | Remove-AzStorageBlobImmutabilityPolicy
+    $forceBlobDeletion = $true
+  }
+
+  if ($Blob.BlobProperties.HasLegalHold) {
+    Write-Host "Removing legal hold - blob: $($Blob.Name), account: $StorageAccountName, group: $ResourceGroupName"
+    $Blob | Set-AzStorageBlobLegalHold -DisableLegalHold | Out-Null
+    $forceBlobDeletion = $true
+  }
+
+  if ($Blob.BlobProperties.LeaseState -eq 'Leased') {
+    Write-Host "Breaking blob lease: $($Blob.Name), account: $StorageAccountName, group: $ResourceGroupName"
+    $Blob.ICloudBlob.BreakLease()
+  }
+
+  return $forceBlobDeletion
+}
+
 function DoesSubnetOverlap([string]$ipOrCidr, [string]$overlapIp) {
   [System.Net.IPAddress]$overlapIpAddress = $overlapIp
   $parsed = $ipOrCidr -split '/'

eng/pipelines/live-test-cleanup-template.yml

@@ -68,6 +68,7 @@ steps:
             -Verbose `
             ${{ parameters.AdditionalParameters }} `
             -UseExistingAzContext `
+            -GroupFilter '${{ parameters.GroupFilter }}' `
             -WhatIf:$${{ parameters.DryRun }}
 
       displayName: ${{ parameters.DisplayName }}
@@ -90,8 +91,8 @@ steps:
           @subscriptionConfiguration `
           -Verbose `
           ${{ parameters.AdditionalParameters }} `
-          -WhatIf:$${{ parameters.DryRun }} `
-          -GroupFilter '${{ parameters.GroupFilter }}'
+          -GroupFilter '${{ parameters.GroupFilter }}' `
+          -WhatIf:$${{ parameters.DryRun }}
       displayName: ${{ parameters.DisplayName }}
       continueOnError: true
       env: