Repository navigation
[Compute] Fix #34083: az vm create: Preserve ARM deployment errors when HTTP response is consumed - #34238
Conversation
Automated repair stopped after 5 attemptsAddressed in this revision: Retained the shared-core error-preservation fix, expanded SDK unit regressions, and added VM success/error ScenarioTests. Live validation and replay recordings remain incomplete. The PR is still failing after reaching the three-attempt automated repair limit; the recorded cycle has 5 counted attempts and 3 no-change attempts. No further automated fix is requested. The independent review confirmed 2 PR-related failure concerns, 1 unrelated concern, and 0 uncertain concerns, counting the CI build once. Outstanding findings: test strengthMissing replay cassettes (PR-related). The traced path is the new scenarios -> testsdk Unavailable new live-test fixture (PR-related). The VM scenario file uses Upstream CIAzure.azure-cli Full Test, build 365299: PR-related. All 52 checks completed; 49 passed and three failed checks represent this one build and its two failed tasks. Python312 Profile Latest instance2 / azdev test and Python314 Profile Latest instance2 / azdev test both fail during the new success scenario's Test validation
The supplied related-test list does not prove completion: management-group/resource-group outcomes are not supplied, and the VM success scenario was not reached. The automated coverage applicability result does not establish cassette completeness. Core unit regressions do not replace recordings of the VM command's success and error paths. Subscription what-if failure: not PR-related. The supplied human feedback identifies missing test-identity permission in Deterministic review tools reported no findings. No additional production-code defect was confirmed in the parsed-error preservation branch or its traced callers. Focused core regression validation remains appropriate alongside both recorded VM scenarios and the two affected Full Test jobs. Question for maintainersWill a maintainer take over the already specified VM fixture changes and commit both passing scenario recordings while preserving every assertion? ⚖️ Risk assessment — 31/100 · MediumThe Medium rating is driven by failure-handling behavior, shared azure-cli-core code.
🧪 Live test — FailedFAIL (exit 1) Selectors: Changed test files runNew test filesWorkflow run: https://github.com/Azure/issue-sentinel/actions/runs/40510965266 Live-test recordings: 1 regenerated — archived in workflow artifact
Recording filesLast 80 lines of azdev outputRelated command live tests
Ask the Agent Tick a box to queue an action. Requires write access to this repository.
|
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟢 Approval recommended
The narrow handler change has focused regression coverage and no identified blocking issues.
No open issues 🎉
What changed in this PR
Fixes #34083 by preserving Azure deployment errors when az vm create encounters an already-consumed HTTP response.
Changes:
- Uses the SDK error message without rereading the response body.
- Preserves error classification and existing fallback behavior.
- Adds core regressions and command-level error and success tests.
| File | Description |
|---|---|
| src/azure-cli/azure/cli/command_modules/vm/tests/latest/test_vm_create_errors.py | Tests registered command error handling and successful output. |
| src/azure-cli-core/azure/cli/core/commands/tests/test_arm.py | Covers consumed responses, nested details, classification, and fallbacks. |
| src/azure-cli-core/azure/cli/core/commands/arm.py | Adds the SDK-message fallback for deployment errors. |
Review effort: Balanced
|
Please fix CI issues |
There was a problem hiding this comment.
Aditya Pujara (@a0x1ab) Ready for human review. See the X Engineering Agent Report.
2f1e180 to
43512dc
Compare
|
@x-engineering-agent please fix the live test failure so the two scenarios can be recorded. In https://github.com/Azure/issue-sentinel/actions/runs/40083588348 |
|
Automated processing is paused because the local security scan detected sensitive information in this pull request.
Please remove the sensitive values from the PR description, discussion, review comments, and commits. Rewrite the affected commit history if needed, then rotate or revoke any exposed credentials. Automated processing will resume after the PR is clean. For safety, the detected values are not reproduced here. |
43512dc to
849a076
Compare
|
Both scenarios now request Standard_D2s_v3, and the existing VM-size assertion matches; all other assertions are unchanged. Live execution and both cassettes remain pending the approved recording workflow, so this feedback is not yet fully resolved. This host cannot access credentials or commit recordings. The unrelated deployment-stack permission failure is untouched. Commit |
a45bf07 to
c2b6d81
Compare
c2b6d81 to
451041c
Compare
5fdf9cc to
83ea795
Compare
…rors when HTTP response is consumed Root cause: src/azure-cli/azure/cli/command_modules/vm/tests/latest/test_vm_create_errors.py:VMCreateErrorsScenarioTest.test_vm_create_windows_os_disk_too_small requested Standard_D2s_v3 in westus2. The supplied completed live run establishes that this SKU is unavailable to the test subscription, so SkuNotAvailable prevents reaching the intended InvalidParameter disk-size error. Earlier attempts selected a commonly used SKU; this correction instead uses the review's subscription-specific evidence that Standard_D2s_v5 is allowed in westus2. No additional handler or authentication defect is established. Change: Changed only the negative Windows VM scenario's size literal from Standard_D2s_v3 to Standard_D2s_v5. Retained westus2, the 1-GB OS disk and every existing assertion. The success scenario's Standard_D2s_v3 in eastus, production code, authentication and recordings remain unchanged. Restored report after checkpoint omitted disposable report artifacts; no additional source changes made. Alternatives considered: - Repeat Standard_D2s_v3: rejected because completed live evidence establishes that it is unavailable to this subscription. - Change region, other scenario sizes, disk size or assertions: rejected because the review requires preserving those exactly. Verified: - Trusted validation passed (1 command). - New or changed tests fail on the base revision and pass with the change: `src/azure-cli-core/azure/cli/core/commands/tests/test_arm.py::TestTemplateBasedException::test_template_exception_consumed_invalid_template`, `src/azure-cli-core/azure/cli/core/commands/tests/test_arm.py::TestTemplateBasedException::test_template_exception_consumed_message_without_parsed_error`, `src/azure-cli-core/azure/cli/core/commands/tests/test_arm.py::TestTemplateBasedException::test_template_exception_consumed_nested_error`. - Unit tests: 12 executed, 0 skipped. - flake8 and pylint passed. - Pre-publication gates passed: release-artifact, generated-ownership, command-help, suppression, independent-review. - Scenario recordings must be refreshed by a live test run. - Repository CI and live tests run on this PR.
83ea795 to
0e4b40e
Compare
🤖 PR Validation — ❌ Action needed
❌AzureCLI-FullTest
Related command
az vm createDescription
Fixes #34083.
Supersedes #34102 (closed as stale after 30 days without human activity). This rework was regenerated from a fresh analysis of the issue against the current base branch.
Now also changes:
src/azure-cli/azure/cli/command_modules/vm/tests/latest/test_vm_create_errors.py.Preserve ARM deployment errors when HTTP response is consumed.
Root cause: src/azure-cli-core/azure/cli/core/commands/arm.py:handle_template_based_exception reread ex.response.internal_response.text after an absent inner_exception raised AttributeError. An uncached consumed requests stream then raised RuntimeError, hiding the already parsed Azure deployment error. The existing five-line fix correctly uses HttpResponseError.message. This refresh addresses review feedback: previous VM tests replaced a command loader and patched a private invoker rather than exercising the real Azure deployment and error renderer.
Fix: Retained the existing shared-core product fix unchanged. Replaced mocked VM loader/private-invoker tests with genuine self.cmd ScenarioTests for the exact Windows/Standard_B2s/StandardSSD_LRS/empty-public-IP success configuration and a Compute undersized OS-disk deployment error. Expanded credential-free SDK regressions with a three-region/five-SKU matrix, quota guidance and recursively nested Azure details. No generated files or recordings changed.
Origin: commit: 520231e; pull_request: #18132; date: 2021-07-01; evidence: git blame was attempted but this snapshot has no Git metadata. Public commit API patch https://api.github.com/repos/Azure/azure-cli/commits/520231e7e42b877ea7052961d8a8ce73970b4c95 verifies that PR #18132 added raise_subdivision_deployment_error and the direct ex.response.internal_response.text fallback. Its intended feature was NetworkApiVersion support for flexible VMSS creation and deployment error classification. The older …
Why now: cause: Unknown; no recent dependency, Python or Azure service trigger has been established.; evidence: The offending fallback dates to 2021. The sandbox did not execute reported 2.89.1, latest 2.91.0 or development VM creation. Local SDK evidence establishes the exhausted-stream precondition, not why a particular deployment encountered it now.
Blast radius: commands: vm create; vm availability-set create; vmss create; network application-gateway create; network lb create; network cross-region-lb create; network vpn-connection create; deployment group/sub/mg/tenant create and validate; stack group/sub/mg create and validate; sql vm create; sql vm group create; sql vm group ag-listener create; reason: These registrations share handle_template_based_exception. A VM-only workaround …
Risks and limitations (from the job report):
Testing Guide
src/azure-cli-core/azure/cli/core/commands/tests/test_arm.py::TestTemplateBasedException::test_template_exception_consumed_invalid_template,src/azure-cli-core/azure/cli/core/commands/tests/test_arm.py::TestTemplateBasedException::test_template_exception_consumed_message_without_parsed_error,src/azure-cli-core/azure/cli/core/commands/tests/test_arm.py::TestTemplateBasedException::test_template_exception_consumed_nested_error.History Notes
[Compute]
az vm create: Preserve ARM deployment errors when HTTP response is consumedThis 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.
Independent review: an independent reviewer checked this change against 10 acceptance criteria derived from the issue (7 required); every required criterion is backed by evidence in the diff.