{AKS} Fix missing wait on LRO pollers in azuremonitormetrics helpers - #33734
Merged
Julie Zhu (yanzhudd) merged 1 commit intoAug 4, 2026
Merged
Conversation
|
Validation for Azure CLI Full Test Starting...
Thanks for your contribution! |
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
microsoft-github-policy-service
Bot
requested review from
elvazhu521,
Julie Zhu (yanzhudd) and
Yong Zhang (yonzhan)
July 15, 2026 07:30
microsoft-github-policy-service
Bot
added
Auto-Assign
Merged
3 tasks
Collaborator
|
AKS |
Member
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
3 tasks
create_dce, create_dcr, create_dcra, addon_put, create_default_mac and link_grafana_instance all call begin_create_or_update[_by_id] but never call .result()/.wait() on the returned poller before returning. Since the generic ResourceManagementClient used here polls via a background thread, the calling code proceeds as soon as the initial PUT is accepted, without waiting for the resource to actually finish provisioning. This is normally invisible in VCR-recorded tests (responses replay instantly), but is exposed in live tests: azure-cli-testsdk's tearDown() asserts that no thread named "LROPoller" is still alive (scenario_tests/base.py), which fails when a live 'az aks create --sku automatic' run leaves one of these pollers still polling in the background after the command returns. Fix: capture the poller returned by each begin_create_or_update[_by_id] call and call .result() on it before returning, so callers only get control back once the resource has actually finished provisioning.
FumingZhang
force-pushed
the
fix/azuremonitormetrics-lro-wait
branch
from
August 4, 2026 03:41
352bbdf to
4f9c713
Compare
FumingZhang
marked this pull request as ready for review
August 4, 2026 04:25
Member
Author
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a reliability issue in the AKS Azure Monitor metrics provisioning helpers by ensuring ARM long-running operations (LROs) are fully awaited before returning, preventing background LROPoller threads from surviving past command completion in live scenarios.
Changes:
- Capture the poller returned by
begin_create_or_update[_by_id]and call.result()in multipleazuremonitormetricshelper paths (DCE/DCR/DCRA, default AMW, Grafana link). - Await the poller returned by
managed_clusters.begin_create_or_updateinaddon_put.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/dc/dcra_api.py | Awaits DCRA create/update-by-id LRO via poller.result(). |
| src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/dc/dcr_api.py | Awaits DCR create/update-by-id LRO via poller.result(). |
| src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/dc/dce_api.py | Awaits DCE create/update-by-id LRO via poller.result(). |
| src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/amw/create.py | Awaits default AMW create/update-by-id LRO via poller.result(). |
| src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/amg/link.py | Awaits role assignment and Grafana update-by-id LROs via poller.result(). |
| src/azure-cli/azure/cli/command_modules/acs/azuremonitormetrics/addonput.py | Awaits AKS managed cluster update LRO via poller.result(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "properties": {} | ||
| } | ||
| ) | ||
| poller.result() |
Comment on lines
119
to
120
| except CLIError as e: | ||
| raise CLIError(e) |
Comment on lines
+25
to
28
| poller = client.begin_create_or_update(cluster_resource_group_name, cluster_name, mc) | ||
| poller.result() | ||
| except Exception as e: | ||
| raise UnknownError(e) |
Julie Zhu (yanzhudd)
approved these changes
Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 PR Validation — ️✔️ All clear
Related command
az aks create --sku automatic,az aks addon/Azure Monitor metrics enablement commands (any command path that callsensure_azure_monitor_profile_prerequisites, e.g.create_dce/create_dcr/create_dcra/addon_put/create_default_mac/link_grafana_instance).Description
create_dce,create_dcr,create_dcra,addon_put,create_default_mac, andlink_grafana_instance(inazure.cli.command_modules.acs.azuremonitormetrics) all callbegin_create_or_update/begin_create_or_update_by_idbut never call.result()/.wait()on the returned poller before returning.Because the generic
ResourceManagementClientused here polls via a background thread, the code proceeds as soon as the initial PUT is accepted instead of waiting for the resource to actually finish provisioning.This is invisible in VCR-recorded tests (responses replay instantly, so the background poller thread finishes almost immediately), but is exposed in live tests:
azure-cli-testsdk'stearDown()asserts that no thread named"LROPoller"is still alive (scenario_tests/base.py). A liveaz aks create --sku automaticrun (which always provisions DCE/DCR/DCRA/Grafana-link resources for Azure Monitor metrics) can leave one of these pollers still polling in the background after the command returns, failing that assertion, and more generally means the CLI can return control to the caller before these dependent resources are actually ready.Fix: capture the poller returned by each
begin_create_or_update/begin_create_or_update_by_idcall and call.result()on it before returning, so callers only get control back once the resource has actually finished provisioning. No public interface/behavior changes other than the command now waiting for these ARM operations to complete.Testing Guide
Before the fix, a live test run of any
@live_only()scenario that creates a--sku automaticcluster (which always enables Azure Monitor metrics and calls the affected helpers) could fail intearDown()with:After the fix, this teardown assertion should no longer trigger from these code paths, since each
begin_create_or_update[_by_id]call is now fully awaited via.result().History Notes
[AKS] Fix missing wait on LRO pollers when provisioning Azure Monitor metrics artifacts (DCE/DCR/DCRA/Grafana link) for
az aks create --sku automaticThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.