[AI Generated] BugFix: vm_resize security_profile filter and clearer skip#4572
Open
rabdulfaizy wants to merge 1 commit into
Open
[AI Generated] BugFix: vm_resize security_profile filter and clearer skip#4572rabdulfaizy wants to merge 1 commit into
rabdulfaizy wants to merge 1 commit into
Conversation
…skip Two related fixes in the VM resize test path: 1. azure Resize feature: when selecting a candidate VM size to resize to, also filter out SKUs whose advertised security_profile SetSpace does not include the source VM's deployed security profile (CVM / TrustedLaunch / Standard). Without this filter the random picker can choose a non-CVM SKU for a CVM-deployed VM and every Azure update call is rejected with PropertyChangeNotAllowed, exhausting all retries. 2. VmResize test suite: when the retry loop ends without a successful resize, raise SkippedException including the last Azure error instead of asserting 'fail to find proper vm size'. This is an environment limitation (no compatible candidate was picked across retries), not a defect in the system under test.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves reliability and diagnosability of the Azure VM resize path by filtering resize targets based on the VM’s security profile and by surfacing the last retryable Azure error when resize retries are exhausted.
Changes:
- Add security-profile compatibility filtering when selecting candidate Azure VM sizes for resize.
- Improve
vm_resizeretry exhaustion behavior by skipping with the last observed Azure error instead of failing with a generic assertion.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lisa/sut_orchestrator/azure/features.py | Adds security-profile-aware candidate VM size validation during resize selection. |
| lisa/microsoft/testsuites/core/vm_resize.py | Improves retry-loop failure reporting by raising SkippedException with the last Azure error. |
Comments suppressed due to low confidence (1)
lisa/microsoft/testsuites/core/vm_resize.py:160
- The retry loop uses time.sleep(1), which is discouraged in this repo’s test code because it adds fixed delays and can cause flakiness. Prefer a bounded retry helper (e.g. the
retrydecorator /retry_without_exceptions) with a controlled delay/backoff strategy.
last_error = str(e)
retry = retry + 1
else:
raise e
time.sleep(1)
Comment on lines
+2583
to
+2596
| if not actual_security_profile: | ||
| return True | ||
| assert candidate_size.capability | ||
| assert candidate_size.capability.features | ||
| candidate_sp = next( | ||
| ( | ||
| feature | ||
| for feature in candidate_size.capability.features | ||
| if feature.type == SecurityProfileSettings.type | ||
| ), | ||
| None, | ||
| ) | ||
| if not isinstance(candidate_sp, SecurityProfileSettings): | ||
| return True |
Comment on lines
+2550
to
+2555
| # Read the security profile the source VM was actually deployed with | ||
| # (a concrete value), as opposed to what the SKU advertises in its | ||
| # capability SetSpace. Resizing a CVM-deployed VM to a SKU whose | ||
| # advertised SetSpace includes CVM AND Standard is fine; resizing | ||
| # to a SKU that only advertises Standard will be rejected by Azure | ||
| # with PropertyChangeNotAllowed. |
Comment on lines
158
to
159
| else: | ||
| raise e |
❌ AI Test Selection — FAILED88 test case(s) selected (view run) Marketplace image: canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest
Test case details
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two related fixes in the VM resize test path.
1.
azure/features.py--Resize._select_vm_sizeWhen picking a candidate VM size to resize to, also filter out SKUs whose advertised
security_profileSetSpace does not include the source VM''s deployed security profile (Standard / SecureBoot / CVM)._get_actual_security_profile()reads the deployed VM''ssecurity_profile.security_typevia the compute client and mapsTrustedLaunch->SecureBoot,ConfidentialVM->CVM, else ->Standard._compare_security_profile(candidate_size, actual_security_profile)and its use in_is_candidate_size_validskip any candidate that cannot host the source VM''s security profile.Without this filter the random picker can choose a non-CVM SKU for a CVM-deployed VM and every Azure update call is rejected with
PropertyChangeNotAllowed, exhausting all retries.2.
microsoft/testsuites/core/vm_resize.pyWhen every retry hits retryable Azure errors (
SkuNotAvailable,AllocationFailed, capacity, ...), raiseSkippedExceptionincluding the last Azure error instead of a bareassertthat hid the cause.expected_vm_capability,origin_vm_size,final_vm_size,last_errorbefore the loop.last_error = str(e)in the retryable-error branch.assertwithraise SkippedException(...)includinglast_error.Why
Before this change,
vm_resizeon VMs with SecureBoot/TrustedLaunch or CVM security profiles would routinely:AssertionErrorthat hid the real Azure error -- misclassifying an environment issue as a test bug.How tested
Local runs of
verify_vm_resizeacross all three security profiles on Azure:Standard_F1sStandard_A1_v2_Gen2Standard_DC2as_v5Standard_EC2es_v6Standard_D2ds_v5Standard_A2m_v2_Gen2Lint clean on changed files: black (23.1.x, pinned), flake8, pylint 10/10, mypy (no new errors introduced).
Notes
actual_security_profileisOptional[SecurityProfileType]; whenNonethe filter is a no-op.