{Compute} az image create: Migrate command to aaz-based implementation#32884
{Compute} az image create: Migrate command to aaz-based implementation#32884william051200 wants to merge 5 commits intoAzure:devfrom
az image create: Migrate command to aaz-based implementation#32884Conversation
william051200
requested review from
jsntcy,
wangzelin007 and
yanzhudd
as code owners
️✔️AzureCLI-FullTest
|
❌AzureCLI-BreakingChangeTest
Please submit your Breaking Change Pre-announcement ASAP if you haven't already. Please note:
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
microsoft-github-policy-service
bot
added
the
Compute
There was a problem hiding this comment.
Pull request overview
Migrates az image create in the VM command module from a management-plane SDK implementation to an AAZ-based implementation.
Changes:
- Switches the
imagecommand group registration to no longer use the compute image SDK command group binding. - Updates
process_image_create_namespaceto resolve VM/disk/snapshot sources via AAZ show operations. - Introduces a local
CachingTypesenum and adjusts parameter model loading accordingly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/azure-cli/azure/cli/command_modules/vm/commands.py | Updates image command group registration to support the AAZ migration path. |
| src/azure-cli/azure/cli/command_modules/vm/_vm_utils.py | Adds a local caching enum used by params after model-loading changes. |
| src/azure-cli/azure/cli/command_modules/vm/_validators.py | Replaces mgmt SDK lookups with AAZ show operations for VM/disk/snapshot source resolution. |
| src/azure-cli/azure/cli/command_modules/vm/_params.py | Stops loading CachingTypes from SDK models and imports the new local enum. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if source_from_vm: | ||
| # pylint: disable=no-member | ||
| namespace.os_type = vm_info.storage_profile.os_disk.os_type | ||
| namespace.os_type = vm_info.get('storageProfile', {}).get('osDisk', {}).get('osType', '') |
There was a problem hiding this comment.
Defaulting namespace.os_type to the empty string can lead to constructing an invalid image create payload (osType is expected to be a valid value like Linux/Windows). Consider either (1) leaving it unset (e.g., None) when it cannot be inferred, or (2) raising a CLIError with a clear message when the VM response does not contain storageProfile.osDisk.osType, so the failure happens early and explains what’s missing.
| namespace.os_type = vm_info.get('storageProfile', {}).get('osDisk', {}).get('osType', '') | |
| os_type = vm_info.get('storageProfile', {}).get('osDisk', {}).get('osType') | |
| if not os_type: | |
| raise CLIError("Failed to infer the OS type from the source virtual machine. " | |
| "The field 'storageProfile.osDisk.osType' is missing in the VM resource. " | |
| "Please ensure the VM has a valid OS type before creating an image from it.") | |
| namespace.os_type = os_type |
| SECURITY = 'Security' | ||
|
|
||
|
|
||
| class CachingTypes(Enum): |
There was a problem hiding this comment.
Introducing a new CachingTypes enum with the same name as the previously SDK-sourced model type can be confusing (especially in a module where CachingTypes historically referred to an RP model). Consider renaming this local enum to something more specific (e.g., DiskCachingTypes) and updating the import in _params.py accordingly to make it clear this is a CLI-local enum.
| class CachingTypes(Enum): | |
| class DiskCachingTypes(Enum): |
| def _figure_out_storage_source_by_aaz(cli_ctx, resource_group_name, source): | ||
| source_blob_uri = None | ||
| source_disk = None | ||
| source_snapshot = None | ||
| source_info = None | ||
| source_restore_point = None | ||
| if urlparse(source).scheme: # a uri? | ||
| source_blob_uri = source | ||
| elif '/disks/' in source.lower(): | ||
| source_disk = source | ||
| elif '/snapshots/' in source.lower(): | ||
| source_snapshot = source | ||
| elif '/restorepoints/' in source.lower(): | ||
| source_restore_point = source | ||
| else: | ||
| source_info, is_snapshot = _get_disk_or_snapshot_info_by_aaz(cli_ctx, resource_group_name, source) | ||
| if is_snapshot: | ||
| source_snapshot = source_info.get('id') | ||
| else: | ||
| source_disk = source_info.get('id') | ||
|
|
||
| return (source_blob_uri, source_disk, source_snapshot, source_restore_point, source_info) |
There was a problem hiding this comment.
This adds a near-duplicate of _figure_out_storage_source with only the underlying ‘get disk/snapshot info’ mechanism changed. To reduce drift and make future fixes apply to both paths, consider refactoring into a single helper that accepts a resolver function (SDK vs AAZ) or consolidating into one implementation with a strategy switch. That keeps the URI/resource-ID parsing logic in one place.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Related command
az image createDescription
Migration from mgmt.compute to aaz-based
Testing Guide
History Notes
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.