Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{Compute} az sig image-version create: Change --virtual-machine parameter map to new VM backend field #28627

Merged
merged 4 commits into from
Mar 26, 2024

Conversation

Jing-song
Copy link
Contributor

Related command

az sig image-version create

Description

close #28488

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change
[Component Name 2] az command b: Add some customer-facing feature


This checklist is used to make sure that common guidelines for a pull request are followed.

Copy link

azure-client-tools-bot-prd bot commented Mar 25, 2024

️✔️AzureCLI-FullTest
️✔️acr
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.11
️✔️3.9
️✔️ams
️✔️latest
️✔️3.11
️✔️3.9
️✔️apim
️✔️latest
️✔️3.11
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.11
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.11
️✔️3.9
️✔️aro
️✔️latest
️✔️3.11
️✔️3.9
️✔️backup
️✔️latest
️✔️3.11
️✔️3.9
️✔️batch
️✔️latest
️✔️3.11
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.11
️✔️3.9
️✔️billing
️✔️latest
️✔️3.11
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.11
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.11
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.11
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.11
️✔️3.9
️✔️config
️✔️latest
️✔️3.11
️✔️3.9
️✔️configure
️✔️latest
️✔️3.11
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.11
️✔️3.9
️✔️container
️✔️latest
️✔️3.11
️✔️3.9
️✔️containerapp
️✔️latest
️✔️3.11
️✔️3.9
️✔️core
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.11
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️dla
️✔️latest
️✔️3.11
️✔️3.9
️✔️dls
️✔️latest
️✔️3.11
️✔️3.9
️✔️dms
️✔️latest
️✔️3.11
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.11
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.11
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.11
️✔️3.9
️✔️find
️✔️latest
️✔️3.11
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.11
️✔️3.9
️✔️identity
️✔️latest
️✔️3.11
️✔️3.9
️✔️iot
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️kusto
️✔️latest
️✔️3.11
️✔️3.9
️✔️lab
️✔️latest
️✔️3.11
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.11
️✔️3.9
️✔️maps
️✔️latest
️✔️3.11
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.11
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.11
️✔️3.9
️✔️mysql
️✔️latest
️✔️3.11
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.11
️✔️3.9
️✔️network
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.11
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.11
️✔️3.9
️✔️profile
️✔️latest
️✔️3.11
️✔️3.9
️✔️rdbms
️✔️latest
️✔️3.11
️✔️3.9
️✔️redis
️✔️latest
️✔️3.11
️✔️3.9
️✔️relay
️✔️latest
️✔️3.11
️✔️3.9
️✔️resource
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️role
️✔️latest
️✔️3.11
️✔️3.9
️✔️search
️✔️latest
️✔️3.11
️✔️3.9
️✔️security
️✔️latest
️✔️3.11
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.11
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.11
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.11
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.11
️✔️3.9
️✔️sql
️✔️latest
️✔️3.11
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.11
️✔️3.9
️✔️storage
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.11
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️util
️✔️latest
️✔️3.11
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9

Copy link

Hi @Jing-song,
Since the current milestone time is less than 7 days, this pr will be reviewed in the next milestone.

Copy link

azure-client-tools-bot-prd bot commented Mar 25, 2024

️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes

@yonzhan
Copy link
Collaborator

yonzhan commented Mar 25, 2024

Compute

@microsoft-github-policy-service microsoft-github-policy-service bot added the Auto-Assign Auto assign by bot label Mar 25, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Compute az vm/vmss/image/disk/snapshot label Mar 25, 2024
@Jing-song Jing-song changed the title [Compute] az sig image-version create: Change managed-image parameter map to new VM backend field [Compute] az sig image-version create: Change --virtual-machine parameter map to new VM backend field Mar 25, 2024
@Jing-song Jing-song marked this pull request as ready for review March 26, 2024 03:15
@zhoxing-ms zhoxing-ms requested a review from ReaNAiveD March 26, 2024 04:14
@zhoxing-ms
Copy link
Contributor

@ReaNAiveD Could you please help review this PR?

Comment on lines 4783 to 4786
if managed_image is not None:
source = GalleryArtifactVersionFullSource(id=managed_image)
if virtual_machine is not None:
source = GalleryArtifactVersionFullSource(virtual_machine_id=virtual_machine)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accord to the L2425-L2434 in file vm/_validators.py, the virtual_machine value is also assigned to managed_image. Consequently, the line source = GalleryArtifactVersionFullSource(id=managed_image) will be executed even when a --virtual-machine argument is provided. This overlap could introduce potential risks.

@@ -4782,6 +4782,8 @@ def create_image_version(cmd, resource_group_name, gallery_name, gallery_image_n
source = os_disk_image = data_disk_images = None
if managed_image is not None:
source = GalleryArtifactVersionFullSource(id=managed_image)
if virtual_machine is not None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue mentioned the min API version is 2023-07-03. Should we wrap the code with min_api check?
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes,added.

@zhoxing-ms
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@Jing-song
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@Jing-song Jing-song closed this Mar 26, 2024
@Jing-song Jing-song reopened this Mar 26, 2024
@Azure Azure deleted a comment from azure-pipelines bot Mar 26, 2024
@Jing-song
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@zhoxing-ms zhoxing-ms changed the title [Compute] az sig image-version create: Change --virtual-machine parameter map to new VM backend field {Compute} az sig image-version create: Change --virtual-machine parameter map to new VM backend field Mar 26, 2024
@zhoxing-ms zhoxing-ms merged commit e52f774 into Azure:dev Mar 26, 2024
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Assign Auto assign by bot Compute az vm/vmss/image/disk/snapshot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sig image-version create managed-image parameter should map to new VM backend field
5 participants