Repository navigation
[Compute] Fix #34083: az vm create: Preserve ARM deployment errors when HTTP response is consumed - #34238
Conversation
|
Aditya Pujara (@a0x1ab) No semantic defect confirmed at Addressed in this revision: The prior private request asked for the negative VM fixture to use Failure summary: 0 PR-related, 23 not PR-related, 0 uncertain handoff records. The 22 upstream records represent two Azure DevOps builds with 20 failed child tasks, not 22 independent errors; the remaining record is the Agent live run.
|
|
/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. |
c2b6d81 to
451041c
Compare
5f0b3f4 to
184242e
Compare
|
Please fix CI issues |
…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 selected Standard_D2s_v3 in eastus. Supplied live run 40572557688 returned SkuNotAvailable before the intended image-disk validation, failing the InvalidParameter assertion. Earlier attempts kept selecting v3; the current feedback explicitly establishes Standard_D2s_v5 eligibility in eastus. No additional production defect was established in the existing ARM handler fix. Change: Changed exactly one SKU literal in test_vm_create_windows_os_disk_too_small from Standard_D2s_v3 to Standard_D2s_v5. Kept eastus, --os-disk-size-gb 1, every assertion, the success scenario, production code and recordings unchanged. The previous slice's baseline byte comparison confirmed no other changes; restored source inspection confirms the eligible SKU and retained assertions. Alternatives considered: - Keep Standard_D2s_v3: rejected because current subscription evidence and live failure establish it is unavailable in eastus. - Move region: rejected because feedback requires eastus and supplies Standard_D2s_v5 eligibility there. 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.
184242e to
559c047
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.