From ee5f331f4c8a3e255f2d340e56babee554ec1db0 Mon Sep 17 00:00:00 2001 From: Benjamin Labrecque Date: Thu, 20 Aug 2026 12:12:11 +0200 Subject: [PATCH] feat: render qt/ at deploy time and enforce the suffix (step 4) 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) since a06fded, 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-' 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 --- .gitea/workflows/deploy.yml | 5 + bin/sls_hla_launch_gui.sh | 9 +- cli/src/models/service_manager_ui.py | 5 +- .../expected_output/qt/A_BD_ServiceManager.ui | 10 +- .../test_repo_root/qt/A_BD_ServiceManager.ui | 10 +- qt/A_BD_ServiceManager.ui | 10 +- scripts/ci/check_ui_files.py | 96 ++++++++++++++++--- scripts/tests/test_check_ui_files.py | 66 ++++++++++++- 8 files changed, 174 insertions(+), 37 deletions(-) diff --git a/.gitea/workflows/deploy.yml b/.gitea/workflows/deploy.yml index ef7e9ff..d211764 100644 --- a/.gitea/workflows/deploy.yml +++ b/.gitea/workflows/deploy.yml @@ -116,6 +116,11 @@ jobs: --exclude='.pytest_cache/' \ ./qt/* "${DEST_DIR}/" + # Resolve @@token@@ in the deployed UI files. qt/ holds no + # per-service token, so no --service is needed; a file that + # acquired one would fail here rather than ship the placeholder. + uv run --directory scripts ci/render_subs.py --env "${AGEBD_ENV}" "${DEST_DIR}" + # ----------------------------------------------------------------- # Services Detection, Testing, and Deployment # ----------------------------------------------------------------- diff --git a/bin/sls_hla_launch_gui.sh b/bin/sls_hla_launch_gui.sh index 88de269..84aa7bc 100755 --- a/bin/sls_hla_launch_gui.sh +++ b/bin/sls_hla_launch_gui.sh @@ -4,12 +4,13 @@ UI_FILENAME=$1 AGEBD_ENV=$2 # 'dev' for dev environment, any other string launches prod. REMAINING_ARGS=( "${@:3}" ) +# The environment suffix is baked into the UI files when qt/ is rendered at +# deploy time, from config/environments.yml. Launching from +# /sls/bd/hla/${AGEBD_ENV}/qt therefore already targets the right devices, so +# no AGEBD_ENV_SUFFIX macro is passed here. if [ "$AGEBD_ENV" = "dev" ]; then - AGEBD_ENV_SUFFIX="-DEV" EPICS_CA_ADDR_LIST="$EPICS_CA_ADDR_LIST 129.129.146.255" -else - AGEBD_ENV_SUFFIX="" fi export EPICS_CA_ADDR_LIST -caqtdm -macro "AGEBD_ENV_SUFFIX=${AGEBD_ENV_SUFFIX}" "${REMAINING_ARGS[@]}" "${UI_FILENAME}" +caqtdm "${REMAINING_ARGS[@]}" "${UI_FILENAME}" diff --git a/cli/src/models/service_manager_ui.py b/cli/src/models/service_manager_ui.py index 8dc3a45..4d64ebe 100644 --- a/cli/src/models/service_manager_ui.py +++ b/cli/src/models/service_manager_ui.py @@ -133,8 +133,11 @@ class _ServiceManagerUIMacroElement(BaseModel): ui_filename: str def to_string(self) -> str: + # The IOC macro is used as a PV prefix inside the included UI + # ($(IOC):STATUS, $(IOC):RESTART), so it carries the environment + # suffix token and is resolved when qt/ is rendered at deploy time. return ( - f"IOC=AGEBD-CPCL-{self.service_name_upper}," + f"IOC=AGEBD-CPCL-{self.service_name_upper}@@agebd_env_suffix_upper@@," f"service={self.service_name_upper}," f"name={self.service_name_camel}," f"panel={self.panel}," diff --git a/cli/tests/fixtures/expected_output/qt/A_BD_ServiceManager.ui b/cli/tests/fixtures/expected_output/qt/A_BD_ServiceManager.ui index 4260a44..812e708 100644 --- a/cli/tests/fixtures/expected_output/qt/A_BD_ServiceManager.ui +++ b/cli/tests/fixtures/expected_output/qt/A_BD_ServiceManager.ui @@ -138,7 +138,7 @@ caLabel[statusTip="Operation"]{ - IOC=AGEBD-CPCL-DBPM3CURR,service=DBPM3CURR,name=DBPM3Curr,panel=1,svc_exist=1,panelcmd=A_BD_DBPM3Curr.ui;IOC=AGEBD-CPCL-TAUBPM,service=TAUBPM,name=TauBPM,panel=1,svc_exist=1,panelcmd=A_BD_TauBPM.ui;IOC=AGEBD-CPCL-TAUPCT,service=TAUPCT,name=TauPCT,panel=1,svc_exist=1,panelcmd=A_BD_TauPCT.ui;IOC=AGEBD-CPCL-TIMING,service=TIMING,name=Timing,panel=1,svc_exist=1,panelcmd=A_BD_Timing.ui;IOC=AGEBD-CPCL-SCRUBBING,service=SCRUBBING,name=Scrubbing,panel=1,svc_exist=1,panelcmd=A_BD_Scrubbing.ui;IOC=AGEBD-CPCL-INJECTIONGUARD,service=INJECTIONGUARD,name=InjectionGuard,panel=1,svc_exist=1,panelcmd=A_BD_InjectionGuard.ui;IOC=AGEBD-CPCL-POSTMORTEMLOG,service=POSTMORTEMLOG,name=PostMortemLog,panel=1,svc_exist=1,panelcmd=A_BD_PostMortemLog.ui;IOC=AGEBD-CPCL-TUNEBUMP,service=TUNEBUMP,name=TuneBump,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-PLOTS,service=PLOTS,name=Plots,panel=0,svc_exist=1,panelcmd=A_BD_Plots.ui;IOC=AGEBD-CPCL-ORBITBUMP,service=ORBITBUMP,name=OrbitBump,panel=1,svc_exist=0,panelcmd=A_BD_OrbitBump.ui;IOC=AGEBD-CPCL-TUNEFBX,service=TUNEFBX,name=Hor. Tune Feedback,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-TUNEFBY,service=TUNEFBY,name=Ver. Tune Feedback,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-TESTSERVICE99-X00,service=TESTSERVICE99-X00,name=TestService99-X00,panel=1,svc_exist=1,panelcmd=A_BD_TestService99-X00.ui + IOC=AGEBD-CPCL-DBPM3CURR@@agebd_env_suffix_upper@@,service=DBPM3CURR,name=DBPM3Curr,panel=1,svc_exist=1,panelcmd=A_BD_DBPM3Curr.ui;IOC=AGEBD-CPCL-TAUBPM@@agebd_env_suffix_upper@@,service=TAUBPM,name=TauBPM,panel=1,svc_exist=1,panelcmd=A_BD_TauBPM.ui;IOC=AGEBD-CPCL-TAUPCT@@agebd_env_suffix_upper@@,service=TAUPCT,name=TauPCT,panel=1,svc_exist=1,panelcmd=A_BD_TauPCT.ui;IOC=AGEBD-CPCL-TIMING@@agebd_env_suffix_upper@@,service=TIMING,name=Timing,panel=1,svc_exist=1,panelcmd=A_BD_Timing.ui;IOC=AGEBD-CPCL-SCRUBBING@@agebd_env_suffix_upper@@,service=SCRUBBING,name=Scrubbing,panel=1,svc_exist=1,panelcmd=A_BD_Scrubbing.ui;IOC=AGEBD-CPCL-INJECTIONGUARD@@agebd_env_suffix_upper@@,service=INJECTIONGUARD,name=InjectionGuard,panel=1,svc_exist=1,panelcmd=A_BD_InjectionGuard.ui;IOC=AGEBD-CPCL-POSTMORTEMLOG@@agebd_env_suffix_upper@@,service=POSTMORTEMLOG,name=PostMortemLog,panel=1,svc_exist=1,panelcmd=A_BD_PostMortemLog.ui;IOC=AGEBD-CPCL-TUNEBUMP@@agebd_env_suffix_upper@@,service=TUNEBUMP,name=TuneBump,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-PLOTS@@agebd_env_suffix_upper@@,service=PLOTS,name=Plots,panel=0,svc_exist=1,panelcmd=A_BD_Plots.ui;IOC=AGEBD-CPCL-ORBITBUMP@@agebd_env_suffix_upper@@,service=ORBITBUMP,name=OrbitBump,panel=1,svc_exist=0,panelcmd=A_BD_OrbitBump.ui;IOC=AGEBD-CPCL-TUNEFBX@@agebd_env_suffix_upper@@,service=TUNEFBX,name=Hor. Tune Feedback,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-TUNEFBY@@agebd_env_suffix_upper@@,service=TUNEFBY,name=Ver. Tune Feedback,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-TESTSERVICE99-X00@@agebd_env_suffix_upper@@,service=TESTSERVICE99-X00,name=TestService99-X00,panel=1,svc_exist=1,panelcmd=A_BD_TestService99-X00.ui A_BD_ServiceManager_inc.ui @@ -246,7 +246,7 @@ caLabel[statusTip="Operation"]{ - IOC=AGEBD-CPCL-NTURNS,service=NTURNS,name=NTurns,panel=1,svc_exist=1,panelcmd=A_BD_NTurns.ui;IOC=AGEBD-CPCL-TOPUPTOOL,service=TOPUPTOOL,name=TopUpTool,panel=1,svc_exist=1,panelcmd=A_BD_TopUpTool.ui;IOC=AGEBD-CPCL-TUNE,service=TUNE,name=Tune,panel=1,svc_exist=1,panelcmd=A_BD_Tune.ui;IOC=AGEOP-CPCL-SHIFTTOOL,service=SHIFTTOOL,name=Shifttool,panel=1,svc_exist=1,panelcmd=A_OP_SchichtProtokollTool.ui,macro=FACILITY=SLS + IOC=AGEBD-CPCL-NTURNS@@agebd_env_suffix_upper@@,service=NTURNS,name=NTurns,panel=1,svc_exist=1,panelcmd=A_BD_NTurns.ui;IOC=AGEBD-CPCL-TOPUPTOOL@@agebd_env_suffix_upper@@,service=TOPUPTOOL,name=TopUpTool,panel=1,svc_exist=1,panelcmd=A_BD_TopUpTool.ui;IOC=AGEBD-CPCL-TUNE@@agebd_env_suffix_upper@@,service=TUNE,name=Tune,panel=1,svc_exist=1,panelcmd=A_BD_Tune.ui;IOC=AGEOP-CPCL-SHIFTTOOL,service=SHIFTTOOL,name=Shifttool,panel=1,svc_exist=1,panelcmd=A_OP_SchichtProtokollTool.ui,macro=FACILITY=SLS A_BD_ServiceManager_inc.ui @@ -792,7 +792,7 @@ caLabel[statusTip="Operation"]{ /ioc/modules/qt/iocStats.ui - IOC=AGEBD-CPCL-MASTER; + IOC=AGEBD-CPCL-MASTER@@agebd_env_suffix_upper@@; @@ -986,7 +986,7 @@ caLabel[statusTip="Operation"]{ false - ssh svcusr-sls2hla@sls-vserv-bd-hla01 systemctl --user restart AGEBD-SERVICE-MASTER.service + ssh svcusr-sls2hla@sls-vserv-bd-hla01@@agebd_env_suffix_lower@@ systemctl --user restart AGEBD-SERVICE-MASTER.service @@ -1015,7 +1015,7 @@ caLabel[statusTip="Operation"]{ false - terminator -e "ssh svcusr-sls2hla@sls-vserv-bd-hla01" + terminator -e "ssh svcusr-sls2hla@sls-vserv-bd-hla01@@agebd_env_suffix_lower@@" diff --git a/cli/tests/fixtures/test_repo_root/qt/A_BD_ServiceManager.ui b/cli/tests/fixtures/test_repo_root/qt/A_BD_ServiceManager.ui index d2c3936..39cf352 100644 --- a/cli/tests/fixtures/test_repo_root/qt/A_BD_ServiceManager.ui +++ b/cli/tests/fixtures/test_repo_root/qt/A_BD_ServiceManager.ui @@ -138,7 +138,7 @@ caLabel[statusTip="Operation"]{ - IOC=AGEBD-CPCL-DBPM3CURR,service=DBPM3CURR,name=DBPM3Curr,panel=1,svc_exist=1,panelcmd=A_BD_DBPM3Curr.ui;IOC=AGEBD-CPCL-TAUBPM,service=TAUBPM,name=TauBPM,panel=1,svc_exist=1,panelcmd=A_BD_TauBPM.ui;IOC=AGEBD-CPCL-TAUPCT,service=TAUPCT,name=TauPCT,panel=1,svc_exist=1,panelcmd=A_BD_TauPCT.ui;IOC=AGEBD-CPCL-TIMING,service=TIMING,name=Timing,panel=1,svc_exist=1,panelcmd=A_BD_Timing.ui;IOC=AGEBD-CPCL-SCRUBBING,service=SCRUBBING,name=Scrubbing,panel=1,svc_exist=1,panelcmd=A_BD_Scrubbing.ui;IOC=AGEBD-CPCL-INJECTIONGUARD,service=INJECTIONGUARD,name=InjectionGuard,panel=1,svc_exist=1,panelcmd=A_BD_InjectionGuard.ui;IOC=AGEBD-CPCL-POSTMORTEMLOG,service=POSTMORTEMLOG,name=PostMortemLog,panel=1,svc_exist=1,panelcmd=A_BD_PostMortemLog.ui;IOC=AGEBD-CPCL-TUNEBUMP,service=TUNEBUMP,name=TuneBump,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-PLOTS,service=PLOTS,name=Plots,panel=0,svc_exist=1,panelcmd=A_BD_Plots.ui;IOC=AGEBD-CPCL-ORBITBUMP,service=ORBITBUMP,name=OrbitBump,panel=1,svc_exist=0,panelcmd=A_BD_OrbitBump.ui;IOC=AGEBD-CPCL-TUNEFBX,service=TUNEFBX,name=Hor. Tune Feedback,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-TUNEFBY,service=TUNEFBY,name=Ver. Tune Feedback,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui + IOC=AGEBD-CPCL-DBPM3CURR@@agebd_env_suffix_upper@@,service=DBPM3CURR,name=DBPM3Curr,panel=1,svc_exist=1,panelcmd=A_BD_DBPM3Curr.ui;IOC=AGEBD-CPCL-TAUBPM@@agebd_env_suffix_upper@@,service=TAUBPM,name=TauBPM,panel=1,svc_exist=1,panelcmd=A_BD_TauBPM.ui;IOC=AGEBD-CPCL-TAUPCT@@agebd_env_suffix_upper@@,service=TAUPCT,name=TauPCT,panel=1,svc_exist=1,panelcmd=A_BD_TauPCT.ui;IOC=AGEBD-CPCL-TIMING@@agebd_env_suffix_upper@@,service=TIMING,name=Timing,panel=1,svc_exist=1,panelcmd=A_BD_Timing.ui;IOC=AGEBD-CPCL-SCRUBBING@@agebd_env_suffix_upper@@,service=SCRUBBING,name=Scrubbing,panel=1,svc_exist=1,panelcmd=A_BD_Scrubbing.ui;IOC=AGEBD-CPCL-INJECTIONGUARD@@agebd_env_suffix_upper@@,service=INJECTIONGUARD,name=InjectionGuard,panel=1,svc_exist=1,panelcmd=A_BD_InjectionGuard.ui;IOC=AGEBD-CPCL-POSTMORTEMLOG@@agebd_env_suffix_upper@@,service=POSTMORTEMLOG,name=PostMortemLog,panel=1,svc_exist=1,panelcmd=A_BD_PostMortemLog.ui;IOC=AGEBD-CPCL-TUNEBUMP@@agebd_env_suffix_upper@@,service=TUNEBUMP,name=TuneBump,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-PLOTS@@agebd_env_suffix_upper@@,service=PLOTS,name=Plots,panel=0,svc_exist=1,panelcmd=A_BD_Plots.ui;IOC=AGEBD-CPCL-ORBITBUMP@@agebd_env_suffix_upper@@,service=ORBITBUMP,name=OrbitBump,panel=1,svc_exist=0,panelcmd=A_BD_OrbitBump.ui;IOC=AGEBD-CPCL-TUNEFBX@@agebd_env_suffix_upper@@,service=TUNEFBX,name=Hor. Tune Feedback,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-TUNEFBY@@agebd_env_suffix_upper@@,service=TUNEFBY,name=Ver. Tune Feedback,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui A_BD_ServiceManager_inc.ui @@ -246,7 +246,7 @@ caLabel[statusTip="Operation"]{ - IOC=AGEBD-CPCL-NTURNS,service=NTURNS,name=NTurns,panel=1,svc_exist=1,panelcmd=A_BD_NTurns.ui;IOC=AGEBD-CPCL-TOPUPTOOL,service=TOPUPTOOL,name=TopUpTool,panel=1,svc_exist=1,panelcmd=A_BD_TopUpTool.ui;IOC=AGEBD-CPCL-TUNE,service=TUNE,name=Tune,panel=1,svc_exist=1,panelcmd=A_BD_Tune.ui;IOC=AGEOP-CPCL-SHIFTTOOL,service=SHIFTTOOL,name=Shifttool,panel=1,svc_exist=1,panelcmd=A_OP_SchichtProtokollTool.ui,macro=FACILITY=SLS + IOC=AGEBD-CPCL-NTURNS@@agebd_env_suffix_upper@@,service=NTURNS,name=NTurns,panel=1,svc_exist=1,panelcmd=A_BD_NTurns.ui;IOC=AGEBD-CPCL-TOPUPTOOL@@agebd_env_suffix_upper@@,service=TOPUPTOOL,name=TopUpTool,panel=1,svc_exist=1,panelcmd=A_BD_TopUpTool.ui;IOC=AGEBD-CPCL-TUNE@@agebd_env_suffix_upper@@,service=TUNE,name=Tune,panel=1,svc_exist=1,panelcmd=A_BD_Tune.ui;IOC=AGEOP-CPCL-SHIFTTOOL,service=SHIFTTOOL,name=Shifttool,panel=1,svc_exist=1,panelcmd=A_OP_SchichtProtokollTool.ui,macro=FACILITY=SLS A_BD_ServiceManager_inc.ui @@ -792,7 +792,7 @@ caLabel[statusTip="Operation"]{ /ioc/modules/qt/iocStats.ui - IOC=AGEBD-CPCL-MASTER; + IOC=AGEBD-CPCL-MASTER@@agebd_env_suffix_upper@@; @@ -986,7 +986,7 @@ caLabel[statusTip="Operation"]{ false - ssh svcusr-sls2hla@sls-vserv-bd-hla01 systemctl --user restart AGEBD-SERVICE-MASTER.service + ssh svcusr-sls2hla@sls-vserv-bd-hla01@@agebd_env_suffix_lower@@ systemctl --user restart AGEBD-SERVICE-MASTER.service @@ -1015,7 +1015,7 @@ caLabel[statusTip="Operation"]{ false - terminator -e "ssh svcusr-sls2hla@sls-vserv-bd-hla01" + terminator -e "ssh svcusr-sls2hla@sls-vserv-bd-hla01@@agebd_env_suffix_lower@@" diff --git a/qt/A_BD_ServiceManager.ui b/qt/A_BD_ServiceManager.ui index 3539295..70c6b15 100644 --- a/qt/A_BD_ServiceManager.ui +++ b/qt/A_BD_ServiceManager.ui @@ -138,7 +138,7 @@ caLabel[statusTip="Operation"]{ - IOC=AGEBD-CPCL-DBPM3CURR,service=DBPM3CURR,name=DBPM3Curr,panel=1,svc_exist=1,panelcmd=A_BD_DBPM3Curr.ui;IOC=AGEBD-CPCL-TAUBPM,service=TAUBPM,name=TauBPM,panel=1,svc_exist=1,panelcmd=A_BD_TauBPM.ui;IOC=AGEBD-CPCL-TAUPCT,service=TAUPCT,name=TauPCT,panel=1,svc_exist=1,panelcmd=A_BD_TauPCT.ui;IOC=AGEBD-CPCL-TIMING,service=TIMING,name=Timing,panel=1,svc_exist=1,panelcmd=A_BD_Timing.ui;IOC=AGEBD-CPCL-SCRUBBING,service=SCRUBBING,name=Scrubbing,panel=1,svc_exist=1,panelcmd=A_BD_Scrubbing.ui;IOC=AGEBD-CPCL-INJECTIONGUARD,service=INJECTIONGUARD,name=InjectionGuard,panel=1,svc_exist=1,panelcmd=A_BD_InjectionGuard.ui;IOC=AGEBD-CPCL-POSTMORTEMLOG,service=POSTMORTEMLOG,name=PostMortemLog,panel=1,svc_exist=1,panelcmd=A_BD_PostMortemLog.ui;IOC=AGEBD-CPCL-TUNEBUMP,service=TUNEBUMP,name=TuneBump,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-PLOTS,service=PLOTS,name=Plots,panel=0,svc_exist=1,panelcmd=A_BD_Plots.ui;IOC=AGEBD-CPCL-ORBITBUMP,service=ORBITBUMP,name=OrbitBump,panel=1,svc_exist=0,panelcmd=A_BD_OrbitBump.ui;IOC=AGEBD-CPCL-TUNEFBX,service=TUNEFBX,name=TuneFBx,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-TUNEFBY,service=TUNEFBY,name=TuneFBy,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui + IOC=AGEBD-CPCL-DBPM3CURR@@agebd_env_suffix_upper@@,service=DBPM3CURR,name=DBPM3Curr,panel=1,svc_exist=1,panelcmd=A_BD_DBPM3Curr.ui;IOC=AGEBD-CPCL-TAUBPM@@agebd_env_suffix_upper@@,service=TAUBPM,name=TauBPM,panel=1,svc_exist=1,panelcmd=A_BD_TauBPM.ui;IOC=AGEBD-CPCL-TAUPCT@@agebd_env_suffix_upper@@,service=TAUPCT,name=TauPCT,panel=1,svc_exist=1,panelcmd=A_BD_TauPCT.ui;IOC=AGEBD-CPCL-TIMING@@agebd_env_suffix_upper@@,service=TIMING,name=Timing,panel=1,svc_exist=1,panelcmd=A_BD_Timing.ui;IOC=AGEBD-CPCL-SCRUBBING@@agebd_env_suffix_upper@@,service=SCRUBBING,name=Scrubbing,panel=1,svc_exist=1,panelcmd=A_BD_Scrubbing.ui;IOC=AGEBD-CPCL-INJECTIONGUARD@@agebd_env_suffix_upper@@,service=INJECTIONGUARD,name=InjectionGuard,panel=1,svc_exist=1,panelcmd=A_BD_InjectionGuard.ui;IOC=AGEBD-CPCL-POSTMORTEMLOG@@agebd_env_suffix_upper@@,service=POSTMORTEMLOG,name=PostMortemLog,panel=1,svc_exist=1,panelcmd=A_BD_PostMortemLog.ui;IOC=AGEBD-CPCL-TUNEBUMP@@agebd_env_suffix_upper@@,service=TUNEBUMP,name=TuneBump,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-PLOTS@@agebd_env_suffix_upper@@,service=PLOTS,name=Plots,panel=0,svc_exist=1,panelcmd=A_BD_Plots.ui;IOC=AGEBD-CPCL-ORBITBUMP@@agebd_env_suffix_upper@@,service=ORBITBUMP,name=OrbitBump,panel=1,svc_exist=0,panelcmd=A_BD_OrbitBump.ui;IOC=AGEBD-CPCL-TUNEFBX@@agebd_env_suffix_upper@@,service=TUNEFBX,name=TuneFBx,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui;IOC=AGEBD-CPCL-TUNEFBY@@agebd_env_suffix_upper@@,service=TUNEFBY,name=TuneFBy,panel=1,svc_exist=1,panelcmd=A_BD_TuneBump.ui A_BD_ServiceManager_inc.ui @@ -246,7 +246,7 @@ caLabel[statusTip="Operation"]{ - IOC=AGEBD-CPCL-NTURNS,service=NTURNS,name=NTurns,panel=1,svc_exist=1,panelcmd=A_BD_NTurns.ui;IOC=AGEBD-CPCL-TOPUPTOOL,service=TOPUPTOOL,name=TopUpTool,panel=1,svc_exist=1,panelcmd=A_BD_TopUpTool.ui;IOC=AGEBD-CPCL-TUNE,service=TUNE,name=Tune,panel=1,svc_exist=1,panelcmd=A_BD_Tune.ui;IOC=AGEOP-CPCL-SHIFTTOOL,service=SHIFTTOOL,name=Shifttool,panel=1,svc_exist=1,panelcmd=A_OP_SchichtProtokollTool.ui,macro=FACILITY=SLS + IOC=AGEBD-CPCL-NTURNS@@agebd_env_suffix_upper@@,service=NTURNS,name=NTurns,panel=1,svc_exist=1,panelcmd=A_BD_NTurns.ui;IOC=AGEBD-CPCL-TOPUPTOOL@@agebd_env_suffix_upper@@,service=TOPUPTOOL,name=TopUpTool,panel=1,svc_exist=1,panelcmd=A_BD_TopUpTool.ui;IOC=AGEBD-CPCL-TUNE@@agebd_env_suffix_upper@@,service=TUNE,name=Tune,panel=1,svc_exist=1,panelcmd=A_BD_Tune.ui;IOC=AGEOP-CPCL-SHIFTTOOL,service=SHIFTTOOL,name=Shifttool,panel=1,svc_exist=1,panelcmd=A_OP_SchichtProtokollTool.ui,macro=FACILITY=SLS A_BD_ServiceManager_inc.ui @@ -792,7 +792,7 @@ caLabel[statusTip="Operation"]{ /ioc/modules/qt/iocStats.ui - IOC=AGEBD-CPCL-MASTER; + IOC=AGEBD-CPCL-MASTER@@agebd_env_suffix_upper@@; @@ -986,7 +986,7 @@ caLabel[statusTip="Operation"]{ false - ssh svcusr-sls2hla@sls-vserv-bd-hla01 systemctl --user restart AGEBD-SERVICE-MASTER.service + ssh svcusr-sls2hla@sls-vserv-bd-hla01@@agebd_env_suffix_lower@@ systemctl --user restart AGEBD-SERVICE-MASTER.service @@ -1015,7 +1015,7 @@ caLabel[statusTip="Operation"]{ false - terminator -e "ssh svcusr-sls2hla@sls-vserv-bd-hla01" + terminator -e "ssh svcusr-sls2hla@sls-vserv-bd-hla01@@agebd_env_suffix_lower@@" diff --git a/scripts/ci/check_ui_files.py b/scripts/ci/check_ui_files.py index 6b7a9c2..1b00d52 100644 --- a/scripts/ci/check_ui_files.py +++ b/scripts/ci/check_ui_files.py @@ -1,25 +1,91 @@ -import re +""" +Guard the environment suffix in Qt UI files. +qt/ is rendered at deploy time (see config/environments.yml), so every EPICS +device name in a .ui file must carry the suffix token. A name without it +resolves to the production device in every environment -- which is how the +dev Service Manager ended up driving prod IOCs. + +The rule is: every AGEBD- name is immediately followed by the suffix token. +The one exception is systemd unit names, which genuinely have no environment +suffix (the unit file is AGEBD-SERVICE-.service in both environments). + +This replaces an earlier check for the caQtDM macro '$(AGEBD_ENV_SUFFIX)', +which was abandoned in favour of deploy-time rendering. That check had also +stopped matching anything once placeholders appeared in the names. +""" + +import re +import sys + +from agebd.subs import SubstitutionsConfig from agebd.utils import get_git_root REPO_ROOT = get_git_root(__file__) QT_DIR = REPO_ROOT / "qt" +SUFFIX_TOKEN_NAME = "agebd_env_suffix_upper" + +#: An EPICS device name belonging to us, e.g. AGEBD-ALH or AGEBD-CPCL-TUNE. +AGEBD_NAME = re.compile(r"AGEBD-[0-9A-Z][0-9A-Z-]*") + +#: Systemd unit names are the same in every environment. +EXEMPT_PREFIXES = ("AGEBD-SERVICE-",) + + +def suffix_token(config): + """ + The token text these files must carry, taken from the central config so + that renaming the token cannot leave this check enforcing a stale name. + """ + if SUFFIX_TOKEN_NAME not in config.tokens: + raise AssertionError( + f"Token '{SUFFIX_TOKEN_NAME}' is not declared in " + f"{config.environments_file}, but qt/ is checked against it." + ) + return f"@@{SUFFIX_TOKEN_NAME}@@" + + +def check_ui_file(text, token, filename): + """ + Report every AGEBD- device name in ``text`` that lacks ``token``. + """ + errors = [] + for match in AGEBD_NAME.finditer(text): + name = match.group(0) + if name.startswith(EXEMPT_PREFIXES): + continue + if text[match.end() : match.end() + len(token)] == token: + continue + line = text.count("\n", 0, match.start()) + 1 + errors.append( + f"{filename}:{line}: '{name}' is missing '{token}', so it resolves to the " + f"production device in every environment." + ) + return errors + def check_ui_files(): """ - All 'AGEBD-' PVs need to contain the '$(AGEBD_ENV_SUFFIX)' macro, - which is: - - '' in PROD - '-DEV' in DEV + Raise AssertionError listing every violation, or return silently. """ - for filename in QT_DIR.glob("*"): - with open(filename, "r") as f: - for linenum, line in enumerate(f): - matches = re.findall("AGEBD-[0-9A-Z$()_]*:", line) - if matches: - for match in matches: - assert "$(AGEBD_ENV_SUFFIX)" in match, ( - f"Missing $(AGEBD_ENV_SUFFIX) in {filename} in line number {linenum + 1}: {line} " - ) + config = SubstitutionsConfig.load(REPO_ROOT / "config") + token = suffix_token(config) + + errors = [] + for path in sorted(QT_DIR.glob("*.ui")): + errors.extend(check_ui_file(path.read_text(), token, path.relative_to(REPO_ROOT))) + + if errors: + raise AssertionError( + "Qt UI files missing the environment suffix:\n\n " + "\n ".join(errors) + ) + + +if __name__ == "__main__": + try: + check_ui_files() + except AssertionError as e: + print(e, file=sys.stderr) + sys.exit(1) + print("All AGEBD- device names in qt/ carry the environment suffix.") diff --git a/scripts/tests/test_check_ui_files.py b/scripts/tests/test_check_ui_files.py index 6dc6793..3ef1a22 100644 --- a/scripts/tests/test_check_ui_files.py +++ b/scripts/tests/test_check_ui_files.py @@ -1,5 +1,67 @@ -from ci.check_ui_files import check_ui_files +import pytest +from agebd.subs import SubstitutionsConfig +from agebd.utils import get_git_root + +from ci.check_ui_files import check_ui_file, check_ui_files, suffix_token + +REPO_ROOT = get_git_root(__file__) +TOKEN = "@@agebd_env_suffix_upper@@" -def test_check_ui_files(): +@pytest.fixture +def token(): + return suffix_token(SubstitutionsConfig.load(REPO_ROOT / "config")) + + +def test_repo_ui_files_all_carry_the_suffix(): check_ui_files() + + +def test_token_comes_from_the_central_config(token): + assert token == TOKEN + + +def test_pv_name_with_the_suffix_is_accepted(token): + text = f'AGEBD-ALH{TOKEN}:NTURNS-ONOFF' + + assert check_ui_file(text, token, "A_BD_NTurns.ui") == [] + + +def test_pv_name_without_the_suffix_is_reported(token): + text = 'AGEBD-ALH:NTURNS-ONOFF' + + errors = check_ui_file(text, token, "A_BD_NTurns.ui") + + assert len(errors) == 1 + assert "AGEBD-ALH" in errors[0] + assert "A_BD_NTurns.ui:1" in errors[0] + + +def test_ioc_macro_value_without_the_suffix_is_reported(token): + """This is the bug that had the dev Service Manager driving prod IOCs.""" + text = "IOC=AGEBD-CPCL-NTURNS,service=NTURNS,name=NTurns" + + errors = check_ui_file(text, token, "A_BD_ServiceManager.ui") + + assert len(errors) == 1 + assert "AGEBD-CPCL-NTURNS" in errors[0] + + +def test_systemd_unit_name_is_exempt(token): + text = "systemctl --user restart AGEBD-SERVICE-MASTER.service" + + assert check_ui_file(text, token, "A_BD_ServiceManager.ui") == [] + + +def test_other_facilities_are_not_our_business(token): + text = "IOC=AGEOP-CPCL-SHIFTTOOL,service=SHIFTTOOL" + + assert check_ui_file(text, token, "A_BD_ServiceManager.ui") == [] + + +def test_reports_the_line_number(token): + text = "line one\nline two\nAGEBD-TUNE:AO\n" + + errors = check_ui_file(text, token, "A_BD_Tune.ui") + + assert "A_BD_Tune.ui:3" in errors[0]