fix(deployer_ui): seed config.json with the node the workspace targets (#131) - #147
fix(deployer_ui): seed config.json with the node the workspace targets (#131)#147pparage wants to merge 1 commit into
Conversation
The bundle rendered "defaultNodeName" from PROXMOX_NODE_NAME, defaulting to the literal "pve". No scenario call-site passes it, so every deployment seeded "pve" regardless of the node it had just deployed to. On dev_deployer_ui_lab the backend registers node_name "pve01" while the SPA seeds "pve" beside it, and the operator still has to correct the node by hand in Settings -- the exact retyping config.json exists to remove. Resolve the default from the active workspace vault's `proxmox_node` instead, loaded with the same vars_files pattern vm.bootstrap, the cloud-init downloads and the template builds already use. An explicit PROXMOX_NODE_NAME still wins, and "pve" survives only as the last-resort fallback when neither is set. Fixing it here rather than at the call-site keeps the lab's node out of an open-source scenario file and fixes every other scenario at the same time. Verified against the real workspace vault: nothing passed -> "pve01" (was "pve"), -e PROXMOX_NODE_NAME=pve99 -> "pve99", output parses as JSON. Scenario syntax-check green. Refs #131
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa3cc6dc4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| default_where: bundle-inline | ||
| default: "pve" | ||
| description: Proxmox node the backend targets, paired with BACKEND_API_URL in config.json. Only consumed when BACKEND_API_URL is set. | ||
| default: "proxmox_node from the workspace vault, else pve" |
There was a problem hiding this comment.
Keep prose out of the machine-readable default
When the catalog UI or another contract consumer materializes optional defaults, it will pass the literal value proxmox_node from the workspace vault, else pve as PROXMOX_NODE_NAME. The play then treats that supplied value as an explicit override at main.yml:149, so config.json receives the explanatory sentence instead of either the vault node or pve. The schema defines default as the effective default, so omit this field to let Ansible resolve it or encode a value the consumer can actually evaluate.
Useful? React with 👍 / 👎.
Part of #131 — the "feed the runtime variables to the containers at deploy, from the active workspace vault" half of the split.
The bug
config.jsonrendersdefaultNodeNamefromPROXMOX_NODE_NAME, defaulting to the literal"pve". No scenario call-site passes it, so every deployment seeds"pve"no matter which node it just deployed to.Caught on a live
dev_deployer_ui_labrun: the backend registers the Proxmox host asnode_name: "pve01", while the SPA sitting next to it seeds"pve". The operator still has to fix the node by hand in Settings — the exact retypingconfig.jsonwas added to remove, so the seeding fix was only half working.The fix
Resolve the default from the active workspace vault's
proxmox_node:The vault is loaded with the same
vars_filespatternproxmox/vm.bootstrap, thecloud_init_image.download.*bundles and thetemplate.build.*bundles already use, so this introduces no new convention.Fixed in the bundle rather than at the call-site on purpose: passing
pve01indev-deployer-ui.ymlwould bake a site-specific node into an open-source scenario, and would leave every other scenario broken.Verified
Against the real workspace vault, both branches:
defaultNodeNamedev_deployer_ui_labcase)pve01— waspve-e PROXMOX_NODE_NAME=pve99pve99— explicit override still winsOutput parses as JSON. Scenario
--syntax-checkgreen.Contract docs updated to match (header comment,
README.md,bundle_parameters.src.yml) andbundles/_tools/generate-bundle-params.pyre-run — it regenerates all 56 bundles but only this one changed.Pairs with
range42/range42-deployer-ui@ba20ba2 — the container was permanently
unhealthybecause theHEALTHCHECKcalledwget, which does not exist innginx:stable-bookworm. Already ondev.