Check the installed verson when extension has patch version#4432
Check the installed verson when extension has patch version#4432lubaihua33 wants to merge 1 commit intomainfrom
Conversation
✅ AI Test Selection — PASSED1 test case(s) selected (view run) Marketplace image: canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest
Test case details
|
There was a problem hiding this comment.
Pull request overview
This PR improves Azure VM extension version validation for pre-publish scenarios where an extension version may be provided as Major.Minor.Patch, while Azure deployment APIs only accept Major.Minor for typeHandlerVersion. It adds helper logic to normalize requested versions for installation and (optionally) validate the installed patch version via VM extension instance view.
Changes:
- Added version parsing/normalization and installed-version lookup helpers to
AzureExtension. - Updated
verify_vm_extension_install_uninstallto install using normalizedMajor.Minorwhile validating installedMajor.Minor.Patchwhen requested.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| lisa/sut_orchestrator/azure/features.py | Add helpers to normalize typeHandlerVersion input and retrieve installed extension version from instance view. |
| lisa/microsoft/testsuites/vm_extensions/generic_vm_extension.py | Use normalization for install and assert installed patch version when a patch version is requested. |
| "Invalid extension_version format. Expected 'Major.Minor' " | ||
| f"or 'Major.Minor.Patch', got '{version}'." |
There was a problem hiding this comment.
normalize_type_handler_version() strips whitespace into requested_version, but callers (and the exception message) still use the unstripped version. This makes behavior inconsistent: e.g., a runbook value like "1.7.1 " will normalize correctly for install, but later comparisons / resource naming may still use the whitespace. Consider returning the stripped version (or stripping at the call site) and using the sanitized value consistently for extension name and any version comparisons.
| "Invalid extension_version format. Expected 'Major.Minor' " | |
| f"or 'Major.Minor.Patch', got '{version}'." | |
| "Invalid extension_version format after trimming whitespace. " | |
| "Expected 'Major.Minor' or 'Major.Minor.Patch', " | |
| f"got '{requested_version}'. Verify the runbook value does not " | |
| "contain extra characters and uses only numeric version parts." |
| return str( | ||
| getattr(extension_obj, "type_handler_version_name", "unknown") |
There was a problem hiding this comment.
get_installed_type_handler_version() falls back to returning the string "unknown" when it can't find a version field. That can allow callers (especially the generic test when is_patch_version is false) to proceed without actually having an installed version, masking API/SDK issues. Prefer raising a LisaException when the installed version can't be determined (and include what fields were missing) so failures are explicit and actionable.
| return str( | |
| getattr(extension_obj, "type_handler_version_name", "unknown") | |
| version = getattr(extension_obj, "type_handler_version_name", None) | |
| if version: | |
| return str(version) | |
| missing_fields = [ | |
| "instance_view.type_handler_version", | |
| "type_handler_version", | |
| "type_handler_version_name", | |
| ] | |
| raise LisaException( | |
| f"Unable to determine installed type handler version for extension " | |
| f"'{name}'. Checked fields {missing_fields}, but none were present " | |
| f"or populated in the Azure extension response. instance_view " | |
| f"{'was present' if instance_view is not None else 'was missing'}. " | |
| f"Verify the extension exists and inspect the Azure API/SDK " | |
| f"response for the extension instance view." |
| def assert_installed_type_handler_version( | ||
| self, name: str, expected_version: str | ||
| ) -> str: | ||
| actual_version = self.get_installed_type_handler_version(name) | ||
| if actual_version != expected_version: | ||
| raise LisaException( | ||
| f"Installed extension '{name}' version mismatch: " | ||
| f"expected '{expected_version}', actual '{actual_version}'." | ||
| ) | ||
| return actual_version | ||
|
|
There was a problem hiding this comment.
assert_installed_type_handler_version() is newly added but appears unused in the repo (no references found). To reduce dead code and keep a single source of truth for the mismatch message/behavior, either use this helper from the test(s) or remove it until there’s a concrete caller.
| def assert_installed_type_handler_version( | |
| self, name: str, expected_version: str | |
| ) -> str: | |
| actual_version = self.get_installed_type_handler_version(name) | |
| if actual_version != expected_version: | |
| raise LisaException( | |
| f"Installed extension '{name}' version mismatch: " | |
| f"expected '{expected_version}', actual '{actual_version}'." | |
| ) | |
| return actual_version |
| extension_name = f"{publisher}.{type_}-{version}" | ||
| install_version, is_patch_version = ( | ||
| extension.normalize_type_handler_version(version) | ||
| ) |
There was a problem hiding this comment.
extension_name is built from the raw version, but normalize_type_handler_version() strips/validates a sanitized version separately. If the runbook value has leading/trailing whitespace, the extension name can become invalid even though the normalized install version is fine. Consider stripping/validating version once up front, then use the sanitized value for both extension_name and later comparisons/logging.
| install_version, is_patch_version = ( | ||
| extension.normalize_type_handler_version(version) | ||
| ) |
There was a problem hiding this comment.
The PR description leaves the "Related Issue" section empty. Consider linking the related tracking issue (or explaining why there isn’t one) for traceability, since this change adjusts extension version validation behavior.
lubaihua33
force-pushed
the
baihua/fix0423
branch
from
c4dfcaf to
74bff72
Compare
✅ AI Test Selection — PASSED1 test case(s) selected (view run) Marketplace image: canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest
Test case details
|
Description
When installing a VM extension using the Python SDK or an ARM template, there is only a single parameter available for version control: typeHandlerVersion. This parameter only supports the Major.Minor format. However, for the pre-publish validation scenario, the version is in Major.Minor.Patch format.
Azure platform has the following behavior:
Customers are only able to specify the Major.Minor version during deployment of an extension. When AutoUpgradeMinorVersion is set "True", then the platform will deliver the latest Minor.Patch.Hotfix version of that Major version. If this value is set to "False" then the platform will deliver the latest Patch.Hotfix of the Major.Minor pair.
This PR adds a check for the installed version if the extension version passed in the case is Major.Minor.Patch format. From the response of GET VM extension, we can get the installed version from the instance_view.type_handler_version. Then check if the installed version same as the expected version. If yes, then the case is passed. If not, the case is failed.
Related Issue
Type of Change
Checklist
Test Validation
Test for the following runbooks:
variable:
value: "Microsoft.Azure.Monitor"
is_case_visible: true
value: "AzureMonitorLinuxAgent"
is_case_visible: true
value: "1.7.1"
is_case_visible: true
variable:
value: "Microsoft.Azure.Monitor"
is_case_visible: true
value: "AzureMonitorLinuxAgent"
is_case_visible: true
value: "1.7"
is_case_visible: true
variable:
value: "Microsoft.Azure.Monitor"
is_case_visible: true
value: "AzureMonitorLinuxAgent"
is_case_visible: true
value: "1.7.0"
is_case_visible: true
Key Test Cases:
verify_vm_extension_install_uninstall
Impacted LISA Features:
VM Extension
Tested Azure Marketplace Images:
-Canonical 0001-com-ubuntu-server-focal 20_04-lts-gen2 latest
Test Results