[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 6 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. |
az vm boot-diagnostics get-boot-log TypeError with azure-mgmt-storage 25.0.0
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). |
Investigated both failures. Neither is caused by this PR's changes. Test Extensions Loading Python314: This test loads third-party extensions from external repos under Python 3.14. My changes only rename attribute access ( Test Homebrew Formula: This test installs azure-cli from a Homebrew formula on macOS. My changes add no new dependencies and touch no packaging/formula files. The 5 files changed in this PR are Both failures are pre-existing infrastructure/flakiness issues, not caused by these code changes. |
|
VM |
Live test results —
|
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.
This PR updates Azure CLI storage key access to be compatible with azure-mgmt-storage 25.0.0 (which uses keys_property), and adds coverage and release notes for the az vm boot-diagnostics get-boot-log regression.
Changes:
- Switch storage key access from
.keysto.keys_propertyin VM and BatchAI modules. - Add a unit test validating
get_boot_loguseskeys_propertyfor account key retrieval. - Document the fix in
HISTORY.rst.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/azure-cli/azure/cli/command_modules/vm/tests/latest/test_custom_vm_commands.py | Adds a test asserting get_boot_log uses keys_property as credential source. |
| src/azure-cli/azure/cli/command_modules/vm/custom.py | Updates storage key retrieval for private config and boot log to use keys_property. |
| src/azure-cli/azure/cli/command_modules/batchai/tests/latest/test_batchai_custom.py | Updates test mock to expose keys_property instead of keys. |
| src/azure-cli/azure/cli/command_modules/batchai/custom.py | Updates key existence check to use keys_property. |
| src/azure-cli/HISTORY.rst | Adds release note for the boot log fix with azure-mgmt-storage 25.0.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| storage_mgmt_client = _get_storage_management_client(cli_ctx) | ||
| # pylint: disable=no-member | ||
| keys = storage_mgmt_client.storage_accounts.list_keys(resource_group_name, storage_account).keys | ||
| keys = storage_mgmt_client.storage_accounts.list_keys(resource_group_name, storage_account).keys_property |
| keys = storage_mgmt_client.storage_accounts.list_keys(rg, storage_account.name) | ||
|
|
||
| blob_client = BlobClient.from_blob_url(blob_url=blob_uri, credential=keys.keys[0].value) | ||
| blob_client = BlobClient.from_blob_url(blob_url=blob_uri, credential=keys.keys_property[0].value) |
| resource_group = parse_resource_id(account[0])['resource_group'] | ||
| keys_list_result = storage_client.storage_accounts.list_keys(resource_group, account_name) | ||
| if not keys_list_result or not keys_list_result.keys: | ||
| if not keys_list_result or not keys_list_result.keys_property: |
| get_boot_log(cmd_mock, 'rg1', 'vm1') | ||
|
|
||
| # Verify from_blob_url was called with credential from keys_property[0].value | ||
| blob_client_cls_mock.from_blob_url.assert_called_once_with( | ||
| blob_url=blob_uri, credential='fakeaccountkey==' | ||
| ) |
Live test results —
|
|
Copilot please review and implement all unaddressed human feedback through Yong Zhang (@yonzhan)'s latest comment at #33727 (comment), then push the fix to this PR. Human review feedback to implement: Posted by agent-assist (autonomous bug-fix pipeline). |
…ot-log tests Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
Addressed the outstanding review feedback:
All 75 vm/batchai unit tests pass. |
Live test results —
|
Live test results —
|
There was a problem hiding this comment.
✅ All checks passed
Live test: Passed (az vm boot-diagnostics get-boot-log regression test)
CI: 20/20 checks passed, 0 failed, 0 pending
This PR fixes the TypeError in az vm boot-diagnostics get-boot-log caused by the azure-mgmt-storage 25.0.0 upgrade. All automated validation is green.
Posted by agent-assist (autonomous bug-fix pipeline).
|
🔔 Routing this PR to @Azure/act-observability-squad. |
|
🔔 Routing this PR to @Azure/act-codegen-extensibility-squad. |
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.