Skip to content

Conversation

@zhoxing-ms
Copy link
Contributor

@zhoxing-ms zhoxing-ms commented Mar 29, 2022

Feature request: #21815

Description

  • Support creating VM/VMSS from community gallery image (previously, it was only implemented in the private package from edge build, and there was no merge into the official CLI)
  • Add community gallery legal agreement acceptance
  • Add the verification of whether --os-type is correct when creating VM from community gallery image or shared gallery image

Testing Guide

History Notes

[Compute] az vm/vmss create: Support creating VM/VMSS from community gallery image
[Compute] az vm/vmss create: Add community gallery legal agreement acceptance
[Compute] az vm/vmss create: Add the verification of whether --os-type is correct when creating VM from community gallery image or shared gallery image


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

@ghost ghost requested a review from yonzhan March 29, 2022 10:12
@ghost ghost added the Auto-Assign Auto assign by bot label Mar 29, 2022
@zhoxing-ms zhoxing-ms self-assigned this Mar 29, 2022
@ghost ghost requested a review from wangzelin007 March 29, 2022 10:12
@ghost ghost added this to the Mar 2022 (2022-04-06) milestone Mar 29, 2022
@ghost ghost added the Compute az vm/vmss/image/disk/snapshot label Mar 29, 2022
@yonzhan
Copy link
Collaborator

yonzhan commented Mar 29, 2022

Compute

@zhoxing-ms zhoxing-ms requested a review from kangsun-ctrl March 29, 2022 10:13
@kangsun-ctrl
Copy link
Contributor

Hey Xing, can we update the description of 'eula' in the same pr too? it is #3 from #21815

@zhoxing-ms
Copy link
Contributor Author

zhoxing-ms commented Mar 30, 2022

Hey Xing, can we update the description of 'eula' in the same pr too? it is #3 from #21815

@kangsun-ctrl I don't find any code related to eula in #3. Do you mean by the following code?

c.argument('eula', help='The Eula agreement for the gallery image')

@zhoxing-ms
Copy link
Contributor Author

@kangsun-ctrl By the way, since the community gallery image related commands are still in image-gallery extension, it is not convenient to create scenario tests in the main repo.
Could you help test the private package generated by the edge build before we release this feature to see whether the effect meets your expectation?

@zhoxing-ms zhoxing-ms force-pushed the community_gallery_legal_agreement branch from e439df8 to 340bd61 Compare March 31, 2022 07:40
@zhoxing-ms zhoxing-ms changed the title [Compute] az vm create: Add community gallery legal agreement acceptance [Compute] az vm/vmss create: Support creating VM/VMSS from community gallery image and add community gallery legal agreement acceptance Mar 31, 2022
@zhoxing-ms zhoxing-ms marked this pull request as ready for review March 31, 2022 10:04
@zhoxing-ms zhoxing-ms requested a review from jsntcy as a code owner March 31, 2022 10:04
@zhoxing-ms zhoxing-ms force-pushed the community_gallery_legal_agreement branch from 53451c6 to 263c9cb Compare March 31, 2022 10:09
Comment on lines +418 to +419
image_info = re.search(r'^/CommunityGalleries/([^/]*)/Images/([^/]*)/Versions/.*$', image_reference, re.IGNORECASE)
if not image_info or len(image_info.groups()) < 2:
Copy link
Member

Choose a reason for hiding this comment

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

Why not check image version ?

Suggested change
image_info = re.search(r'^/CommunityGalleries/([^/]*)/Images/([^/]*)/Versions/.*$', image_reference, re.IGNORECASE)
if not image_info or len(image_info.groups()) < 2:
image_info = re.search(r'^/CommunityGalleries/([^/]*)/Images/([^/]*)/Versions/(.*)$', image_reference, re.IGNORECASE)
if not image_info or len(image_info.groups()) < 3:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At present, we need to parse the public_gallery_name and gallery_image_name from community gallery image id to query the community gallery image info, but version is not used for the time being, so we do not further match version.

from ._vm_utils import parse_community_gallery_image_id
image_info = parse_community_gallery_image_id(namespace.image)
from ._client_factory import cf_community_gallery_image
community_gallery_image_info = cf_community_gallery_image(cmd.cli_ctx).get(
location=namespace.location, public_gallery_name=image_info[0], gallery_image_name=image_info[1])

I think we can add version matching when we need to parse the version. What do you think?

@wangzelin007 wangzelin007 removed this from the Apr 2022 (2022-04-26) milestone Apr 1, 2022
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.

5 participants