VMBackup: Fix Python 3.12+ compatibility for Azure Linux 4.0#2177
VMBackup: Fix Python 3.12+ compatibility for Azure Linux 4.0#2177jonathanbrenes wants to merge 3 commits into
Conversation
Evidence: Extension is broken on Azure Linux 4.0Production failureOn an unpatched Azure Linux 4.0 VM (Python 3.14.3), the VMSnapshot extension cannot start at all. Every enable attempt exits with code 1, causing Azure Backup jobs to fail with error Extension log ( Crash traceback — cascading The import chain All 4 removed APIs confirmed brokenAfter patching — extension worksUnit tests on AZL4 VM (8/8 pass): End-to-end backup — PASS:
Backward compatibility — 15 distros, all PASS
All patches are guarded by |
The VMSnapshot extension crashes on Azure Linux 4.0 (Python 3.14) due to
four Python APIs removed in recent versions. The extension fails at import
time, causing Azure Backup jobs to fail with error 400039 after hours of
retries.
Root cause — removed Python APIs used by VMBackup:
- distutils.version.LooseVersion (removed in 3.12)
- imp module (removed in 3.12)
- platform.dist() (removed in 3.8)
- platform.linux_distribution() (removed in 3.8)
Changes:
WaagentLib.py:
- Add try/except for LooseVersion import with a minimal shim class
that provides comparison operators (__lt__, __gt__, __eq__, __le__,
__ge__) using regex-based version parsing.
Note: Python never added a stdlib replacement for LooseVersion.
PEP 632 recommends the third-party `packaging` library, but it is
not suitable here: (1) it is not in the stdlib and may be absent on
minimal installs, (2) it enforces strict PEP 440 parsing and rejects
the loose version strings this codebase passes, and (3) adding a
dependency to a VM extension that must run on every Linux distro from
RHEL 7 (Python 2.7) to AZL4 (Python 3.14) is fragile. The inline
shim is zero-dependency and drop-in compatible.
WAAgentUtil.py:
- Add try/except around the `import imp` fallback so it does not crash
on Python 3.12+ when the primary importlib path fails
HandlerUtil.py:
- Replace platform.dist() in get_dist_info() with /etc/os-release
parsing (NAME + VERSION fields)
- Fix invalid escape sequence: replace('\/', '/') was a no-op
(invalid escape treated as literal char); changed to
replace('\\/', '/') which correctly replaces \/ with /
patch/__init__.py:
- Add /etc/os-release ID parsing to DistInfo() fallback
- Add 'azurelinux' mapping in GetMyPatching() -> AzureLinuxPatching
patch/AzureLinuxPatching.py (new):
- Dedicated patching class for Azure Linux 4.0
- All binary paths set to /usr/bin/ (AZL4 merged /usr)
- Uses dnf for package installation
Tested:
- End-to-end Azure Backup on AZL4 VM: PASS (26 min, all phases)
- Backward compat across 10 distros (Python 3.6-3.14): all PASS
RHEL 8/9, Oracle Linux 9, SLES 15/16, Debian 12,
Ubuntu 20.04/22.04/24.04, Azure Linux 4.0
986e482 to
037ab48
Compare
Combine the strengths of both fallback shims discussed in review: - Keep pre-release precedence (alpha < beta < rc < release) via negative sentinels and the [.\-_] separator split (broader than a digits-only tokenizer for kernel-style strings). - Add explicit str() coercion when tuple positions have mismatched types so Py 3 does not raise TypeError comparing int vs str (e.g. '1.0.5' vs '1.0.foo'). This matches the safe pattern from PR Azure#2177. - Add __ne__ so Py 2.7 does not fall back to identity comparison; Py 3 auto-derives it but Py 2.7 does not. - Add __repr__ for cleaner logs and debugging. - Fold six near-identical rich-comparison methods into a single _cmp() helper and one-line delegates, halving the surface area. No behavior change for the 4 in-tree call sites (all pass numeric plugin version strings such as '1.0.9231.0'); shim now degrades safely on arbitrary inputs instead of raising. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5519adfb-3554-4ee6-84ff-61ec7bab7e1a
…seVersion shim - WaagentLib.py: guard 'import crypt' with legacycrypt/passlib fallbacks so the extension no longer crashes at import on Python 3.13+ (e.g. Ubuntu 26.04); gen_password_hash() uses whichever backend resolved, else raises a clear error. - Harden the distutils LooseVersion fallback shim: split on '.', '-', '_'; numeric tokens compare as ints; pre-release tokens (alpha<beta<rc<release) use negative sentinels; mixed-type positions coerce to str to avoid TypeError. Adopts the review-hardened shim (matches vupadhyay-ms a8b9ff6).
Update: crypt removal handling (Python 3.13+ / Ubuntu 26.04) + hardened
|
|
Tested distros / Python versions (patched code executed on-VM under each native Python — import guard, password hashing, version compare, and distro/patch-class selection all PASS):
Not yet re-validated on-VM (Azure run-command guest handler got stuck — infra, not code): Ubuntu 26.04 (Python 3.13+, the |
| passLibImported = False | ||
|
|
||
| try: | ||
| from crypt import crypt as crypt |
There was a problem hiding this comment.
nit: from crypt import crypt as crypt — the as crypt is a no-op. from crypt import crypt does the same thing.
There was a problem hiding this comment.
1. nit — redundant as crypt alias
Changed from crypt import crypt as crypt → from crypt import crypt.
| try: | ||
| from legacycrypt import crypt | ||
| cryptImported = True | ||
| except ImportError: |
There was a problem hiding this comment.
non-blocking: All three fallback branches (crypt, legacycrypt, passlib) use except ImportError: pass, which swallows the reason silently. If none of the three are available and gen_password_hash is later hit, debugging becomes archaeology. Could we log once at import time (or on first call) which sources were tried and failed?
Also worth a comment somewhere that legacycrypt and passlib are not stdlib — same reasoning we've discussed elsewhere for skipping packaging.version. Not a blocker since VMBackup doesn't call gen_password_hash today, but a note prevents someone from wiring this into a real code path assuming it works on every VM.
There was a problem hiding this comment.
Each backend's failure reason is now captured in a cryptImportErrors list and surfaced in gen_password_hash()'s error message (deterministic "on first call" reporting — there's no reliable logger at module import in the vendored waagent). Also added a comment noting legacycrypt is not stdlib and that hashing is only available where a backend is present.
| if cryptImported: | ||
| return crypt(password, salt) | ||
| elif passLibImported: | ||
| return sha512_crypt.hash(password) |
There was a problem hiding this comment.
blocking (potential defect): the caller constructed salt = "${crypt_id}${salt}" at line 451, but the passlib branch doesn't pass it in:
elif passLibImported:
return sha512_crypt.hash(password) # <-- caller's salt is droppedpasslib.hash.sha512_crypt.hash(password) generates its own random salt with default rounds, so if this branch ever runs the customer gets a different hash format than the crypt and legacycrypt branches produce. That's a silent semantic drift under the customer.
Suggest either:
sha512_crypt.using(salt=<parsed_salt_chars>, rounds=<rounds>).hash(password)(parsing the salt back out of the $id$salt string), or just dropping the passlib branch entirely if we're not willing to preserve the exact hash format.
There was a problem hiding this comment.
Took the "drop it" option — the passlib branch and passLibImported flag are removed; hashing is now crypt → legacycrypt, else a clear ImportError. This eliminates the format drift and the non-stdlib dependency. Updated test_waagentlib_password_hash.py accordingly (removed the obsolete passlib test).
| @@ -66,10 +66,11 @@ def searchWAAgentOld(): | |||
| spec.loader.exec_module(waagent) | |||
| except ImportError: | |||
There was a problem hiding this comment.
this is exactly the shape that broke PR #2124 — except ImportError doesn't catch AttributeError, which is what Py versions have started raising when importlib.util.module_from_spec isn't available. A capability check is safer and immune to future exception-type churn:
import importlib.util
if hasattr(importlib.util, 'module_from_spec'):
spec = importlib.util.spec_from_file_location('waagent', agentPath)
waagent = importlib.util.module_from_spec(spec)
spec.loader.exec_module(waagent)
else:
import imp
waagent = imp.load_source('waagent', agentPath)T
There was a problem hiding this comment.
Replaced the exception-based branch with hasattr(importlib.util, 'module_from_spec'). One refinement over the snippet: I also guarded the import importlib.util itself, since Python 2.7 has no importlib.util (a bare import raises ImportError there). Covers all three cases — importlib.util missing (2.6/2.7), module_from_spec missing (3.3–3.4), present (3.5+).
| date_string = r'\/Date(' + str((int)(time_span)) + r')\/' | ||
| stat_rept = "[" + json.dumps(stat_rept, cls = ComplexEncoder) + "]" | ||
| stat_rept = stat_rept.replace('\\\/', '\/') # To fix the datetime format of CreationTime to be consumed by C# DateTimeOffset | ||
| stat_rept = stat_rept.replace('\\/', '/') # To fix the datetime format of CreationTime to be consumed by C# DateTimeOffset |
There was a problem hiding this comment.
blocking (behavior change, not a pure fix): the original .replace('\\\/', '\/') was a no-op — \/ is an invalid escape sequence and Python treated it literally, so the replacement was \/ → \/ (identity). The new code .replace('\\/', '/') actually strips the backslashes, so the C# DateTimeOffset consumer downstream now sees /Date(...)/ instead of \/Date(...)\/. That's a real payload change.
Could we get a sign-off from the C# side (the CreationTime consumer for this status file), or split this into its own PR so it can be audited independently? Right now it rides along inside a Python-3 compatibility PR, which makes it easy to miss during review.
There was a problem hiding this comment.
Good catch prompting a closer look. I tested a regression script that reproduces the exact do_status_report sequence and proved the two forms emit byte-identical output — the replace() runs before the \/Date(...)\/ token is substituted, and json.dumps doesn't emit \/ for normal content, so nothing changes on the wire. To keep this PR a pure compatibility change, I've reverted the line to the master original (verified: no diff vs master). The test stays in to guard the behavior.
| return distinfo[0]+"-"+distinfo[1],platform.release() | ||
| # platform.dist() removed in Python 3.8; parse /etc/os-release instead | ||
| try: | ||
| with open("/etc/os-release", "r") as f: |
There was a problem hiding this comment.
nit: the exact same /etc/os-release parsing block appears in three places:
- here (
get_dist_info) WaagentLib.pyDistInfo()around line 4641patch/__init__.pyDistInfo()around line 64
All three do the same open + partition('=') + strip('"') loop. Would you consider extracting a single read_os_release() helper (e.g., in Utils/) so future distro additions only need one change? Copy-paste tends to drift over time.
There was a problem hiding this comment.
Extracted a single Utils/DistroUtil.read_os_release() helper and wired all three sites (HandlerUtil.get_dist_info, patch.DistInfo, WaagentLib.DistInfo). WaagentLib uses a guarded import so the vendored waagent still runs standalone.
| elif ('redhat'.lower() in Distro.lower()): | ||
| Distro = 'redhat' | ||
| elif ('azurelinux' in Distro.lower()): | ||
| Distro = 'AzureLinux' |
There was a problem hiding this comment.
nit: the AzureLinux distro string is set to two different values in two code paths:
WaagentLib.GetMyDistro(line 4598) mapsazurelinux→'fedora'- here in
GetMyPatchingmapsazurelinux→'AzureLinux'
Is that intentional (patcher class name vs distro-string consumers), or should they match? Either way a one-line comment explaining the divergence would help future readers — right now it looks like a bug even if it isn't.
There was a problem hiding this comment.
It's intentional, and I added comments at both sites saying so. I also validated it on a real Azure Linux 4.0 VM (Python 3.14.3) with a small script — 8/8 checks pass: patch.DistInfo()/WaagentLib.DistInfo() = ['azurelinux','4.0'], GetMyPatching() → AzureLinuxPatching, GetMyDistro() → fedoraDistro. So the patch layer uses the dedicated class while the waagent distro layer reuses the Fedora family, exactly as intended.
- WaagentLib.py: drop redundant 'as crypt'; capture per-backend import failure reasons (cryptImportErrors) and surface them in gen_password_hash(); remove the passlib branch (it self-generated a salt, dropping the caller's -> different hash format) so hashing is crypt -> legacycrypt else a clear ImportError. - WAAgentUtil.py: select the module loader by capability (hasattr(importlib.util, 'module_from_spec')) instead of exception type; guard 'import importlib.util' for Python 2.7 which lacks it. - Utils/DistroUtil.py (new): single shared read_os_release() helper. - HandlerUtil.py, patch/__init__.py, WaagentLib.py: use the shared helper for /etc/os-release parsing (WaagentLib via a guarded import to stay standalone). - HandlerUtil.py: revert the CreationTime escape line to the master original (proven byte-identical output; keeps this a pure compatibility change). - Document the intentional azurelinux mapping divergence (GetMyDistro -> 'fedora' vs GetMyPatching -> 'AzureLinux') with comments at both sites.
|
LGTM |
There was a problem hiding this comment.
Pull request overview
This PR updates the VMBackup (VMSnapshot) extension to run on Azure Linux 4.0 / Python 3.12+ by removing reliance on stdlib APIs that were deprecated/removed in newer Python versions and by adding Azure Linux–specific distro/patching handling.
Changes:
- Added compatibility shims/guards for removed Python APIs (
distutils.version.LooseVersion,crypt,imp,platform.dist()/platform.linux_distribution()). - Introduced shared
/etc/os-releaseparsing to replace deprecated platform distro APIs. - Added an Azure Linux–specific patching implementation and mapping.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| VMBackup/main/WaagentLib.py | Makes crypt and LooseVersion imports resilient on newer Python; adds Azure Linux distro detection/mapping. |
| VMBackup/main/Utils/WAAgentUtil.py | Avoids crashing on Python versions where importlib.util capabilities differ; avoids unsafe imp fallback on 3.12+. |
| VMBackup/main/Utils/HandlerUtil.py | Replaces removed platform.dist() usage with /etc/os-release parsing for telemetry. |
| VMBackup/main/Utils/DistroUtil.py | New shared helper for parsing /etc/os-release. |
| VMBackup/main/patch/AzureLinuxPatching.py | New Azure Linux 4.0 patching class using merged /usr paths and dnf. |
| VMBackup/main/patch/init.py | Adds Azure Linux distro detection via /etc/os-release and maps to AzureLinuxPatching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # platform.dist() removed in Python 3.8; parse /etc/os-release instead | ||
| osr = read_os_release() | ||
| if osr: | ||
| return osr.get("NAME", "Unknown") + "-" + osr.get("VERSION", "Unknown"), platform.release() | ||
| return "Unknown", "Unknown" |
| # platform.dist() / platform.linux_distribution() were removed in Python 3.8, | ||
| # so distro detection now reads /etc/os-release directly. This helper is the | ||
| # single source of truth for that parsing (previously copy-pasted in | ||
| # HandlerUtil.get_dist_info(), WaagentLib.DistInfo(), and patch/__init__.py). |
arisettisanjana
left a comment
arisettisanjana
left a comment
There was a problem hiding this comment.
have tested the latest changes with py3.14 version and it is working fine, LGTM
There was a problem hiding this comment.
ModuleNotFoundError: No module named 'crypt'
when running with python3.14
| if cryptImported == False: | ||
| if (sys.version_info[0] == 3 and sys.version_info[1] >= 13) or (sys.version_info[0] > 3): | ||
| try: | ||
| from legacycrypt import crypt |
There was a problem hiding this comment.
i remember testing it out with the legacycrypt, i have seen an error with 3.14 python version, can you confirm that it is been tested in ubuntu26
Problem
The VMSnapshot (VMBackup) extension crashes on Azure Linux 4.0 (Python 3.14) due to four Python APIs that were removed in recent Python versions. The extension fails at import time, causing Azure Backup jobs to fail with error 400039 after hours of retries.
Removed APIs used by VMBackup
distutils.version.LooseVersionimpmoduleplatform.dist()platform.linux_distribution()Changes
WaagentLib.pytry/exceptforLooseVersionimport with a minimal shim class providing comparison operators (__lt__,__gt__,__eq__,__le__,__ge__) via regex-based version parsing.WAAgentUtil.pytry/exceptaround theimport impfallback so it doesn't crash on Python 3.12+ when the primaryimportlib.utilpath fails.HandlerUtil.pyplatform.dist()inget_dist_info()with/etc/os-releaseparsing (NAME+VERSIONfields).replace('\/', '/')was a no-op (invalid escape treated as literal); changed toreplace('\\/', '/').patch/__init__.py/etc/os-releaseIDparsing toDistInfo()fallback for distros whereplatform.linux_distribution()is gone.'azurelinux'mapping inGetMyPatching()→AzureLinuxPatching.patch/AzureLinuxPatching.py(new)/usr/bin/(AZL4 uses merged/usr).dnffor package installation.Testing
End-to-end Azure Backup on AZL4
Full backup job completed successfully (26 min, all phases: pre-snapshot, snapshot, post-snapshot).
Backward compatibility
All patches are no-ops on older Python versions (guarded by
try/exceptwith fallback to original imports). Verified across 15 distros (Python 2.7.5 – 3.14.3), including end-to-end Azure Backup on all Python 2 VMs: