Refactor/central substitutions #70
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: sls/hla_framework_bd#70
Reference in New Issue
Block a user
Centralizes every deploy-time substitution on one source of truth, and drops the ansible tree.
Previously the same values were defined in four independent places: sed in the gitea deploy script, ansible.builtin.replace in two playbooks, a hardcoded -DEV in bin/sls_hla_launch_gui.sh, and another in agebd/pv.py. Nothing kept
them in sync, and every failure mode was silent.
What this adds
the output.
Delimiters now separate the two stages
{{ token }} stays creation-time (copier). Deploy-time becomes @@token@@, across 89 files / 821 placeholders. This removes the {{ '{{ token }}' }} escaping from templates/. Angle brackets were tried first and rejected: << isn't
well-formed XML and cli parses qt/*.ui with ElementTree.
Bugs this surfaced and fixes
Ansible removal
ansible/ is deleted along with cli/src/ioc.py, core/ansible.py, IOC_ENV, the ansible deps, and docs/developer/ansible.md. This removes agebd ioc install|start|restart — pure playbook wrappers that couldn't outlive the playbooks.
IOC install/start/restart now has no automation path here.
Verification
Notes for review
Deploy-time placeholders were spread across four independent implementations: sed in the Gitea deploy script, ansible.builtin.replace in two playbooks, a hardcoded suffix in bin/sls_hla_launch_gui.sh, and another in agebd/pv.py. Each defined its own values, so nothing kept them in sync and a missed substitution failed silently. Step 1 -- one source of truth: * config/environments.yml declares every deploy-time token, its value per environment, and where tokens are allowed to live. An environment that is missing a token, or defines an undeclared one, is a load-time error. * agebd.subs loads it and renders. Rendering is a strict regex over declared token names rather than a template engine: the rendered files are EPICS db/substitution files and Qt XML, and an unknown token must raise rather than survive into the output. * scripts/ci/check_subs.py enforces the contract in CI: tokens in a rendered path are declared, declared tokens only appear where they are rendered or generated, and no pre-migration delimiters survive. Step 2 -- separate the two substitution stages by delimiter: * Creation-time (copier, once per new service) keeps {{ token }}. Deploy-time (every deploy, per environment) becomes @@token@@. * This drops the {{ '{{ token }}' }} escaping from templates/, which existed only because one Jinja pass had to emit syntax for a second. * Angle brackets were tried first and rejected: << >> is not well-formed XML, and cli parses qt/*.ui with ElementTree. * The existing sed and ansible consumers are repointed at the new delimiters so this stays behaviour-preserving. Routing them through agebd.subs is step 3. Also fixes, as a side effect of having one implementation: * The sed pattern '{{ *agebd_env_suffix_upper* }}' quantified 'r' rather than the space, so it only matched one exact spacing. * agebd.subs.read_ioc_port compares service names case-insensitively; ansible/tasks/utils/get-ioc-port.yml still compares CamelCase registry names against a lowercase input and never matches. qt/*.ui is declared as a render path but nothing renders it yet -- the Qt deploy step is still a plain rsync. Wiring that up is step 4. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Both deploy paths now take their token values from config/environments.yml instead of defining their own copies. CI / shell path: * scripts/ci/render_subs.py renders deployed files via agebd.subs. * deploy-and-restart-service.sh drops its dev/prod if-else and its four sed calls for one render_subs.py invocation, and aborts the deploy if rendering fails rather than restarting a service whose files still contain placeholders. Ansible path: * ansible/tasks/utils/load-substitutions.yml reads the central config on the controller and exposes the token table for this environment. * ioc-install and service-deploy-and-restart now loop over that table instead of listing tokens inline, so a new token needs no ansible edit. * env_suffix_upper/lower are gone from both inventories. The inventory says which environment it is; the central config says what that environment means. The facts are renamed to agebd_env_suffix_upper / _lower to match the token names, removing a rename trap. Ansible keeps using ansible.builtin.replace rather than calling agebd.subs, because ioc-install runs on hosts with no uv or Python environment for this repo. Only the mechanism differs now; the values and the token list are shared. Verified: both playbook paths run locally against dev and prod and produce the expected output, an undeclared environment fails loudly in both paths, and a misspelled token exits non-zero instead of shipping the placeholder. Not addressed here, still step 5: ansible/tasks/utils/get-ioc-port.yml matches CamelCase registry names against a lowercase input and returns nothing, so the ansible ioc-install path cannot resolve a port. This is pre-existing and unchanged by this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>qt/ was declared a render path in step 1 but nothing rendered it: the Qt deploy step was a plain rsync, so the UI files shipped with literal placeholders in every PV name. * deploy.yml renders the deployed qt/ directory with render_subs.py after the rsync. No --service is needed; qt holds no per-service token, and a file that acquired one would fail the step rather than ship the placeholder. * bin/sls_hla_launch_gui.sh no longer passes an AGEBD_ENV_SUFFIX macro. No .ui file has referenced $(AGEBD_ENV_SUFFIX) sincea06fded, and the suffix is now baked in at deploy time. This removes the last hardcoded '-DEV' outside config/environments.yml. check_ui_files.py is rewritten for the chosen convention. It asserted the old caQtDM macro, and its regex had stopped matching anything at all once placeholders appeared in device names -- against the pre-fix Service Manager it inspected 0 names, where the new check finds 16 violations. The new rule: every AGEBD- device name carries the suffix token, except systemd unit names, which are the same in both environments. The token name is read from the central config so a rename cannot leave this check enforcing a stale one. Fixing what that check surfaced, all in A_BD_ServiceManager.ui: * 16 'IOC=AGEBD-CPCL-<NAME>' caInclude macro values had no suffix. The included UI uses them as PV prefixes ($(IOC):STATUS, $(IOC):RESTART), so the dev Service Manager was reading status from, and sending restarts to, production IOCs. * Two ssh commands hardcoded sls-vserv-bd-hla01, so the dev GUI's restart button targeted the production HLA host. * cli generates those macro values, so the generator emits the token too and new services are no longer born with the bug. These were latent while qt/ shipped unrendered placeholders; turning rendering on would have made them live. Verified: dev renders -DEV device names and the -dev host, prod renders byte-identical to the currently deployed prod files, no placeholder survives in either, and all 24 UI files stay well-formed XML. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Consolidates the last duplicated derivations, and removes the ansible tree along with the CLI surface that existed only to drive it. Naming rules: * agebd/names.py holds ServiceName -- the single definition of how a service name is spelled in each context: dir_name, module_name, ioc_name, systemd_unit_name and the case forms. The environment suffix is deliberately not part of it; that stays a deploy-time token applied to these base names. * cli's Service model delegates every name_* property to it, so the CLI and the CI scripts derive names the same way. * scripts/ci/service_names.py emits the derived forms as shell assignments, and deploy-and-restart-service.sh evals them instead of carrying its own tr calls and its own copy of the systemd naming convention. Values are shell-quoted, and an invalid name exits non-zero emitting nothing, so an eval cannot leave empty variables behind. Registry lookups: * agebd.subs.find_service_entry is now the only place that answers "which registry entry is this service", used by read_ioc_port, get_ioc_port.py and cli's ServiceRegistry. get_ioc_port.py no longer parses the registry itself. * That also settles a semantic split: ServiceRegistry matched names case-sensitively while read_ioc_port did not. Both now match case-insensitively via ServiceName, so 'tune', 'Tune' and 'TUNE' resolve to the same service everywhere. Ansible removal: * ansible/ is deleted, along with cli/src/ioc.py, cli/src/core/ ansible.py, the IOC_ENV enum, the ansible and ansible-runner dependencies, and docs/developer/ansible.md. * This removes the 'agebd ioc install|start|restart' commands. They were pure playbook wrappers, so they could not outlive the playbooks; IOC install, start and restart now have no automation path in this repo. bin/start_service.sh keeps deriving its two name forms in bash, with a comment saying why: it runs on the deploy host before the service venv exists, so it cannot import agebd. It is the one deliberate exception. Verified: get_ioc_port.py, cli's ServiceRegistry and agebd's renderer all return 50007 for 'tune', 'Tune' and 'TUNE'; the shell-emitted names match the committed on-disk layout for real services including the dashed OpticsFF-X02S; the CLI runs with only the 'service' command left. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.