Skip to content

Conversation

@qwordy
Copy link
Member

@qwordy qwordy commented Dec 28, 2020

Description

Resolve #15868

[Compute] Upgrade gallery_image_versions to 2020-09-30
[Compute] sig image-version create: Support creating from a VHD

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.

@yonzhan
Copy link
Collaborator

yonzhan commented Dec 28, 2020

Compute

@yungezz yungezz added the Compute az vm/vmss/image/disk/snapshot label Dec 28, 2020
@qwordy qwordy marked this pull request as ready for review February 1, 2021 09:05
@qwordy
Copy link
Member Author

qwordy commented Feb 4, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

c.argument('target_region_encryption', nargs='+',
help='Space-separated list of customer managed keys for encrypting the OS and data disks in the gallery artifact for each region. Format for each region: `<os_des>,<lun1>,<lun1_des>,<lun2>,<lun2_des>`. Use "null" as a placeholder.')
c.argument('vhd', help='Source VHD URI of OS disk', min_api='2020-09-30')
c.argument('vhd_storage_account', help='Name or ID of storage account of source VHD URI of OS disk', min_api='2020-09-30')
Copy link
Contributor

Choose a reason for hiding this comment

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

could vhd resource in another tenant?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. If true, I need to add cross-tenant authentication.

@olayemio
Copy link
Contributor

olayemio commented Feb 8, 2021

I have a few questions looking at the api support. So a customer using VHD will supply vhd and vhd_storage_account flag, but not the os_snapshot flag?

  1. Would it be possible for customers to use the os_snapshot field for the ID of the storage account, and then a separate argument for the URI of the VHD for the OS disk?
  2. Is there support for data disks to use VHD as well? data_snapshots could be used to specify the ID of the storage accounts and then a separate argument for the URI of all the VHDs

@yungezz
Copy link
Member

yungezz commented Mar 8, 2021

hi @qwordy, what's status of this PR? is it ok to merge?

@qwordy
Copy link
Member Author

qwordy commented Mar 18, 2021

  1. Agree. The arguments can be better.
  2. We should also support data disks.
    I plan to merge OS disk part, than put data disks in another PR.

@olayemio
Copy link
Contributor

@qwordy
If you want some suggestions on the arguments, here's something I'm thinking

  • for OS disk, can use --os-snapshot-uri for the VHD uri
  • for data disks, since we need the storage account id and VHD uri, --data-snapshots can be used for the storage account, and the VHD can be specified using --data-vhd and --data-vhd-luns to indicate which uri match up with which storage account

@qwordy
Copy link
Member Author

qwordy commented Apr 7, 2021

@olayemio How about --os-vhd, --os-vhd-storage-account, --data-vhd, --data-vhd-luns, --data-vhd-storage-accounts? Can we omit storage account? A VHD URI is unique.

@qwordy
Copy link
Member Author

qwordy commented Apr 9, 2021

The OS part is done.

@olayemio
Copy link
Contributor

olayemio commented Apr 9, 2021

@qwordy I propose the following --os-vhd-uri, --os-vhd-storage-account, --data-vhds-uri, --data-vhds-luns, --data-vhds-storage-accounts?
The storage account is required to ensure that there is a linked access check for the VHD

@qwordy qwordy merged commit fa56bf5 into Azure:dev Apr 15, 2021
@qwordy
Copy link
Member Author

qwordy commented Apr 15, 2021

Merge OS part first. Data disks will be in another PR.

@qwordy
Copy link
Member Author

qwordy commented Apr 15, 2021

#17706

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Compute az vm/vmss/image/disk/snapshot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Import VHD

8 participants