Skip to content

Image detection updates #8590

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.

Sign up for GitHub

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

Closed
pagienge wants to merge 2 commits into from
Closed

Image detection updates #8590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
19b1552
Update custom.py
pagienge Mar 21, 2025
d38a850
Update repair_utils.py
pagienge Mar 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/vm-repair/azext_vm_repair/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def create(cmd, vm_name, resource_group_name, repair_password=None, repair_usern
# If the Hyper-V generation is 'V2', log this information and select the Linux distribution for a Gen2 VM.
logger.info('Generation 2 VM detected')
os_image_urn = _select_distro_linux_gen2(distro)
if architecture_type == 'Arm64':
elif architecture_type == 'Arm64':
Copy link
Copy Markdown
Contributor

@Sandido Sandido Mar 24, 2025

Choose a reason for hiding this comment

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

please add a test or show how an existing test passed with this change.

Copy link
Copy Markdown
Contributor

@Sandido Sandido Mar 24, 2025

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Contributor Author

@pagienge pagienge Mar 24, 2025

Choose a reason for hiding this comment

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

When you select arm64 you aren't allowed to change the generation

armNoGen

Copy link
Copy Markdown
Contributor Author

@pagienge pagienge Mar 24, 2025

Choose a reason for hiding this comment

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

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

# If the architecture type is 'Arm64', log this information and select the Linux distribution for an Arm64 VM.
logger.info('ARM64 VM detected')
os_image_urn = _select_distro_linux_Arm64(distro)
Expand Down
27 changes: 20 additions & 7 deletions src/vm-repair/azext_vm_repair/repair_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,11 +526,16 @@ def _fetch_compatible_windows_os_urn_v2(source_vm):


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
Copy link
Copy Markdown
Contributor

@Sandido Sandido Mar 24, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@pagienge pagienge Mar 24, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor

@Sandido Sandido Mar 26, 2025

Choose a reason for hiding this comment

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

@pagienge guidelines are her: https://github.com/Azure/azure-cli/blob/dev/doc/extensions/versioning_guidelines.md
I'll add to my doc

# https://learn.microsoft.com/cli/azure/vm/repair?view=azure-cli-latest#az-vm-repair-create-optional-parameters
image_lookup = {
'rhel7': 'RedHat:rhel-raw:7-raw:latest',
'rhel8': 'RedHat:rhel-raw:8-raw:latest',
'rhel9': 'RedHat:rhel-raw:9-raw:latest',
'ubuntu18': 'Canonical:UbuntuServer:18.04-LTS:latest',
'ubuntu20': 'Canonical:0001-com-ubuntu-server-focal:20_04-lts:latest',
'ubuntu22': 'Canonical:0001-com-ubuntu-server-jammy:22_04-lts:latest',
'ubuntu24': 'Canonical:ubuntu-24_04-lts:server-gen1:latest',
'centos6': 'OpenLogic:CentOS:6.10:latest',
'centos7': 'OpenLogic:CentOS:7_9:latest',
'centos8': 'OpenLogic:CentOS:8_4:latest',
Expand All @@ -547,16 +552,20 @@ def _select_distro_linux(distro):
os_image_urn = distro
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"
Copy link
Copy Markdown
Contributor

@Sandido Sandido Mar 24, 2025

Choose a reason for hiding this comment

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

why this specific image?

Copy link
Copy Markdown
Contributor Author

@pagienge pagienge Mar 24, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@pagienge pagienge Mar 24, 2025

Choose a reason for hiding this comment

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

I did also update the default ARM version since it was 18.04 and that version is way past EoL.

return os_image_urn


def _select_distro_linux_Arm64(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
# https://learn.microsoft.com/cli/azure/vm/repair?view=azure-cli-latest#az-vm-repair-create-optional-parameters
image_lookup = {
'rhel8': 'RedHat:rhel-arm64:8_8-arm64-gen2:latest',
'rhel9': 'RedHat:rhel-arm64:9_2-arm64:latest',
'rhel9': 'RedHat:rhel-arm64:9_3-arm64:latest',
'ubuntu18': 'Canonical:UbuntuServer:18_04-lts-arm64:latest',
'ubuntu20': 'Canonical:0001-com-ubuntu-server-focal:20_04-lts-arm64:latest',
'ubuntu22': 'Canonical:0001-com-ubuntu-server-jammy:22_04-lts-arm64:latest',
'ubuntu24': 'Canonical:ubuntu-24_04-lts:server-arm64:latest',
'centos7': 'OpenLogic:CentOS:7_9-arm64:latest',
}
if distro in image_lookup:
Expand All @@ -567,23 +576,27 @@ def _select_distro_linux_Arm64(distro):
os_image_urn = distro
else:
logger.info('No specific distro was provided , using the default ARM64 Ubuntu distro')
os_image_urn = "Canonical:UbuntuServer:18_04-lts-arm64:latest"
os_image_urn = "Canonical:ubuntu-24_04-lts:server-arm64:latest"
return os_image_urn


def _select_distro_linux_gen2(distro):
# base on the document : https://learn.microsoft.com/en-us/azure/virtual-machines/generation-2#generation-2-vm-images-in-azure-marketplace
# 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
# https://learn.microsoft.com/cli/azure/vm/repair?view=azure-cli-latest#az-vm-repair-create-optional-parameters
image_lookup = {
'rhel7': 'RedHat:rhel-raw:7-raw-gen2:latest',
'rhel8': 'RedHat:rhel-raw:8-raw-gen2:latest',
'rhel9': 'RedHat:rhel-raw:9-raw-gen2:latest',
'ubuntu18': 'Canonical:UbuntuServer:18_04-lts-gen2:latest',
'ubuntu20': 'Canonical:0001-com-ubuntu-server-focal:20_04-lts-gen2:latest',
'ubuntu22': 'Canonical:0001-com-ubuntu-server-jammy:22_04-lts-gen2:latest',
'ubuntu24': 'Canonical:ubuntu-24_04-lts:server:latest',
'centos7': 'OpenLogic:CentOS:7_9-gen2:latest',
'centos8': 'OpenLogic:CentOS:8_4-gen2:latest',
'oracle7': 'Oracle:Oracle-Linux:ol79-gen2:latest',
'oracle8': 'Oracle:Oracle-Linux:ol82-gen2:latest',
'sles12': 'SUSE:sles-12-sp5:gen2:latest',
'sles15': 'SUSE:sles-15-sp3:gen2:latest',
'sles15': 'SUSE:sles-15-sp6:gen2:latest',
}
if distro in image_lookup:
os_image_urn = image_lookup[distro]
Expand All @@ -597,7 +610,7 @@ def _select_distro_linux_gen2(distro):
os_image_urn = "Canonical:UbuntuServer:18_04-lts-gen2:latest"
else:
logger.info('No specific distro was provided , using the default Ubuntu distro')
os_image_urn = "Canonical:UbuntuServer:18_04-lts-gen2:latest"
os_image_urn = "Canonical:ubuntu-24_04-lts:server:latest"
return os_image_urn


Expand Down Expand Up @@ -809,4 +822,4 @@ def _make_public_ip_name(repair_vm_name, associate_public_ip):
public_ip_name = '""'
if associate_public_ip:
public_ip_name = repair_vm_name + "PublicIP"
return public_ip_name
return public_ip_name
Loading