[vm] fix az vm boot-diagnostics get-boot-log TypeError with azure-mgmt-storage 25.0.0 - #33727
[vm] fix az vm boot-diagnostics get-boot-log TypeError with azure-mgmt-storage 25.0.0#33727Aditya Pujara (a0x1ab) with Copilot wants to merge 12 commits into
az vm boot-diagnostics get-boot-log TypeError with azure-mgmt-storage 25.0.0#33727Conversation
|
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
added
Compute
az vm boot-diagnostics get-boot-log TypeError with azure-mgmt-storage 25.0.0
![azure-client-tools-agent[bot]](https://avatars.githubusercontent.com/in/4205534?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated CI Review — PR #33727
CI has 2 failing checks on the current head commit that need to be resolved:
- Test Extensions Loading Python314 — failed. See the Azure DevOps run: https://dev.azure.com/azclitools/5147fa83-336e-44ef-bbe0-c86b8ae86cbb/_build/results?buildId=329676
- Test Homebrew Formula — failed. Same build: https://dev.azure.com/azclitools/5147fa83-336e-44ef-bbe0-c86b8ae86cbb/_build/results?buildId=329676
71 of 73 completed checks passed; 17 checks are still pending/running.
Note: the issue-sentinel live-test dispatch could not be triggered this round due to a token-permission restriction (403 Resource not accessible) on the automation side — this is unrelated to the PR itself and will be retried automatically once the underlying CI failures above are addressed.
Please investigate the two failing checks above (Test Extensions Loading Python314 and Test Homebrew Formula) and push a fix.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Copilot the automated CI and live-test review on this PR found failures. Please look into the requested changes here: #33727 (review) and push a fix. Posted by agent-assist (autonomous bug-fix pipeline). |
… update HISTORY.rst
Investigated the two failing checks. Here's what I found:
However, while investigating I found two additional broken
Also added a Fixes pushed in commit |
There was a problem hiding this comment.
❌ CI Failures — Test Mock Needs Update
The following check suites failed:
Azure.azure-cli Full Test (Automation Full Test Python312 Profile Latest instance3)Azure.azure-cli Full Test (Automation Full Test Python314 Profile Latest instance3)
Root Cause
batchai/custom.py was correctly updated to use .keys_property instead of .keys, but the corresponding batchai unit test mock was not updated.
In src/azure-cli/azure/cli/command_modules/batchai/tests/latest/test_batchai_custom.py, the helper function _get_mock_storage_accounts_and_keys (line 60) still creates a namedtuple with the old field name:
Keys = collections.namedtuple('Keys', 'keys') # ← still 'keys', not 'keys_property'When the code now accesses keys_list_result.keys_property, it gets an AttributeError because the namedtuple only has a keys field.
Fix Required
Update _get_mock_storage_accounts_and_keys in test_batchai_custom.py to use keys_property instead of keys:
# Change this:
Keys = collections.namedtuple('Keys', 'keys')
# To this:
Keys = collections.namedtuple('Keys', 'keys_property')This aligns the test mock with the updated batchai/custom.py code that now accesses .keys_property (compatible with azure-mgmt-storage >= 25.0.0).
Posted by agent-assist (autonomous bug-fix pipeline).
|
Copilot the automated CI and live-test review on this PR found failures. Please look into the requested changes here: #33727 (review) and push a fix. Posted by agent-assist (autonomous bug-fix pipeline). |
|
|
||
| # our streamwriter not seekable, so no parallel. | ||
| downloader = blob_client.download_blob(max_concurrency=1) | ||
| downloader.readinto(BootLogStreamWriter(sys.stdout)) |
There was a problem hiding this comment.
Copilot We are able to get the correct account keys but the command continues to fail. It looks like it is failing to read everything into the stream. Here is the log
The command failed with an unexpected error. Here is the traceback:
unsupported operand type(s) for +=: 'int' and 'NoneType'
Traceback (most recent call last):
File "C:\Users\jaysontai\Code\azure-cli\env\Lib\site-packages\knack\cli.py", line 233, in invoke
cmd_result = self.invocation.execute(args)
File "C:\Users\jaysontai\Code\azure-cli\src\azure-cli-core\azure\cli\core\commands_init_.py", line 677, in execute
raise ex
File "C:\Users\jaysontai\Code\azure-cli\src\azure-cli-core\azure\cli\core\commands_init_.py", line 820, in _run_jobs_serially
results.append(self.run_job(expanded_arg, cmd_copy))
~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\jaysontai\Code\azure-cli\src\azure-cli-core\azure\cli\core\commands_init.py", line 789, in run_job
result = cmd_copy(params)
File "C:\Users\jaysontai\Code\azure-cli\src\azure-cli-core\azure\cli\core\commands_init.py", line 335, in call
return self.handler(*args, **kwargs)
~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
File "C:\Users\jaysontai\Code\azure-cli\src\azure-cli-core\azure\cli\core\commands\command_operation.py", line 120, in handler
return op(**command_args)
File "C:\Users\jaysontai\Code\azure-cli\src\azure-cli\azure\cli\command_modules\vm\custom.py", line 2320, in get_boot_log
downloader.readinto(BootLogStreamWriter(sys.stdout))
~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\jaysontai\Code\azure-cli\env\Lib\site-packages\azure\storage\blob_download.py", line 811, in readinto
self._current_content_offset += count
TypeError: unsupported operand type(s) for +=: 'int' and 'NoneType'
To check existing issues, please visit: https://github.com/Azure/azure-cli/issues
There was a problem hiding this comment.
Good catch — that's a separate bug in BootLogStreamWriter. The storage SDK's readinto does self._current_content_offset += stream.write(...), so the target stream's write must return the number of bytes consumed. BootLogStreamWriter.write returned None implicitly.
Fixed in 1af1c81: write now returns len(str_or_bytes) on all paths (including the UnicodeEncodeError fallback, where fewer characters are actually printed but the full chunk has been consumed). Added test_vm_boot_log_writer_returns_written_length covering the normal, non-unicode-shell, and empty-chunk cases, and updated the HISTORY.rst entry.
There was a problem hiding this comment.
Copilot This should've been caught by a scenario test. The managed storage scenario should also be tested by a scenario test it doesnt look like it is right now. It just tests that it turns on.
self.cmd('vm boot-diagnostics enable -g {rg} -n {vm}')
self.cmd('vm show -g {rg} -n {vm}', checks=[
self.check('diagnosticsProfile.bootDiagnostics.enabled', True),
self.check('diagnosticsProfile.bootDiagnostics.storageUri', None)
])
self.cmd('vm boot-diagnostics get-boot-log-uris -g {rg} -n {vm} --expire 100', checks=[
self.exists('consoleScreenshotBlobUri'),
self.exists('serialConsoleLogBlobUri')
])
self.cmd('vm boot-diagnostics get-boot-log -g {rg} -n {vm}')
self.cmd('vm boot-diagnostics enable -g {rg} -n {vm} --storage {sa}')
self.cmd('vm show -g {rg} -n {vm}', checks=[
self.check('diagnosticsProfile.bootDiagnostics.enabled', True),
self.check('diagnosticsProfile.bootDiagnostics.storageUri', '{storage_uri}')
])
It doesn't get the diagnostic logs after enabling managed storage.
There was a problem hiding this comment.
Added vm boot-diagnostics get-boot-log to the custom storage path in test_vm_boot_diagnostics (commit 7ab81e6). The scenario test now calls get-boot-log after both enable (managed storage) and enable --storage {sa} (custom storage), so regressions in either path will be caught by live scenario tests.
There was a problem hiding this comment.
Added get-boot-log-uris and get-boot-log calls after enabling managed storage in the scenario test. The test now verifies both the managed (no storage URI) and explicit storage account paths end-to-end.
…s offsets Co-authored-by: JaysonTaiMicrosoft <268525319+JaysonTaiMicrosoft@users.noreply.github.com>
|
Started a Copilot task using Posted by agent-assist (autonomous bug-fix pipeline). |
Co-authored-by: JaysonTaiMicrosoft <268525319+JaysonTaiMicrosoft@users.noreply.github.com>
|
Started a Copilot task using Posted by agent-assist (autonomous bug-fix pipeline). |
…diagnostics test Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
Live test results —
|
There was a problem hiding this comment.
Live Test Result — Failing
The live-test run for this PR failed on an existing test unrelated to the boot-diagnostics fix:
Failed:
src/azure-cli/azure/cli/command_modules/vm/tests/latest/test_vm_commands.py::VMImageListByAliasesScenarioTest::test_vm_image_list_by_aliasThis asserts the last entry returned byself.assertTrue('lts' in result[-1]['sku']) E AssertionError: False is not trueaz vm image list --offer ubuntu(served from the offline curated image list) has an'lts'SKU. The offline image list data appears to have drifted so the last Ubuntu/Canonical entry no longer hasltsin its SKU name.
Passed:
- All 5
TestVMBootLogunit tests (the actual target of this fix) passed:
test_vm_boot_log_falls_back_to_keys,test_vm_boot_log_handle_unicode,
test_vm_boot_log_init_storage_sdk,test_vm_boot_log_uses_keys_property,
test_vm_boot_log_writer_returns_written_length. - All
test_batchai_customtests (87 tests) passed.
CI: all required checks passed (0 failed, 0 pending).
Run: https://github.com/Azure/issue-sentinel/actions/runs/32091767979
Please investigate test_vm_image_list_by_alias — it was swept into this live-test run because it lives in the same file (test_vm_commands.py) as the boot-diagnostics tests, but the assertion failure ('lts' not in result[-1]['sku']) looks unrelated to this PR's change and may reflect stale/updated offline VM image list data rather than a regression caused by this fix. If it's confirmed pre-existing/unrelated, update the assertion to match current data or mark it appropriately; otherwise fix any real regression it uncovers.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Started a Copilot task using Posted by agent-assist (autonomous bug-fix pipeline). |
…date order-dependent assertions Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
Live test results —
|
Live test results —
|
There was a problem hiding this comment.
Automated CI and Live-Test Review — Failures Found
PR: [vm] fix \az vm boot-diagnostics get-boot-log` TypeError with azure-mgmt-storage 25.0.0(headff97e26`)
Live test (azdev test --live)
- ❌ Run failed: https://github.com/Azure/issue-sentinel/actions/runs/32096027837
- The live-test workflow ran the changed test file(s) for this PR against real Azure resources and the job's test step reported failures. The workflow already posted its own detailed comment with the recorded output on this PR — please check that comment for the exact assertion/traceback.
CI checks (3 failed / 20 total)
- ❌ Azure.azure-cli Full Test — https://github.com/Azure/azure-cli/runs/95595636572 (
Build #20260818.9 had test failures) - ❌ Azure.azure-cli Full Test (Automation Full Test Python314 Profile Latest instance2) — https://github.com/Azure/azure-cli/runs/95594767659
- ❌ Azure.azure-cli Full Test (Automation Full Test Python312 Profile Latest instance2) — https://github.com/Azure/azure-cli/runs/95594469950
All other CI checks passed (linter, style, CLA, and the remaining 15 test shards).
Requested action
Please investigate why the vm module's boot-diagnostics get-boot-log fix causes the live test and the two Automation Full Test shards above to fail, fix the regression (or update the affected tests/recordings if the failure is a legitimate behavior change), and push a commit so CI and the live test can be re-run.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Started a Copilot task using Posted by agent-assist (autonomous bug-fix pipeline). |
…lback - Add 4 missing HTTP interactions to test_vm_boot_diagnostics.yaml for the custom-storage path (get-boot-log-uris + get-boot-log), fixing CI Full Test instance2 playback failures - Update batchai test mock to include both keys_property and legacy keys attributes, ensuring the fallback path is covered Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
|
Automated sensitive-information remediation ran on this pull request.
Agent Assist does not modify source files. The PR creator must remove or replace each suspected value at the linked line:
If a credential was exposed, rotate or revoke it immediately. Detected values are never copied into this comment. ✅ Confirm the finding · ❌ Dispute the finding GitHub only supports a fixed reaction set, so 👍 represents ✅ and 👎 represents ❌. The bot-created reactions are only poll choices. Posted by agent-assist (autonomous bug-fix pipeline). |
🤖 PR Validation — ️✔️ All clear
Related command
az vm boot-diagnostics get-boot-logDescription
In
azure-mgmt-storage25.0.0,StorageAccountListKeysResultnow inherits fromMutableMapping, causing.keysto shadow theStorageAccountKeylist with the built-inMutableMapping.keys()method. The keys list was renamed tokeys_propertyin the regenerated model.vm/custom.py: Inget_boot_log, changekeys.keys[0].value→keys.keys_property[0].valuewhen retrieving the storage account credential for a custom (non-managed) boot diagnostics storage account.test_custom_vm_commands.py: Addtest_vm_boot_log_uses_keys_propertyto assert the correct attribute is used when constructing theBlobClient.Testing Guide
History Notes
[vm]
az vm boot-diagnostics get-boot-log: FixTypeError: 'method' object is not subscriptablewhen VM uses a custom storage account for boot diagnostics (regression with azure-mgmt-storage 25.0.0)This 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.