Improve resiliency and performance of manage-payg-transition.ps1 by pochiraju · Pull Request #1511 · microsoft/sql-server-samples

Raj Pochiraju (pochiraju) · 2026-09-03T20:16:36Z

Follow-up to #1501: adds retry/backoff for transient Azure network errors, auto-install of Azure CLI and required Az modules, tenant-scoped subscription resolution, and a single shared pre-connection for the Arc/Azure sub-scripts to avoid redundant logins.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

…n.ps1 - Add Invoke-AzCliArgsWithRetry/Invoke-AzCmdletWithRetry helpers that retry transient network failures (socket exhaustion, HttpRequestException, timeouts) with backoff. - Use retry wrapper in Invoke-AzCliLicenseUpdate and Invoke-AzCliQuery. - Fix DataFactory SSIS section: Set-AzContext/Get-AzDataFactoryV2/ Get-AzDataFactoryV2IntegrationRuntime now use -ErrorAction Stop + retry so a transient failure is no longer silently mistaken for 'no integration runtimes found' or left running against the wrong subscription context. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Get-AzSubscription -SubscriptionId / Get-AzSubscription had no -ErrorAction Stop, so a transient HttpRequestException (seen repeatedly outside the dev environment) was a non-terminating error: \ stayed empty, the foreach loop over subscriptions ran zero times, and the script printed a clean-looking 'No resources were marked for modification' summary instead of surfacing the failure. Fixed in both the Arc and Azure sections: retry up to 3x with a 5s backoff, then exit 1 with a clear error if subscription resolution still fails, and abort if the resolved subscription list is empty for any other reason. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…sition blips Real-world runs showed the Arc script exhausting 3 retries (5s each, ~10s total) on 'Unable to acquire token ... An error occurred while sending the request' immediately before the very next (Azure) invocation succeeded with no retry needed at all, confirming these are short-lived (10-30s) transient blips rather than a hard block. Increased to 5 attempts with increasing backoff (5/10/20/30/30s, ~95s total) in both the Arc and Azure sections so the script has a realistic chance to ride out the blip instead of exiting before it clears. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Get-AzSubscription was being called without -TenantId, so for a multi-tenant signed-in account (guest access to other tenants), it fanned out and tried to acquire a token for *every* tenant the account belongs to in order to resolve the subscription -> tenant mapping. This produced unrelated 'Authentication failed against tenant ... conditional access ... MFA' warnings for guest tenants even when a specific -TenantId was passed to the script, and made the whole call more fragile since it depended on tenants the user never intended to touch. Added -TenantId \ to both Get-AzSubscription call sites (single-subscription and list-all) in both the Arc and Azure sections, scoping resolution strictly to the requested tenant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…aph, az CLI) - Arc section: auto-install Az.ConnectedMachine/Az.ResourceGraph on demand (mirroring the existing Az.DataFactory pattern) instead of silently continuing after a failed Import-Module, which left Search-AzGraph undefined and crashed later with an unrelated ArgumentNullException. - Guard Search-AzGraph's result against \ before AddRange to avoid 'Value cannot be null' if the module still can't be resolved. - Azure section: fail fast with an actionable error if the 'az' CLI isn't installed, instead of letting every downstream az call fail later with a confusing 'term az is not recognized' error mid-run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Attempts a silent 'winget install Microsoft.AzureCLI' when 'az' isn't on PATH, refreshing the process PATH afterwards so a fresh install can be picked up without restarting the shell. Falls back to the existing clear, actionable error (with a manual install link) if winget isn't available or the install fails. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…sence Replaces the single winget attempt with a real self-healing install path: 1. Try 'winget install Microsoft.AzureCLI' and verify az is now resolvable. 2. If winget is unavailable or didn't work, fall back to downloading and silently running the official Azure CLI MSI (aka.ms/installazurecliwindows) via msiexec, which doesn't depend on winget being present. 3. Only after both real install attempts fail does the script report the actionable manual-install error and exit - it no longer gives up after a single unverified winget call. Adds a shared Refresh-Path helper to reload PATH from Machine/User scopes after each install attempt so a freshly installed az.cmd is found in the same process without restarting the shell. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Adds a single pre-connect step in the top-level (non-embedded) part of the script for -RunMode Single: establishes the Az PowerShell context and, when targeting Azure, verifies/logs in the Azure CLI exactly once before invoking the Arc and Azure sub-scripts. Sets \ / \ so the embedded Azure script's Connect-Azure short-circuits its own Azure CLI install-check/login entirely when the parent already verified it for the same tenant, instead of repeating that work. Falls back to the original full connect logic unchanged when run standalone (e.g. as an Azure Automation runbook) where these env vars won't be set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Raj Pochiraju (pochiraju) · 2026-09-03T20:16:51Z

Travis Wright (@twright-msft) could you please review this PR when you have a chance? Thanks!

Raj Pochiraju (pochiraju) · 2026-09-03T21:58:18Z

Travis Wright (@twright-msft) Great question — a few concrete reasons the script uses az CLI for the Azure SQL resource-license updates instead of pure Az PowerShell:

SQL VMs are broken in Az PowerShell today. \Update-AzSqlVM\ advertises -NoWait/-AsJob\ for async updates, but both are broken in \Az.SqlVirtualMachine\ 2.4.0 — -NoWait\ forwards the bound parameter into the internal \Get-AzSqlVM\ call (which rejects it) and -AsJob\ throws a \NullReferenceException. \�z sql vm update\ doesn't have --no-wait\ either, so for SQL VMs specifically we talk to ARM directly (read-modify-write) to get true async submission — see \Invoke-SqlVmLicenseUpdate.
Consistent async (--no-wait) support across every Azure SQL resource type. \�z sql db/mi/server/elastic-pool/instance-pool update\ all support --no-wait, letting the whole license-transition run submit updates asynchronously and move on, instead of blocking ~1-2 min per resource. Given the SQL VM cmdlet bugs above, we didn't want to assume the PowerShell equivalents (\Set-AzSqlDatabase, \Set-AzSqlElasticPool, \Set-AzSqlInstance, etc.) reliably support non-blocking updates without hitting similar issues, so CLI was used uniformly for that whole family of resources.
Where Az PowerShell is the better/only fit, we do use it exclusively:
- Arc-connected SQL Servers use \Get-AzConnectedMachine/\Set-AzConnectedMachineExtension -NoWait\ (Az.ConnectedMachine) — no CLI equivalent needed, and -NoWait\ here works fine.
- SSIS Integration Runtimes use \Set-AzDataFactoryV2IntegrationRuntime\ (Az.DataFactory) — \�z\ has no direct equivalent for this.
- Subscription/tenant/context management (\Get-AzSubscription, \Set-AzContext) and resource discovery (\Search-AzGraph) are all Az PowerShell.

So it's not an arbitrary mix — Arc and DataFactory paths are pure Az PowerShell where it's solid, and the Azure SQL DB/MI/Server/Pool/VM license-update path uses az CLI specifically because of the async-support gaps/bugs we hit trying to do it purely in Az PowerShell. Happy to explore consolidating further if you know of a reliable non-blocking path in Az.Sql we're missing — could simplify the script by dropping the CLI dependency entirely for that piece.

Raj Pochiraju (pochiraju) · 2026-09-03T22:42:43Z

Travis Wright (@twright-msft) Yes -- and worth noting, this is narrower than it sounds. Checking the Az PowerShell surface for every resource type this script touches:

Resource	Az PowerShell cmdlet	LicenseType param?	Async support?
SQL DB	`Set-AzSqlDatabase`	Yes	`-AsJob` (works)
Elastic Pool	`Set-AzSqlElasticPool`	Yes	`-AsJob` (works)
Managed Instance	`Set-AzSqlInstance`	Yes	`-AsJob` (works)
Instance Pool	`Set-AzSqlInstancePool`	Yes	`-AsJob` (works)
SQL Server enumeration	`Get-AzSqlServer`	n/a	n/a
SQL VM	`Update-AzSqlVM`	Yes	`-NoWait`/`-AsJob` broken in `Az.SqlVirtualMachine` 2.4.0

So only the SQL VM cmdlet is actually broken -- everything else (DB, Elastic Pool, MI, Instance Pool, Server listing) already has working, non-blocking Az PowerShell equivalents in the mature Az.Sql module (a different module than Az.SqlVirtualMachine).

And even for SQL VMs, we don't strictly need the cmdlet fixed: the script already avoids Update-AzSqlVM/az sql vm update entirely for the default async path by talking to ARM directly via Invoke-AzRestMethod (see Invoke-SqlVmLicenseUpdate) -- that's pure Az PowerShell too, no CLI. The only place az is still used for VMs is the synchronous -WaitForCompletion fallback path, which could just as easily poll the ARM operation directly instead of shelling out to az sql vm update.

Bottom line: yes, we could drop the az CLI dependency entirely today, without waiting on the Update-AzSqlVM bug fix, by:

Replacing az sql db/mi/server/elastic-pool/instance-pool list/update with Get-/Set-AzSql* + -AsJob.
Replacing the -WaitForCompletion VM path's az sql vm update call with a direct ARM poll (reusing the existing Invoke-AzRestMethod plumbing).

That would simplify setup (no more auto-installing az via winget/MSI) and remove an entire class of "is az CLI installed/logged in" bootstrapping code. If you're OK with it, I can follow up with a PR that migrates the Azure SQL logic to pure Az PowerShell and removes the CLI dependency -- happy to do that as a fast-follow rather than block this PR on it, since this PR's core value (retry/backoff, auto-install, preconnect optimization) is orthogonal to which tool issues the actual license-type update.

- Rewrite modify-azure-sql-license-type.ps1 (embedded) to use Az PowerShell cmdlets (Get-/Set-AzSqlVM/Instance/Database/ElasticPool/InstancePool, Set-AzDataFactoryV2IntegrationRuntime) instead of Azure CLI, addressing review feedback to drop the CLI dependency. - Add explicit Az.Sql / Az.SqlVirtualMachine module ensure/install/import logic (previously relied on implicit autoloading, which fails when the modules are not already installed). - SQL VM license updates remain fully synchronous (Update-AzSqlVM), since -NoWait/-AsJob are broken in Az.SqlVirtualMachine 2.4.0; other resource types keep the async default via -AsJob, mirroring the previous --no-wait behavior. - Update README.md to reflect CLI-free prerequisites and current synchronous/async behavior per resource type. - Update TESTPLAN.md with a new Round 2 section documenting live validation of the CLI removal across two subscriptions (SQL VM, Managed Instance, SQL Database), the Az.Sql/Az.SqlVirtualMachine regression and fix, and refresh the Known gaps / Required permissions sections for the PowerShell-only implementation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

TESTPLAN.md records live test results against real Microsoft-internal subscriptions, resource names, tenant IDs, and account details. Per Travis's review feedback, remove it from source control and add it to .gitignore so this internal information is not published in the public repo. The file remains locally as a development record; it is simply no longer tracked. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Raj Pochiraju (pochiraju) · 2026-09-04T17:47:20Z

Travis Wright (@twright-msft) Following up: the CLI dependency has now been fully removed as discussed above.

Changes (commit 2eb2123):

Replaced all az sql db/mi/server/elastic-pool/instance-pool calls with Get-/Set-AzSqlDatabase, Get-/Set-AzSqlInstance, Get-/Set-AzSqlElasticPool, Get-/Set-AzSqlInstancePool, Get-AzSqlServer, using -AsJob for the non-blocking default path.
SQL VM updates go through Update-AzSqlVM and are now always synchronous (never -AsJob), since -NoWait/-AsJob are confirmed broken in Az.SqlVirtualMachine 2.4.0 (matches what I described earlier) -- this preserves the guarantee that a VM license-type change is fully committed before the script reports success.
Added explicit Az.Sql/Az.SqlVirtualMachine module install/import steps (previously relied on implicit autoload, which failed on a machine that didn't already have them installed -- caught during live testing).
Verified zero remaining az CLI invocations (grep), removed the CLI auto-install/login bootstrapping code, and updated README.md accordingly.
Live-tested end-to-end across two subscriptions: SQL VM (sync, confirmed via Get-AzSqlVM), Managed Instance round trip, and SQL Database round trip (both directions confirmed via Get-AzSqlDatabase/Get-AzSqlInstance), plus clean -ReportOnly dry runs.

Also removed TESTPLAN.md from source control (commit 1709f37) since it recorded internal subscription/resource details from my testing -- added to .gitignore going forward.

Ready for re-review whenever you get a chance -- thanks for the feedback!

Raj Pochiraju (pochiraju) · 2026-09-04T20:06:37Z

Travis Wright (@twright-msft) Just to clarify -- the CLI removal itself landed in commit 2eb2123 ("Replace Azure CLI with Az PowerShell cmdlets in manage-payg-transition"), which is the commit immediately before the latest one (1709f37, which only removed TESTPLAN.md/.gitignore -- that's the "minor change" you're seeing as most recent). If you're viewing commits individually, that CLI-removal commit might be easy to miss since it's not the very last one; the full cumulative diff on the "Files changed" tab of the PR will show it.

Could you point me to the specific comments/threads that are still open? I don't see any additional review comments recorded on this PR beyond what's in this comment thread, so I want to make sure I address exactly what you're referring to rather than guessing. Happy to make whatever changes are needed.

Travis Wright (twright-msft) · 2026-09-03T21:47:23Z

+#     and (for the Azure CLI) its install-check, instead of repeating that work.
+if ($RunMode -eq "Single") {
+    if (-not (Get-Module -ListAvailable -Name Az.Accounts)) {
+        Write-Output "Az.Accounts module not found. Installing..."


Suggest prompting the user to install here if running interactively. Add -Force parameter to allow for install in non-interactive use.

Travis Wright (twright-msft) · 2026-09-03T21:50:17Z

+
+    $currentCtx = Get-AzContext -ErrorAction SilentlyContinue
+    if ($currentCtx -and $currentCtx.Account -and ([string]::IsNullOrWhiteSpace($TenantId) -or $currentCtx.Tenant.Id -eq $TenantId)) {
+        Write-Output "Already connected to Azure PowerShell as: $($currentCtx.Account) (tenant $($currentCtx.Tenant.Id)). Reusing this context for both the Arc and Azure runs below."


The distinction between "Arc" and "Azure" is not important to the end user. Remove. If running interactively confirm with the user that it is the right context. If running non-interactively and -Force is used then continue. If not, exit with error stating what context is currently configured and telling the user to run with -Force if the script is already going to be running with the right context.

Travis Wright (twright-msft) · 2026-09-03T21:50:53Z

+        Write-Output "Already connected to Azure PowerShell as: $($currentCtx.Account) (tenant $($currentCtx.Tenant.Id)). Reusing this context for both the Arc and Azure runs below."
+    }
+    else {
+        Write-Output "Connecting to Azure PowerShell once for this run..."


Just say 'Connecting to Azure...'

Travis Wright (twright-msft) · 2026-09-03T21:51:24Z

+        if (Get-Command az -ErrorAction SilentlyContinue) {
+            $acct = az account show --output json 2>$null | ConvertFrom-Json
+            if ($acct -and $acct.tenantId -eq $TenantId) {
+                Write-Output "Azure CLI already logged in as: $($acct.user.name) (tenant $TenantId). Reusing this session for the Azure run below."


Just say 'Reusing this session'.

Travis Wright (twright-msft) · 2026-09-04T16:20:09Z

-            } else {
-                # No filters, get all servers
-                $serverQuery = "[]"
+            Write-Output "Querying SQL Servers within this subscription..."


Should be 'SQL servers' not 'SQL Servers'. Make this change throughout anywhere that there is an Azure SQL DB logical server name shown to the user.

Travis Wright (twright-msft) · 2026-09-04T16:23:45Z

+                Get-AzSqlInstancePool -ErrorAction Stop
+            })
+
+            # Mirrors the previous CLI --query filter (license mismatch, optional RG/name scope,


Remove this comment sentence about mirroring the previous CLI experience.

Travis Wright (twright-msft) · 2026-09-04T16:24:27Z

                if ($ReportOnly) {
                    $ipResult = "ReportOnly"
-                    Write-Output "ReportOnly mode enabled. Skipping modification for SQL Instance Pool '$($pool.name)' in RG '$($pool.resourceGroup)' (would change '$($pool.licenseType)' -> '$LicenseType')."
+                    Write-Output "ReportOnly mode enabled. Skipping modification for SQL Instance Pool '$($pool.Name)' in RG '$($pool.ResourceGroupName)' (would change '$($pool.LicenseType)' -> '$LicenseType')."


Do not abbreviate resource group as 'RG' in output messages. Always say 'resource group'. MAke the change throughout.

Travis Wright (twright-msft) · 2026-09-04T16:26:29Z

  | Arc-connected machine | `-NoWait`, reports `RequestSubmitted` | polls the extension, reports `Succeeded` / `Failed` / `TimedOut` |
-  | SQL virtual machine | direct ARM request, reports `RequestSubmitted` | `az sql vm update` waits, reports `Updated` |
+  | **SQL virtual machine** | **always waits**, reports `Updated` | same |
  | **SSIS integration runtime** | **always waits**, reports `Updated` | same |


Remove SSIS completely from this script. We are not trying to change the license type of SSIS.

Travis Wright (twright-msft) · 2026-09-04T16:27:22Z

-  the change to ARM directly (read the resource, change `sqlServerLicenseType`, write it
-  back), which returns in seconds. If that request fails for any reason it automatically
-  falls back to the synchronous `az sql vm update` path.
+  SQL virtual machines are also always synchronous, but for a different reason:


Just simply say that SQL VM is always synchronous. No long explanation needed.

- Prompt before installing Az modules interactively; add -Force to skip prompts for non-interactive use - Remove Arc/Azure distinction wording when confirming account/tenant context; confirm interactively or require -Force - Simplify connection messages to 'Connecting to Azure...' and 'Reusing this session' - Fix 'SQL Servers' -> 'SQL servers' for Azure SQL DB logical server references - Remove stale 'mirrors the previous CLI' comments - Replace 'RG' abbreviation with 'resource group' throughout output messages - Remove SSIS/Data Factory integration runtime support entirely from the script and README - Simplify README SQL VM synchronous-behavior explanation to one sentence Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Raj Pochiraju (pochiraju) · 2026-09-04T21:00:50Z

Travis Wright (@twright-msft) Thanks for the detailed review — all 9 comments have been addressed in commit e1163bf:

Module install prompt – now prompts interactively before installing missing Az modules; added -Force to skip the prompt (required for non-interactive/unattended runs).
Arc/Azure distinction removed – account/tenant context confirmation no longer distinguishes Arc vs Azure. If interactive, it asks for confirmation; if non-interactive, it requires -Force or throws a clear error stating the current context.
Connection message simplified – now just "Connecting to Azure...".
Reuse message simplified – now just "Reusing this session (...)".
"SQL Servers" -> "SQL servers" – fixed throughout wherever an Azure SQL DB logical server name is shown to the user.
Stale CLI-mirroring comment removed.
"RG" abbreviation removed – all output messages now say "resource group".
SSIS removed completely – all SSIS/Data Factory code, module dependency, role assignment, and README references have been removed from the script.
README SQL VM explanation shortened – now a single sentence: "SQL VM updates are always synchronous, regardless of -WaitForCompletion."

Let me know if anything still needs adjustment.

Raj Pochiraju (pochiraju) and others added 8 commits September 1, 2026 08:34

Raj Pochiraju (pochiraju) and others added 2 commits September 4, 2026 07:23

Travis Wright (twright-msft) reviewed Sep 4, 2026

View reviewed changes

Uh oh!

Conversation

Raj Pochiraju (pochiraju) commented Sep 3, 2026

Uh oh!

Raj Pochiraju (pochiraju) commented Sep 3, 2026

Uh oh!

Raj Pochiraju (pochiraju) commented Sep 3, 2026

Uh oh!

Raj Pochiraju (pochiraju) commented Sep 3, 2026

Uh oh!

Raj Pochiraju (pochiraju) commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Raj Pochiraju (pochiraju) commented Sep 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Raj Pochiraju (pochiraju) commented Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Raj Pochiraju (pochiraju) commented Sep 4, 2026 •

edited

Loading