Repository navigation
fix(ansible): stage the SLM frontend publish and repair three backend path defects (#15557, #15560) - #15602
Conversation
|
Out-of-scope findings from this work, filed rather than folded in:
|
|
Pushed 1. Six new hardcoded path literals (SSOT Configuration Compliance)The new shared task files and the two playbooks carried Fixed by deriving each fallback from the SSOT (
An override of The pre-existing 2. The extraction hid a guarded git command from its checker
The pin now names the shared include, where the command actually lives. The distinction matters: the pin exists so that some file still demonstrates a guarded invocation to the structural walk — keeping it on This is the second guard in this PR that had to follow the behaviour rather than the text; |
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
|
Review pass — approved. No blocking findings; one informational, filed as #15610. Checked the five things most likely to be wrong in an extraction that consolidates four call sites.
The one finding — #15610The promotion is two sequential renames ( Not a regression from this PR — it is the pre-existing code lifted verbatim, and this PR is a behaviour-preserving extraction. But the exposure changed: the window existed at one entry point and now exists at four, precisely because making all four consistent is what this PR did. That is the cost of having got the consistency, and it should be recorded rather than left in a review comment. It is also much smaller than the defect that was fixed: #15557 was about a failed build leaving a broken site serving indefinitely, and the staging directory plus the non-empty- |
Closes #15557, #15560
Thinking Path
Both issues are the same defect, found in the same layer, at sites that sit
beside each other: an Ansible deploy path whose value or file location is
computed correctly in one place and restated — wrongly, or with no gate — in the
others. #15557 is four entry points that build the SLM frontend, one of which
received the staged-publish fix and three of which did not. #15560 is a symlink
target that does not depend on where the link lives, a systemd setting with two
writers, and a variable defined under a name no consumer reads. Every one of
them is "the same fact spelled out more than once, and the copies drifted".
That makes them one review, not two. The remedy is also shared: give each fact a
single writer, make every caller delegate to it, and put a guard over the
delegation with a floor under its own counts — because a fix applied to one of
four copies is exactly what produced both issues.
What Changed
#15557 — staged publish and a failure gate on every SLM-frontend entry point
New
autobot-slm-backend/ansible/roles/_shared/tasks/build_publish_slm_frontend.ymlholds the build-then-swap once: build into
dist.staging, fail the play on anon-zero rc with the build's own stderr, refuse to publish a bundle whose
index.htmlis missing or zero-length, rotate the live bundle todist.previous, then promote. Newroles/_shared/tasks/record_slm_deployed_commit.ymlholds the.deployed_commitwrite once, and fails loudly when the commit cannot be identified rather than
finishing a deploy anonymously.
The seam is an Ansible task file rather than a call into
services/slm_frontend_build.py: that module runs inside the SLM backend's ownvenv and resolves its paths from
SLM_DEPLOYED_ROOT, while three of the fourcallers are plays that run against an arbitrary target node before — or without
— that backend being able to serve a request.
update-node.ymlin particulardeploys to a node that has no SLM backend at all. A task file is the only seam
all four can reach. The Python path keeps its own copy of the same mechanism and
is recorded as such in the shared file's sweep note.
playbooks/update-all-nodes.yml:499roles/slm_manager/tasks/main.yml:806npm run build:slminto the serveddist/, no gateplaybooks/update-node.yml:189npm run build:slminto the serveddist/, no gateplaybooks/provision-fleet-roles.yml:523npm run buildinto the serveddist/, no gate.deployed_commitnow lands on all four.update-all-nodes.yml:358androles/slm_manager/tasks/main.yml:229had a copy each (and update-all-nodesneeded a second task to repair the ownership its shell redirect got wrong);
update-node.yml:200andprovision-fleet-roles.yml:539wrote no marker at all.update-node.ymldeploys from the manager's cache to a node with no checkout ofits own, so the commit is resolved on the manager (
update-node.yml:98) andhanded to the play that writes it.
Sweep (#15557 AC 6), recorded in the shared file's header: the four Ansible entry
points above plus
services/slm_frontend_build.pyare every SLM-frontend buildsite in the tree. The user-frontend build sites carry the same unstaged shape but
are a different component behind a different nginx site — filed separately rather
than folded in silently, along with two metadata files that describe an unstaged
build and one installer script that swallows a build failure outright.
#15560/1 —
ai-stacksymlink targets resolve relative to the linkroles/ai-stack/tasks/main.yml:259built eachsrc:as an absolute path underthe backend install directory, so the link re-anchored to the live tree under any
deploy-root relocation. Now
../../{{ backend_install_dir | basename }}/{{ item }}— the basename comes from the variable rather than being spelled out, so an
install that renames the backend component directory still resolves.
#15560/2 — one writer for the backend unit's PYTHONPATH
playbooks/fix-backend-environment.ymlrenderedroles/backend/templates/autobot-backend.service.j2and then rewrote the unit'sPYTHONPATHandEnvironmentFilewithlineinfilea few lines later. Bothhand-written lines are leftovers from before the template covered those settings
— that is the answer to the issue's first criterion. Removed; the template is now
the only writer. The play reads the value back out of the rendered unit
(
fix-backend-environment.yml:112-131) and the import test exports that(
:162), so the two cannot disagree. The play fails loudly if the rendered unitcarries no
PYTHONPATHrather than papering over it with a writer of its own.The
.envlocation checks now report against the location the template names,which is what the deployed unit on a live node actually carries.
#15560/3 —
backend_shared_dirwired in, not deletedsetup-user-backend.yml:46. The value was right and the name was wrong: rolesread
backend_shared_standalone_dir, whose default is a hardcoded path that doesnot follow the
backend_deployed_rootthis play declares — so the play's deployroot never reached the standalone
autobot_sharedcopy. Renamed to the consumedname. The omission was the bug.
Verification
Two new guards in
repo_tests/— CI's shard command passes an explicit path listthat
autobot-slm-backend/ansibleis not on, so a test beside the playbooks iscollected by a bare local pytest and by nothing that gates a merge.
repo_tests/slm_frontend_staged_publish_15557_test.py(12 tests) resolves eachentry point's include the way Ansible resolves it — a renamed shared file breaks
the check, which a substring match would not — and then sweeps every Ansible YAML
file for a task that still builds the SLM frontend itself, which is what catches
a fifth entry point arriving with a fresh copy. Floors: the entry-point set must
name 4 (a shrinking set is how a caller stops being checked), the file sweep must
reach 60 files, and the shared file must carry both fail gates, each with a
when:.test_the_inline_build_detector_discriminatesre-runs the detector overthe pre-#15557 task shape so a clean sweep means something.
Mutation-checked: reverting
update-node.ymlto an inlinenpm run build:slmfails 2 of the 12 — the delegation check and the inline-build sweep — with the
entry point named.
repo_tests/ansible_backend_path_defects_15560_test.py(27 tests) proves thesymlink property with real symlinks under a
tmp_pathroot that is guaranteednot to be the live tree, resolved with
os.path.realpath, in the style ofautobot_shared_symlink_relative_13539_test.py, plus the contrast mutationshowing the historical absolute form escapes that root. It asserts the backend
unit has exactly one PYTHONPATH writer and that the play still derives a value
from the rendered unit — the negative half alone would pass on a play that
exports nothing. The dead-variable check is general: every variable
setup-user-backend.ymldefines must be referenced somewhere in the Ansibletree, so the next dead one fails too. Floors on the loop item count, the play's
variable count, and the file sweep.
repo_tests/update_deps_detection_window_test.py— the #15430 guard asserted thestaged publish against
update-all-nodes.yml's own text. It now follows theinclude to wherever the logic lives and asserts the property there, with a floor
on the shared file's task count.
Local: 45 tests across the three guard modules pass; 83 tests in the seven other
repo_testsmodules that parse these playbooks and 63 in the six SLM-backendtest modules that do the same are unchanged and green. Full
pre-commitover thecommit range passes with no findings. No Ansible was executed and nothing was
installed — CI is the authority on the suite.
Model Used
Claude Opus 5 (1M context)