Repository navigation
fix(ansible): ensure_venv_version skips a venv whose interpreter is broken — the exact state an interpreter upgrade creates #17990
Description
Part of #13743. Found while gathering host evidence for #13747.
Summary
roles/python_interpreter/tasks/ensure_venv_version.yml removes a wrong-version venv only when the existing venv's interpreter still runs. When the venv exists but its interpreter is broken, the guard no-ops and the stale venv survives — and that is precisely the state an interpreter upgrade produces.
The npu-worker role's own comment states the opposite guarantee:
ensure_venv_versionbelow is what makes this safe on an already-deployed box: it detects the existing 3.11 venv as wrong-version and removes it, so the 3.11-built OpenVINO binaries in site-packages are never carried into a 3.14 venv (which would fail at import, not at deploy).
That guarantee does not hold in the case it is written for.
The chain, each step measured
The guard registers {{ venv_path }}/bin/python --version with failed_when: false, then gates removal on rc == 0:
when:
- _ensure_venv_python_version.rc == 0
- _ensure_venv_expected | trim not in _ensure_venv_python_version.stdoutThe header explains the rc != 0 branch as "Safe/idempotent when the venv is missing (rc != 0 → no-op)". But rc != 0 has two causes, and only one is safe:
rc != 0 because |
Correct action | What happens |
|---|---|---|
| venv is missing | no-op | correct |
| venv exists, its interpreter is gone | remove it | no-op — stale venv survives |
The second is the normal state after the host's old interpreter is removed: venv/bin/python is a symlink to a binary that no longer exists.
Step 1 — the guard no-ops. rc != 0, so both the debug and the file: state=absent task are skipped.
Step 2 — creates: does not save it. The create task is guarded by creates: "{{ npu_install_dir }}/venv/bin/python". Ansible's creates tests path existence, which follows symlinks, so a dangling symlink reads as absent and the task runs:
islink True
exists False <- what creates: tests
lexists True
Step 3 — and re-creating does not clean it. The task is python3.14 -m venv {{ npu_install_dir }}/venv, with no --clear. Measured on a scratch venv by planting a marker in site-packages:
before: 1
after plain re-venv: 1 <- survives
after --clear: 0
Result. A 3.14 venv whose site-packages still holds binaries built for the old interpreter. OpenVINO fails at import, not at deploy — which is the exact failure mode the comment says is prevented, and the reason #13747 asks for host verification rather than a green pip install.
Why it reads as clean
This is the MEASUREMENT_DISCIPLINE shape: rc != 0 means the measurement could not be taken, and the guard treats it as there is nothing to do. Nothing found and did not look are different results, and the comment documents only the first.
Proposed fix
Distinguish the two causes before acting:
- If the venv directory exists (
lexistsonbin/python, or astaton the directory) but the interpreter does not run, treat it as wrong-version and remove it. Unreadable is not clean. - Add
--clearto the create task so a surviving directory cannot carry oldsite-packagesforward even if the guard is bypassed. Defence in depth: the guard decides,--clearmakes the decision unrecoverable-proof. - Keep the genuine no-op only for "the directory does not exist at all".
Acceptance criteria
- A venv whose interpreter symlink dangles is removed by
ensure_venv_version, not skipped. - A venv directory that exists with no
bin/pythonat all is removed rather than skipped. - The create task uses
--clear, so stalesite-packagescannot survive a rebuild. - The genuine missing-venv case is still a silent no-op (no failure, no change).
- Verified by deliberate failure: a planted marker file in
site-packagesis absent after a rebuild, and present before. A test that only asserts the venv exists afterwards would pass without the fix.
Stated limits
Not reproduced against a real deployed host — the deployed worker here is already correctly on 3.14.6, so the broken-interpreter state does not currently exist on it. The chain above is demonstrated step by step (symlink semantics, creates: behaviour, venv without --clear) rather than observed end to end in production. All callers of ensure_venv_version share this guard, so the scope is every role that calls it, not only npu-worker.
Activity