[vm] fix az vm boot-diagnostics get-boot-log TypeError with azure-mgmt-storage 25.0.0

src/azure-cli/HISTORY.rst

@@ -121,6 +121,7 @@ Release History
 
 **Compute**
 
+* `az vm boot-diagnostics get-boot-log`: Fix `TypeError: 'method' object is not subscriptable` with azure-mgmt-storage 25.0.0 (#33727)
 * `az vm create/update/show`: Support scheduled events profile via new parameters `--scheduled-events-api-version` and `--enable-all-instance-down` (#33451)
 * `az vmss create/update/show`: Support scheduled events profile via new parameters `--scheduled-events-api-version` and `--enable-all-instance-down` (#33451)
 * `az availability-set create/show`: Support scheduled events profile via new parameters `--scheduled-events-api-version` and `--enable-all-instance-down` (#33451)

src/azure-cli/azure/cli/command_modules/batchai/custom.py

@@ -160,14 +160,24 @@ def _get_storage_account_key(cli_ctx, account_name, account_key):
         raise CLIError('Cannot find "{0}" storage account.'.format(account_name))
     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:
+    account_keys = _get_account_keys_list(keys_list_result)
+    if not account_keys:
         raise CLIError('Cannot find a key for "{0}" storage account.'.format(account_name))
-    key_value = None
-    try:
-        key_value = keys_list_result.keys_property[0].value
-    except AttributeError:
-        key_value = keys_list_result.keys[0].value
-    return key_value
+    return account_keys[0].value
+
+
+def _get_account_keys_list(keys_list_result):
+    """Returns the list of storage account keys from a list_keys result.
+
+    In azure-mgmt-storage 25.0.0 the `keys` attribute was renamed to `keys_property` because the model
+    now inherits from MutableMapping, where `keys` is a method.
+    """
+    if not keys_list_result:
+        return None
+    account_keys = getattr(keys_list_result, 'keys_property', None)
+    if account_keys is None:
+        account_keys = keys_list_result.keys
+    return account_keys
 
 
 def _get_effective_storage_account_name_and_key(cli_ctx, account_name, account_key):

src/azure-cli/azure/cli/command_modules/batchai/tests/latest/test_batchai_custom.py

@@ -57,7 +57,7 @@ def _get_mock_storage_accounts_and_keys(accounts_and_keys):
             '/subscriptions/000/resourceGroups/rg/providers/Microsoft.Storage/storageAccounts/{0}'.format(a),
             a, Endpoints('https://{0}.file.core.windows.net/'.format(a)))
             for a in accounts_and_keys.keys()])
-    Keys = collections.namedtuple('Keys', 'keys')
+    Keys = collections.namedtuple('Keys', 'keys_property')
     Key = collections.namedtuple('Key', 'value')
     mock_storage_client.storage_accounts.list_keys = MagicMock(
         side_effect=lambda _, account: Keys([Key(accounts_and_keys.get(account, None))]))

src/azure-cli/azure/cli/command_modules/vm/custom.py

@@ -201,10 +201,23 @@ def _get_disk_lun_by_aaz(data_disks):
     return len(existing_luns)
 
 
+def _get_account_keys_list(keys_list_result):
+    """Returns the list of storage account keys from a list_keys result.
+
+    In azure-mgmt-storage 25.0.0 the `keys` attribute was renamed to `keys_property` because the model
+    now inherits from MutableMapping, where `keys` is a method.
+    """
+    account_keys = getattr(keys_list_result, 'keys_property', None)
+    if account_keys is None:
+        account_keys = keys_list_result.keys
+    return account_keys
+
+
 def _get_private_config(cli_ctx, resource_group_name, storage_account):
     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 = _get_account_keys_list(
+        storage_mgmt_client.storage_accounts.list_keys(resource_group_name, storage_account))
 
     private_config = {
         'storageAccountName': storage_account,
@@ -2300,7 +2313,7 @@ def get_boot_log(cmd, resource_group_name, vm_name):
     # Get account key
     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=_get_account_keys_list(keys)[0].value)
 
     # our streamwriter not seekable, so no parallel.
     downloader = blob_client.download_blob(max_concurrency=1)

src/azure-cli/azure/cli/command_modules/vm/tests/latest/test_custom_vm_commands.py

@@ -195,6 +195,112 @@ class ErrorToExitCommandEarly(Exception):
         except ErrorToExitCommandEarly:
             get_sdk_mock.assert_called_with(cli_ctx_mock, ResourceType.DATA_STORAGE_BLOB, '_blob_client#BlobClient')
 
+    @mock.patch('azure.cli.command_modules.vm.custom._get_storage_management_client')
+    @mock.patch('azure.cli.command_modules.vm.custom.get_instance_view')
+    @mock.patch('azure.cli.core.profiles.get_sdk', autospec=True)
+    def test_vm_boot_log_uses_keys_property(self, get_sdk_mock, get_instance_view_mock,
+                                            get_storage_management_client_mock):
+        """Verify get_boot_log uses keys_property (not .keys) for azure-mgmt-storage >= 25.0.0."""
+        blob_uri = 'https://mystorage.blob.core.windows.net/bootdiagnostics/vm1/serial.log'
+
+        # VM with instanceView providing a blob URI (custom/non-managed storage path)
+        get_instance_view_mock.return_value = {
+            'instanceView': {
+                'bootDiagnostics': {
+                    'serialConsoleLogBlobUri': blob_uri
+                }
+            }
+        }
+
+        # Storage account that matches the blob URI
+        storage_account_mock = mock.MagicMock()
+        storage_account_mock.primary_endpoints.blob = 'https://mystorage.blob.core.windows.net/'
+        storage_account_mock.id = '/subscriptions/sub1/resourceGroups/rg1/providers/Microsoft.Storage/storageAccounts/mystorage'
+        storage_account_mock.name = 'mystorage'
+
+        storage_mgmt_mock = mock.MagicMock()
+        storage_mgmt_mock.storage_accounts.list.return_value = [storage_account_mock]
+
+        # Simulate azure-mgmt-storage >= 25.0.0: keys_property holds the list of keys
+        key_mock = mock.MagicMock()
+        key_mock.value = 'fakeaccountkey=='
+        keys_result_mock = mock.MagicMock(spec=[])
+        keys_result_mock.keys_property = [key_mock]
+        storage_mgmt_mock.storage_accounts.list_keys.return_value = keys_result_mock
+
+        get_storage_management_client_mock.return_value = storage_mgmt_mock
+
+        # BlobClient mock
+        blob_client_cls_mock = mock.MagicMock()
+        blob_client_mock = mock.MagicMock()
+        blob_client_cls_mock.from_blob_url.return_value = blob_client_mock
+        downloader_mock = mock.MagicMock()
+        blob_client_mock.download_blob.return_value = downloader_mock
+        get_sdk_mock.return_value = blob_client_cls_mock
+
+        cmd_mock = mock.MagicMock()
+        cli_ctx_mock = mock.MagicMock()
+        cmd_mock.cli_ctx = cli_ctx_mock
+
+        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=='
+        )
+        blob_client_mock.download_blob.assert_called_once_with(max_concurrency=1)
+        self.assertEqual(downloader_mock.readinto.call_count, 1)
+
+    @mock.patch('azure.cli.command_modules.vm.custom._get_storage_management_client')
+    @mock.patch('azure.cli.command_modules.vm.custom.get_instance_view')
+    @mock.patch('azure.cli.core.profiles.get_sdk', autospec=True)
+    def test_vm_boot_log_falls_back_to_keys(self, get_sdk_mock, get_instance_view_mock,
+                                            get_storage_management_client_mock):
+        """Verify get_boot_log still works with azure-mgmt-storage < 25.0.0, which only exposes .keys."""
+        blob_uri = 'https://mystorage.blob.core.windows.net/bootdiagnostics/vm1/serial.log'
+
+        get_instance_view_mock.return_value = {
+            'instanceView': {
+                'bootDiagnostics': {
+                    'serialConsoleLogBlobUri': blob_uri
+                }
+            }
+        }
+
+        storage_account_mock = mock.MagicMock()
+        storage_account_mock.primary_endpoints.blob = 'https://mystorage.blob.core.windows.net/'
+        storage_account_mock.id = '/subscriptions/sub1/resourceGroups/rg1/providers/Microsoft.Storage/storageAccounts/mystorage'
+        storage_account_mock.name = 'mystorage'
+
+        storage_mgmt_mock = mock.MagicMock()
+        storage_mgmt_mock.storage_accounts.list.return_value = [storage_account_mock]
+
+        # Simulate azure-mgmt-storage < 25.0.0: only `keys` is available
+        key_mock = mock.MagicMock()
+        key_mock.value = 'legacykey=='
+        keys_result_mock = mock.MagicMock(spec=['keys'])
+        keys_result_mock.keys = [key_mock]
+        storage_mgmt_mock.storage_accounts.list_keys.return_value = keys_result_mock
+        get_storage_management_client_mock.return_value = storage_mgmt_mock
+
+        blob_client_cls_mock = mock.MagicMock()
+        blob_client_mock = mock.MagicMock()
+        blob_client_cls_mock.from_blob_url.return_value = blob_client_mock
+        downloader_mock = mock.MagicMock()
+        blob_client_mock.download_blob.return_value = downloader_mock
+        get_sdk_mock.return_value = blob_client_cls_mock
+
+        cmd_mock = mock.MagicMock()
+        cmd_mock.cli_ctx = mock.MagicMock()
+
+        get_boot_log(cmd_mock, 'rg1', 'vm1')
+
+        blob_client_cls_mock.from_blob_url.assert_called_once_with(
+            blob_url=blob_uri, credential='legacykey=='
+        )
+        blob_client_mock.download_blob.assert_called_once_with(max_concurrency=1)
+        self.assertEqual(downloader_mock.readinto.call_count, 1)
+
 
 class FakedVM:  # pylint: disable=too-few-public-methods
     def __init__(self, nics=None, disks=None, os_disk=None):