Fix infinite retry loop in SetResource.ps1 (missing $iterator increment)

[madanmishra1223]

Problem

UpdateLoop in .ci/scripts/SetResource.ps1 is meant to retry a failing Set-AzResource call a bounded number of times:

$success = $false
$iterator = 1

while( ($success -eq $false) -and ($iterator -le $maxIterations))
{
    try
    {
        ...
        $success = $true
        break
    }
    catch
    {
        Write-Host("Failed to write resource update - ")
        Write-Host($_.Exception.Message)
        Start-Sleep -Seconds 5
    }
}

if($success -eq $false)
{
    throw "Failed to update resources"
}

$iterator is initialized to 1 but is never incremented. When Set-AzResource keeps failing, $success stays $false and $iterator stays 1, so the guard ($iterator -le $maxIterations) is always true.

Consequences:

• The retry budget the caller passes (UpdateLoop -maxIterations 3 -resource $_) is ignored.
• A resource that can never be tagged (deleted mid-run, insufficient permissions, a provider that rejects the tag) makes the loop spin forever, sleeping 5 seconds and printing the same error each time, until the pipeline job hits its own timeout.
• The throw "Failed to update resources" below the loop is unreachable on persistent failure, so a genuine failure is reported as a hung/timed-out job instead of a clear error.

Fix

Increment $iterator in the catch block, so the loop honors -maxIterations and a persistent failure exits via the existing throw.

             Write-Host($_.Exception.Message)
+            $iterator++
             Start-Sleep -Seconds 5

One line changed; no behavior change on the success path (that path still breaks on the first successful call).

[madanmishra1223]

@microsoft-github-policy-service agree

[Arsh Verma (ArshVermaGit)]

The fix looks correct and the retry counter now advances on failures as expected. Could you add a small test covering persistent Set-AzResource failure and verifying that -maxIterations is respected? That would prevent this infinite-loop regression from coming back. Otherwise the change looks good.

[madanmishra1223]

Thanks Arsh Verma (@ArshVermaGit) — test added in .ci/scripts/SetResource.Tests.ps1 (Pester).

The tricky part

A test that just fails forever is the wrong shape here: if the counter ever regressed, that test would hang the run rather than fail it — the same failure mode as the bug. So the mock fails well past the retry budget and then starts succeeding. A loop that never counts its attempts still terminates, and the test fails on the assertions instead of hanging:

$script:attempts = 0
Mock Set-AzResource {
    $script:attempts++
    if ($script:attempts -le 20) { throw 'persistent failure' }
}

{ UpdateLoop -maxIterations 3 -resource $script:resource } |
    Should -Throw 'Failed to update resources'

$script:attempts | Should -Be 3

Three cases covered: gives up after maxIterations on persistent failure, stops retrying as soon as a retry succeeds, and no retry when the first attempt succeeds.

No Azure is touched — the Az cmdlets are stubbed so Pester has something to mock, and Get-AzResource returns nothing, so dot-sourcing the script just imports its functions and the top-level tagging pass is a no-op.

Verified both ways

Pester 6.1.0 on PowerShell 7.6.5.

With the fix:

  [+] gives up after maxIterations when Set-AzResource keeps failing 140ms
  [+] stops retrying as soon as Set-AzResource succeeds 16ms
  [+] does not retry when the first attempt succeeds 35ms
Tests Passed: 3, Failed: 0

With the $iterator++ reverted (bug reintroduced):

  [-] gives up after maxIterations when Set-AzResource keeps failing 181ms
   Expected an exception with message like 'Failed to update resources' to be thrown, but no exception was thrown.
Tests Passed: 2, Failed: 1

181ms, not a hang — which is the point of the escape hatch above.

One note

This repo has no PowerShell test runner wired up (no GitHub Actions workflows at all, and the .ci/ YAML files are Azure DevOps templates for the linked tutorial repos rather than checks on this repo). So nothing runs this file automatically yet — it is run with:

Install-Module Pester -Scope CurrentUser -Force
Invoke-Pester .ci/scripts/SetResource.Tests.ps1

Happy to add a small workflow to run it on PRs if you'd like that, though I left it out to keep this PR scoped to the fix you reviewed.