Skip to content

Commit 0e4b40e

Browse files
committed
[Compute] Fix #34083: az vm create: Preserve ARM deployment errors 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.
1 parent af86318 commit 0e4b40e

3 files changed

Lines changed: 280 additions & 0 deletions

File tree

‎src/azure-cli-core/azure/cli/core/commands/arm.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,14 @@ def raise_subdivision_deployment_error(error_message, error_code=None):
105105

106106

107107
def handle_template_based_exception(ex):
108+
from azure.core.exceptions import HttpResponseError
109+
108110
try:
109111
raise CLIError(ex.inner_exception.error.message)
110112
except AttributeError:
113+
if isinstance(ex, HttpResponseError):
114+
# The SDK message includes parsed error details; the response stream may be consumed.
115+
raise_subdivision_deployment_error(ex.message, ex.error.code if ex.error else None)
111116
if hasattr(ex, 'response'):
112117
raise_subdivision_deployment_error(ex.response.internal_response.text, ex.error.code if ex.error else None)
113118
else:
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
# --------------------------------------------------------------------------------------------
2+
# Copyright (c) Microsoft Corporation. All rights reserved.
3+
# Licensed under the MIT License. See License.txt in the project root for license information.
4+
# --------------------------------------------------------------------------------------------
5+
6+
import json
7+
import unittest
8+
from types import SimpleNamespace
9+
from unittest import mock
10+
11+
from azure.core.exceptions import HttpResponseError
12+
from azure.core.pipeline.transport import HttpRequest, RequestsTransportResponse
13+
from knack.util import CLIError
14+
from requests import Response
15+
16+
from azure.cli.core.azclierror import DeploymentError, InvalidTemplateError
17+
from azure.cli.core.commands.arm import handle_template_based_exception
18+
19+
20+
class TestTemplateBasedException(unittest.TestCase):
21+
22+
@staticmethod
23+
def _sdk_error(error=None, message=None, consumed=True):
24+
response = Response()
25+
response.status_code = 400
26+
response.reason = 'Bad Request'
27+
response.headers['Content-Type'] = 'application/json'
28+
response._content = json.dumps({'error': error} if error else {}).encode('utf-8')
29+
transport_response = RequestsTransportResponse(
30+
HttpRequest('PUT', 'https://management.azure.com/deployments/test'), response)
31+
exception = HttpResponseError(response=transport_response, message=message)
32+
if consumed:
33+
# Match a streamed requests response whose body was read without caching it.
34+
response._content = False
35+
response._content_consumed = True
36+
return exception
37+
38+
def test_template_exception_consumed_sku_error(self):
39+
message = (
40+
"The requested VM size Standard_B2s is currently not available in location eastus. "
41+
"Please try another size or deploy to a different location or different zone.")
42+
exception = self._sdk_error({'code': 'SkuNotAvailable', 'message': message})
43+
self.assertIsNone(exception.inner_exception)
44+
with self.assertRaisesRegex(RuntimeError, 'already consumed'):
45+
_ = exception.response.internal_response.text
46+
47+
with self.assertRaises(DeploymentError) as raised:
48+
handle_template_based_exception(exception)
49+
self.assertEqual(str(raised.exception), exception.message)
50+
self.assertIn('SkuNotAvailable', str(raised.exception))
51+
self.assertIn(message, str(raised.exception))
52+
53+
def test_template_exception_consumed_sku_errors_across_regions_and_sizes(self):
54+
for region in ('eastus', 'westeurope', 'northeurope'):
55+
for size in ('Standard_B2s', 'Standard_B2ms', 'Standard_D2s_v3',
56+
'Standard_D2s_v5', 'Standard_B1s'):
57+
with self.subTest(region=region, size=size):
58+
message = (
59+
'The requested VM size {} is currently not available in location {}. '
60+
'Please try another size or deploy to a different location or different zone.'
61+
).format(size, region)
62+
exception = self._sdk_error({'code': 'SkuNotAvailable', 'message': message})
63+
self.assertIsNone(exception.inner_exception)
64+
65+
with self.assertRaises(DeploymentError) as raised:
66+
handle_template_based_exception(exception)
67+
self.assertEqual(str(raised.exception), exception.message)
68+
self.assertIn('SkuNotAvailable', str(raised.exception))
69+
self.assertIn(message, str(raised.exception))
70+
71+
def test_template_exception_consumed_quota_error(self):
72+
message = (
73+
'Operation could not be completed as it results in exceeding approved '
74+
'standardDSv5Family Cores quota. Location: eastus, Current Limit: 0, Current Usage: 0, '
75+
'Additional Required: 2. Please request a quota increase.')
76+
exception = self._sdk_error({'code': 'QuotaExceeded', 'message': message})
77+
78+
with self.assertRaises(DeploymentError) as raised:
79+
handle_template_based_exception(exception)
80+
self.assertEqual(str(raised.exception), exception.message)
81+
self.assertIn('QuotaExceeded', str(raised.exception))
82+
self.assertIn(message, str(raised.exception))
83+
84+
def test_template_exception_consumed_nested_error(self):
85+
message = 'At least one resource deployment operation failed.'
86+
detail_message = 'Allocation failed. Please retry later or use a different VM size.'
87+
exception = self._sdk_error({
88+
'code': 'DeploymentFailed',
89+
'message': message,
90+
'details': [{
91+
'code': 'ResourceDeploymentFailure',
92+
'message': 'The resource write operation failed.',
93+
'details': [{'code': 'AllocationFailed', 'message': detail_message}]
94+
}]
95+
})
96+
97+
with self.assertRaises(DeploymentError) as raised:
98+
handle_template_based_exception(exception)
99+
self.assertEqual(str(raised.exception), exception.message)
100+
for expected in ('DeploymentFailed', message, 'ResourceDeploymentFailure',
101+
'The resource write operation failed.', 'AllocationFailed', detail_message):
102+
self.assertIn(expected, str(raised.exception))
103+
104+
def test_template_exception_consumed_invalid_template(self):
105+
message = 'The template deployment is not valid.'
106+
exception = self._sdk_error({'code': 'InvalidTemplateDeployment', 'message': message})
107+
108+
with self.assertRaises(InvalidTemplateError) as raised:
109+
handle_template_based_exception(exception)
110+
self.assertIn('InvalidTemplateDeployment', str(raised.exception))
111+
self.assertIn(message, str(raised.exception))
112+
113+
def test_template_exception_consumed_message_without_parsed_error(self):
114+
message = '(QuotaExceeded) Current Limit: 0, Current Usage: 0, Additional Required: 2.'
115+
exception = self._sdk_error(message=message)
116+
self.assertIsNone(exception.error)
117+
118+
with self.assertRaises(DeploymentError) as raised:
119+
handle_template_based_exception(exception)
120+
self.assertEqual(str(raised.exception), message)
121+
122+
def test_template_exception_sdk_error_without_response(self):
123+
message = 'Deployment failed before a response was available.'
124+
exception = HttpResponseError(message=message)
125+
126+
with self.assertRaises(DeploymentError) as raised:
127+
handle_template_based_exception(exception)
128+
self.assertEqual(str(raised.exception), message)
129+
130+
def test_template_exception_sdk_error_does_not_reread_response(self):
131+
exception = self._sdk_error({'code': 'SkuNotAvailable', 'message': 'Try another VM size.'})
132+
with mock.patch.object(exception.response, 'text', side_effect=AssertionError('Body reread')) as text:
133+
with self.assertRaises(DeploymentError) as raised:
134+
handle_template_based_exception(exception)
135+
text.assert_not_called()
136+
self.assertEqual(str(raised.exception), exception.message)
137+
138+
def test_template_exception_unconsumed_error(self):
139+
message = 'The deployment conflicts with an existing operation.'
140+
exception = self._sdk_error({'code': 'Conflict', 'message': message}, consumed=False)
141+
142+
with self.assertRaises(DeploymentError) as raised:
143+
handle_template_based_exception(exception)
144+
self.assertIn('Conflict', str(raised.exception))
145+
self.assertIn(message, str(raised.exception))
146+
147+
def test_template_exception_inner_error_preserved(self):
148+
exception = self._sdk_error({'code': 'DeploymentFailed', 'message': 'Outer failure.'})
149+
exception.inner_exception = SimpleNamespace(error=SimpleNamespace(message='Inner failure.'))
150+
151+
with self.assertRaises(CLIError) as raised:
152+
handle_template_based_exception(exception)
153+
self.assertIs(type(raised.exception), CLIError)
154+
self.assertEqual(str(raised.exception), 'Inner failure.')
155+
156+
def test_template_exception_legacy_response_preserved(self):
157+
for code, error_type in [('DeploymentFailed', DeploymentError),
158+
('InvalidTemplateDeployment', InvalidTemplateError)]:
159+
with self.subTest(code=code):
160+
exception = SimpleNamespace(
161+
inner_exception=None,
162+
error=SimpleNamespace(code=code),
163+
response=SimpleNamespace(internal_response=SimpleNamespace(text='Original response body.')))
164+
165+
with self.assertRaises(error_type) as raised:
166+
handle_template_based_exception(exception)
167+
self.assertEqual(str(raised.exception), 'Original response body.')
168+
169+
def test_template_exception_unrelated_error_preserved(self):
170+
exception = ValueError('Unrelated VM error.')
171+
172+
with self.assertRaises(CLIError) as raised:
173+
handle_template_based_exception(exception)
174+
self.assertIs(type(raised.exception), CLIError)
175+
self.assertEqual(str(raised.exception), 'Unrelated VM error.')
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
# --------------------------------------------------------------------------------------------
2+
# Copyright (c) Microsoft Corporation. All rights reserved.
3+
# Licensed under the MIT License. See License.txt in the project root for license information.
4+
# --------------------------------------------------------------------------------------------
5+
6+
from azure.cli.core.azclierror import logger as error_logger
7+
from azure.cli.testsdk import ResourceGroupPreparer, ScenarioTest
8+
from azure.cli.testsdk.patches import patch_main_exception_handler
9+
10+
11+
class VMCreateErrorsScenarioTest(ScenarioTest):
12+
13+
def __init__(self, *args, **kwargs):
14+
super().__init__(*args, **kwargs)
15+
# Keep the real CLI error renderer: the default harness patch only raises an exception.
16+
self.recording_patches.remove(patch_main_exception_handler)
17+
self.replay_patches.remove(patch_main_exception_handler)
18+
19+
@ResourceGroupPreparer(name_prefix='cli_test_vm_create_success_', location='eastus')
20+
def test_vm_create_windows_standard_ssd_no_public_ip(self, resource_group):
21+
self.kwargs.update({
22+
'vm': 'windowsvm',
23+
'admin_username': 'clitestuser',
24+
'admin_password': 'TestPassword1234!',
25+
})
26+
27+
result = self.cmd(
28+
'vm create -g {rg} -n {vm} --location eastus '
29+
'--image Win2022Datacenter --size Standard_D2s_v3 '
30+
'--storage-sku StandardSSD_LRS --public-ip-address "" '
31+
'--admin-username {admin_username} --admin-password {admin_password}',
32+
checks=[
33+
self.exists('id'),
34+
self.check('resourceGroup', '{rg}'),
35+
self.check('location', 'eastus'),
36+
self.check('powerState', 'VM running'),
37+
self.check('publicIpAddress', ''),
38+
self.exists('privateIpAddress'),
39+
])
40+
self.assertEqual(result.exit_code, 0)
41+
self.assertTrue(result.output.strip())
42+
created_vm = result.get_output_in_json()
43+
44+
vm = self.cmd('vm show -g {rg} -n {vm}', checks=[
45+
self.check('id', created_vm['id']),
46+
self.check('provisioningState', 'Succeeded'),
47+
self.check('hardwareProfile.vmSize', 'Standard_D2s_v3'),
48+
self.check('storageProfile.imageReference.publisher', 'MicrosoftWindowsServer'),
49+
self.check('storageProfile.imageReference.offer', 'WindowsServer'),
50+
self.check('storageProfile.imageReference.sku', '2022-datacenter-g2'),
51+
self.exists('storageProfile.imageReference.version'),
52+
self.check('storageProfile.osDisk.osType', 'Windows'),
53+
self.check('storageProfile.osDisk.managedDisk.storageAccountType', 'StandardSSD_LRS'),
54+
]).get_output_in_json()
55+
56+
self.cmd('vm get-instance-view -g {rg} -n {vm}', checks=[
57+
self.check("instanceView.statuses[?code=='ProvisioningState/succeeded'] | length(@)", 1),
58+
self.check("instanceView.statuses[?code=='PowerState/running'] | length(@)", 1),
59+
])
60+
61+
nics = vm['networkProfile']['networkInterfaces']
62+
self.assertTrue(nics)
63+
for nic in nics:
64+
self.kwargs['nic_id'] = nic['id']
65+
attached_nic = self.cmd('network nic show --ids {nic_id}').get_output_in_json()
66+
ip_configurations = attached_nic['ipConfigurations']
67+
self.assertTrue(ip_configurations)
68+
for ip_configuration in ip_configurations:
69+
self.assertFalse(ip_configuration.get('publicIpAddress'))
70+
71+
@ResourceGroupPreparer(name_prefix='cli_test_vm_create_disk_error_', location='westus2')
72+
def test_vm_create_windows_os_disk_too_small(self, resource_group):
73+
self.kwargs.update({
74+
'vm': 'invaliddiskvm',
75+
'admin_username': 'clitestuser',
76+
'admin_password': 'TestPassword1234!',
77+
})
78+
79+
# This size is passed into the deployment template; Compute rejects shrinking the image disk.
80+
with self.assertLogs(error_logger, level='WARNING') as logs:
81+
result = self.cmd(
82+
'vm create -g {rg} -n {vm} --location westus2 '
83+
'--image Win2022Datacenter --size Standard_D2s_v5 '
84+
'--storage-sku StandardSSD_LRS --public-ip-address "" '
85+
'--admin-username {admin_username} --admin-password {admin_password} '
86+
'--os-disk-size-gb 1',
87+
expect_failure=True)
88+
89+
self.assertNotEqual(result.exit_code, 0)
90+
self.assertEqual(result.output.strip(), '')
91+
self.assertIsNone(self.cli_ctx.result.result)
92+
self.assertIsNotNone(self.cli_ctx.result.error)
93+
errors = '\n'.join(logs.output)
94+
self.assertIn('InvalidParameter', errors)
95+
self.assertRegex(errors.lower(), r'disk size\s+1\b')
96+
self.assertIn('smaller than', errors.lower())
97+
self.assertIn('image', errors.lower())
98+
for internal_error in ('Traceback', 'RuntimeError', 'already consumed', 'NoneType'):
99+
self.assertNotIn(internal_error, errors)
100+
self.assertNotIn(internal_error, str(self.cli_ctx.result.error))

0 commit comments

Comments
 (0)