Skip to content

[Compute] sig image-version create: Support creating from a VHD#16371

Merged
qwordy merged 27 commits intoAzure:devfrom
qwordy:15868
Apr 15, 2021
Merged

[Compute] sig image-version create: Support creating from a VHD#16371
qwordy merged 27 commits intoAzure:devfrom
qwordy:15868

Conversation

@qwordy
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

qwordy commented Feb 4, 2021

/azp run

@azure-pipelines
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

yungezz commented Mar 8, 2021

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

@qwordy
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

qwordy commented Apr 9, 2021

The OS part is done.

@olayemio
Copy link
Copy Markdown
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
Copy Markdown
Member Author

qwordy commented Apr 15, 2021

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

@qwordy
Copy link
Copy Markdown
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