Conversation
fixing if/else code block to function
Updating image keywords, changing default Gen1 image from a marketplace alias, to match the style of Gen2/ARM, while updating both URNs
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
Hi @pagienge, |
|
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
requested review from
yanzhudd,
yonzhan and
zhoxing-ms
|
Hi @pagienge Release SuggestionsModule: vm-repair
Notes
|
| else: | ||
| logger.info('No specific distro was provided , using the default Ubuntu distro') | ||
| os_image_urn = "Ubuntu2204" | ||
| os_image_urn = "Canonical:ubuntu-24_04-lts:server-gen1:latest" |
There was a problem hiding this comment.
This loosely correlates to the image already being used in the other two code branches (arm64 and gen2). The issue here was that the Ubuntu2204 alias built elsewhere in ARM had been changed to a Gen2 image which broke several use-cases that rely on a Gen1 VM.
There was a problem hiding this comment.
I did also update the default ARM version since it was 18.04 and that version is way past EoL.
| logger.info('Generation 2 VM detected') | ||
| os_image_urn = _select_distro_linux_gen2(distro) | ||
| if architecture_type == 'Arm64': | ||
| elif architecture_type == 'Arm64': |
There was a problem hiding this comment.
please add a test or show how an existing test passed with this change.
| logger.info('Generation 2 VM detected') | ||
| os_image_urn = _select_distro_linux_gen2(distro) | ||
| if architecture_type == 'Arm64': | ||
| elif architecture_type == 'Arm64': |
There was a problem hiding this comment.
Arm64 is an elif to hyper v gen being v2? How are these related?
I can see how they call different methods to set the image urn, and so we only need 1, but why do the if checks make sense? Can Arm64 machines not have hyperv gen2?
There was a problem hiding this comment.
When you select arm64 you aren't allowed to change the generation
There was a problem hiding this comment.
The if/elif/else may not be the absolute best way to solve it, but it makes sense in the current Azure design. The current code is broken in that the initial V2 check never "sticks", it is always set to either ARM or Gen1 because that first V2 'if' is not conditionally tied to the ARM/V1 decision
|
|
||
|
|
||
| def _select_distro_linux(distro): | ||
| # list of images needs to be added to before the docs reflect, and the docs need to remove the keywords long before we remove the reference from the extension |
There was a problem hiding this comment.
As per our VM Repair dev docs, https://dev.azure.com/msazure/AzureWiki/_wiki/wikis/AzureWiki.wiki/702330/VM-Repair , please update the vm repair version in setup.py and history.rst files for this change. Should increment the 2nd integer for this feature.
There was a problem hiding this comment.
I'll try to get a test now that I've got the doc to validate with. Would it be the 2nd or 3rd digit? I don't know what guidelines there are around versions, but I wouldn't think there's really anything important enough for a 'minor', this seems more revisionary, or bug-fix like.
There was a problem hiding this comment.
@pagienge guidelines are her: https://github.com/Azure/azure-cli/blob/dev/doc/extensions/versioning_guidelines.md
I'll add to my doc
|
Moved changes to #8620 for better git fork hygiene |
4 participants
Updates for Linux image detection for az vm repair creation. New images are added, and the Gen 1 default is made to match Gen2 and ARM64. The Gen1 default turned out to actually be a Gen2 VM in practice, as it was dependent on the marketplace aliases.
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az vm repair create --verbose -g BADRG -n BADVM --repair-username USERNAME --repair-password 'yourPassHere!!!1' --distro=KEYWORD
General Guidelines
-- this is probably a revision/update - need a hand with the version info update
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.