{AKS} Add provisioningState retry logic with two-phase check validation#32939
{AKS} Add provisioningState retry logic with two-phase check validation#32939deveshdama wants to merge 1 commit intoAzure:devfrom
Conversation
deveshdama
requested review from
bebound,
jiasli,
jsntcy,
kairu-ms and
wangzelin007
as code owners
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
microsoft-github-policy-service
bot
requested review from
FumingZhang,
yanzhudd,
yonzhan and
zhoxing-ms
microsoft-github-policy-service
bot
added
the
ARM
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in two-phase provisioningState retry mechanism to the Azure CLI test SDK's CheckerMixin, designed to handle race conditions where external actors (e.g., Azure Policy) may modify a resource's provisioningState between when a command completes and when the test checks it.
Changes:
- New methods on
CheckerMixin(_is_provisioning_state_check,_should_retry_for_provisioning_state,_cmd_with_retry) implementing the two-phase check logic. cmd()overrides inScenarioTest(live-mode only) andLiveScenarioTestto invoke the retry path when enabled viaAZURE_CLI_TEST_RETRY_PROVISIONING_CHECK=true.- New test file covering both
_should_retry_for_provisioning_stateand_cmd_with_retryacross multiple scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/azure-cli-testsdk/azure/cli/testsdk/base.py |
Core retry logic: new CheckerMixin methods and cmd() overrides for ScenarioTest/LiveScenarioTest |
src/azure-cli-testsdk/azure/cli/testsdk/scenario_tests/tests/test_provisioning_retry.py |
Unit tests covering all retry scenarios including happy path, polling, etag change, timeout, and two-phase validation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/azure-cli-testsdk/azure/cli/testsdk/scenario_tests/tests/test_provisioning_retry.py
Outdated
Show resolved
Hide resolved
yonzhan
removed
the
ARM
deveshdama
force-pushed
the
provisioningstate-retry
branch
6 times, most recently
from
6111c61 to
d1764bc
Compare
Add opt-in retry logic to the Azure CLI test SDK for handling provisioningState race conditions caused by external modifications (e.g., Azure Policy) during live tests. When enabled via AZURE_CLI_TEST_RETRY_PROVISIONING_CHECK=true, the test framework's cmd() method uses a two-phase check approach: Phase 1: Validate provisioningState == Succeeded. If not, poll the resource via 'az resource show --ids' with exponential backoff until it reaches a terminal state (Succeeded, Failed, or Canceled). Phase 2: Validate all remaining checks against the original command result, ensuring the operation itself produced correct output. Features: - Opt-in via environment variable (zero risk to existing tests) - Only activates in live mode (playback tests unaffected) - Exponential backoff with jitter (configurable via env vars) - ETag change detection for external modification warnings - Terminal state short-circuit (Failed/Canceled stop polling) - TimeoutError after configurable max retries
deveshdama
force-pushed
the
provisioningstate-retry
branch
from
d1764bc to
df2d5cf
Compare
|
Attaching repro script for validations. |
FumingZhang
left a comment
FumingZhang
left a comment
There was a problem hiding this comment.
The changes look good to me based on testing in live mode. However, will the recording file generated during live mode execution, especially when the provisioning state check is retried, still work correctly in replay mode? I’m concerned that there could be additional resource get operations recorded, which might cause a mismatch in replay mode.
The retry logic is gated behind self.is_live, so it never runs during playback. cc: @FumingZhang |
This is exactly what concerns me. In replay mode, it shouldn't need to send a resource get request using |
The retry is gated behind I verified this by tracing through vcrpy's source:
this script demonstrates this: pip install vcrpy && python3 vcr_proof_demo.pyOutput: PHASE 2: REPLAY (record_mode='once', only 1 request, 2 skipped)
Consumed: 1/3 interactions
Unconsumed: 2 (the retry poll entries)
cassette.all_played = False
>>> VCR exited cleanly. No error from unconsumed entries. <<<cc: @FumingZhang |
|
|
Claude Opus 4.6 states this PR shows strong AI aiding signals. If it is true, it may be worth considering if we really need this level of complexity (over engineering?), as this will make reviewing and maintaining harder and require more resources for testing. Pasting Claude Opus 4.6's answer below: Based on analyzing the PR's code, commit messages, PR description, and overall patterns, there are several strong signals suggesting this PR was significantly aided by AI: 🔍 Evidence1. Highly polished commit message structureThe single commit message reads like a well-formatted AI-generated summary — bullet points with marketing-style feature descriptions:
This level of structured, polished prose in a commit message is uncommon for typical human-written commits, which tend to be terser. 2. PR description ≈ commit message ≈ code docstringsThe PR description, commit message body, and inline code docstrings all repeat nearly identical phrasing word-for-word (e.g., "two-phase check approach", "Phase 1/Phase 2" descriptions). AI tools tend to generate consistent descriptions across artifacts from the same prompt. 3. Test file is suspiciously comprehensive for a single commitThe test file ( 4. Boilerplate docstring styleEvery method has a precisely formatted docstring: def _cmd_with_retry(self, command, checks, cli_ctx):
"""Execute command with two-phase check validation.
Phase 1: Validate provisioningState == Succeeded, retrying with poll if needed.
Phase 2: Validate all remaining checks on the original result.
Uses etag to detect external modifications (e.g. Azure Policy).
"""This is clean but feels templated — the style is more consistent than typical human code where docstring quality tends to vary. 5. PR description has unfilled boilerplateThe history notes section was left as template text:
This is common when AI generates the PR body from a template prompt but the author doesn't review/clean up the parts that need manual input. 6. Copilot reviewer suggestion was adopted verbatimThe 7. Code and test style uniformityThe production code and test code show unusually consistent style — same commenting patterns, same level of detail, same structural approach. Human-written code typically shows subtle style shifts between production and test code. 📊 Assessment
Verdict: Likely AI-assisted — The code quality is decent and the logic is sound, but the uniformity of style, the comprehensiveness of tests in a single pass, and the polished-but-template-like documentation strongly suggest significant AI involvement in generating the code and tests. The author likely used an AI tool (possibly GitHub Copilot or ChatGPT) to scaffold the implementation and tests, then made targeted adjustments.
|
|
@jiasli over-engineering concern - On whether we need this - @FumingZhang can comment on the BAMI tenant migration as the long-term fix here. For now, my team has been ignoring all failures with this error message, could cause us to miss out on actual failures. |
Add opt-in retry logic to the Azure CLI test SDK for handling provisioningState race conditions caused by external modifications (e.g., Azure Policy) during live tests.
Related command
N/A
Description
Problem:
provisioningState race condition caused by Azure Policy seen in AKS CLI Runners that use azure cli test sdk. Live tests in Microsoft-managed subscriptions intermittently fail with:
Query 'provisioningState' doesn't yield expected value 'Succeeded', instead the actual value is 'Updating'Root cause (thanks @FumingZhang for the thorough investigation):
The failure occurs due to a race condition between the Azure CLI SDK's internal GET request and an Azure Policy PUT request(in our case).
Sequence of events causing this race:
CLI sends PUT to create/update a managed cluster - the operation succeeds (
provisioningState = Succeededat T+0).Azure Policy sends its own PUT to the same resource at T+6s (
user agent = ARM), re-applying policy compliance. This resets provisioningState to Updating.CLI SDK sends GET at T+8s to read the resource and run test assertions. It sees
provisioningState = Updatinginstead ofSucceeded, and the test fails - even though the CLI operation itself succeeded.Proposed Fix in the PR:
When env var
AZURE_CLI_TEST_RETRY_PROVISIONING_CHECK=true, test frameworkcmd()method splits the checks into two sequential phases:Phase-1 : only provisioningState validation : If
provisioningState != Succeeded, poll the resource viaaz resource show --ids <id>with exponential backoff until it reaches a terminal stateSucceeded,Failed, orCanceled.Phase-2 : remaining validations : Validate all non-provisioningState checks against the original command result, validates CLI operation itself produced correct output.
usage of etag:
When provisioningState is not
Succeeded, poll the resource until it reaches a terminal state or timeout. Expectation here is that extrenal operation completes and provisioningState returnsSucceeded, allowing the tests to succeed.etaghere is being used for observability only and it does not change the flow of the code.Testing Guide
Attaching testing script. #32939 (comment)
History Notes
[Component Name 1] BREAKING CHANGE:
az command a: Make some customer-facing breaking change[Component Name 2]
az command b: Add some customer-facing featureThis 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.