fix(pve, v1.2): repair Python virtual environment in single-node deployment#6131
fix(pve, v1.2): repair Python virtual environment in single-node deployment#6131xuang7 wants to merge 4 commits into
Conversation
Automated Reviewer SuggestionsBased on the
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release/v1.2 #6131 +/- ##
==================================================
- Coverage 53.60% 52.32% -1.29%
- Complexity 1440 2488 +1048
==================================================
Files 797 1074 +277
Lines 33193 42116 +8923
Branches 3324 4535 +1211
==================================================
+ Hits 17794 22036 +4242
- Misses 14534 18775 +4241
- Partials 865 1305 +440
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
v1.2 adaptation of b864745 on main (PR apache#6130): the frontend builds PVE URLs with AppSettings.getApiEndpoint(), so single-node nginx, the dev proxy, and the k8s gateway all become plain routes with no rewriting. Fewer files than on main because this branch predates the StoredPve db endpoints and the gateway chart reorganization. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| val deployPath = Paths.get("/tmp/system-requirements-lock.txt") | ||
| if (Files.exists(deployPath)) deployPath | ||
| else Paths.get("amber/system-requirements-lock.txt") |
There was a problem hiding this comment.
I don't think we should rely on "system-requirements-lock.txt" any more. we removed it in #5608
There was a problem hiding this comment.
This PR targets the release branch, where the lock files are currently present.
There was a problem hiding this comment.
ok. please annotate fix(pve, v1.2) in PR title so we know this is targeting v1.2
| location /api/pve/ { | ||
| proxy_pass http://workflow-runtime-coordinator-service:8085; | ||
| proxy_set_header Host $host; | ||
| proxy_set_header X-Real-IP $remote_addr; | ||
| } | ||
|
|
There was a problem hiding this comment.
this should be moved before /api
…tching Mirrors the same change on the main PR (apache#6130) to keep the two branches identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
99df8e4 to
f496f71
Compare
What changes were proposed in this PR?
Python Virtual Environments (PVE) are broken in the single-node deployment on
release/v1.2for two independent reasons, fixed together here:1. PVE REST calls returned 404 through single-node NGINX. The frontend requested
/pve/...while the backend (PveResource,@Path("/pve"), Jersey mounted under/api/*on the coordinator) serves/api/pve/..., so every gateway had to carry its own/pveto/api/pverewrite rule, and the single-node config was missing one. In the UI this surfaces as a "Could not load Python virtual environments" popup on every Python UDF operator selection.Mirroring #6130 on
main, this removes the mismatch at the source instead of adding a third rewrite rule: the frontend now builds PVE URLs withAppSettings.getApiEndpoint()and requests/api/pve/...directly, and every routing layer becomes a plain routing entry with no path rewriting:bin/single-node/nginx.conf:location /api/pve/passes through to the coordinatorfrontend/proxy.config.json:/api/pveentry (ordered before/api),pathRewriteremovedbin/k8s/templates/gateway-routes.yaml:/api/pvejoins the existing dynamic-backend match list, theURLRewritefilter is removedThis is an adapted port rather than a cherry-pick of the
maincommit: this branch predates the StoredPve db endpoints (/pve/db) and the gateway chart reorganization, so the same change lands in fewer files.2. The system-package list was empty even with routing fixed. The system-requirements lock file is copied to
/tmp/system-requirements-lock.txtin every deployed image (bin/computing-unit-master.dockerfile), butPveManager.getSystemPathchose its path from theisLocalflag (isLocal → amber/…, else/tmp/…). A containerized single-node run is also non-k8s, soisLocalistrueand it looked for the repo-relativeamber/system-requirements-lock.txt, which does not exist in the container → empty system-package set. The flag cannot distinguish a source-tree dev run from a containerized single-node run, so this probes for/tmp/system-requirements-lock.txtfirst and falls back to the repo-relative copy for dev runs.Note: part 2 is
release/v1.2-specific:mainhas a significantly refactoredPveManagerthat does not use the lock file.Any related issues, documentation, discussions?
Closes #5963. Ports #6130 (the
/api/pverouting cleanup onmain) torelease/v1.2.How was this PR tested?
On the single-node Docker stack (rc2 images + dashboard/coordinator rebuilt from this branch):
GET /api/pve/pves?cuid=1returns 200 JSON from the coordinator; the old/pvepath 404s to the dashboard catch-all as expected.getSystemPathbefore/after: the unmodified rc2 coordinator returns{"system":[]}, this branch's coordinator returns the full system-package list.Accepted, envoy routes/api/pveto the dynamic backend with no rewrite.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8, Claude Fable 5)