diff --git a/ansible/roles/llm-inference-multimodel/handlers/main.yml b/ansible/roles/llm-inference-multimodel/handlers/main.yml index d61f023..b23b3cb 100644 --- a/ansible/roles/llm-inference-multimodel/handlers/main.yml +++ b/ansible/roles/llm-inference-multimodel/handlers/main.yml @@ -1,9 +1,21 @@ --- # ------------------------------------------------------------------------------ # FILE: roles/llm-inference-multimodel/handlers/main.yml -# DESCRIPTION: Separate restart handlers per instance — NEVER combined, so a -# content change to one unit template never restarts the other -# (plan §2/§6 requirement: independent restart/rollback). +# DESCRIPTION: Only a daemon-reload handler lives here now (harmless, no +# process impact). Per-service restart/start decisions are made +# explicitly in tasks/verify.yml (Phase 4), keyed off the +# per-unit `changed` result registered in tasks/systemd.yml +# (Phase 2) — NEVER combined, so a content change to one unit +# template still never restarts the other (plan §2/§6 +# requirement: independent restart/rollback). +# +# BUGFIX: this file used to also define "restart +# llama-server-aux" / "restart llama-server-toolcall" handlers, +# notified from Phase 2's template tasks and fired there via +# `meta: flush_handlers` — causing both live services to +# restart during Phase 2, before Phase 3/4 had run. See +# tasks/systemd.yml for the full writeup. Restart logic moved +# to tasks/verify.yml so it only ever fires in Phase 4. # ------------------------------------------------------------------------------ - name: Reload systemd @@ -11,17 +23,3 @@ daemon_reload: true become: true listen: "reload systemd" - -- name: Restart llama-server-aux - ansible.builtin.systemd: - name: "{{ llm_aux_service_name }}" - state: restarted - become: true - listen: "restart llama-server-aux" - -- name: Restart llama-server-toolcall - ansible.builtin.systemd: - name: "{{ llm_toolcall_service_name }}" - state: restarted - become: true - listen: "restart llama-server-toolcall" diff --git a/ansible/roles/llm-inference-multimodel/tasks/systemd.yml b/ansible/roles/llm-inference-multimodel/tasks/systemd.yml index 70ef26d..ba95cdf 100644 --- a/ansible/roles/llm-inference-multimodel/tasks/systemd.yml +++ b/ansible/roles/llm-inference-multimodel/tasks/systemd.yml @@ -25,9 +25,9 @@ group: root mode: "0644" become: true + register: llm_aux_unit_deployed notify: - reload systemd - - restart llama-server-aux - name: Deploy llama-server-toolcall systemd unit ansible.builtin.template: @@ -37,18 +37,34 @@ group: root mode: "0644" become: true + register: llm_toolcall_unit_deployed notify: - reload systemd - - restart llama-server-toolcall - name: Flush handlers so daemon-reload lands before any later phase acts on unit state ansible.builtin.meta: flush_handlers -# NOTE: no `ansible.builtin.systemd: state: started / enabled: true` task here -# on purpose. Units exist on disk after this phase; nothing is running. -# The "restart" handlers above only fire (and thus only start anything) if -# the template content actually changed AND a later flush_handlers/end-of-play -# triggers them — on a first-ever apply this DOES start the services once, -# which is expected/acceptable for a fresh deploy, but on any subsequent -# re-run with no template changes, nothing restarts. Ryan/verify.yml owns -# the deliberate first start + smoke test. +# NOTE: no `ansible.builtin.systemd: state: started / enabled: true / restarted` +# task here on purpose. Units exist on disk after this phase; nothing is +# running or restarted. +# +# BUGFIX (found in production): this file used to `notify: restart +# llama-server-*` on the template tasks above, followed by the +# flush_handlers meta task. That combination meant Phase 2 (the `systemd` +# tag) fired the restart handlers itself — on any run where either unit's +# rendered content changed (including the very first apply), BOTH services +# got restarted immediately, right here in Phase 2, before Phase 3's +# firewall scoping or Phase 4's smoke tests ever ran. That directly +# contradicted this file's own stated purpose (units land on disk, nothing +# starts/restarts until Phase 4) and caused live services to bounce +# unexpectedly on a routine re-run of just `--tags systemd`. +# +# Fix: Phase 2 only reloads the systemd daemon (harmless, no process +# impact) and records whether each unit's content actually changed via +# `llm_aux_unit_deployed` / `llm_toolcall_unit_deployed` (both `.changed` +# booleans, persisted as play vars for later phases in this same run). +# Phase 4 (verify.yml) is the only phase that starts OR restarts either +# service, and it does so per-instance using those recorded `changed` +# flags — so a content change to one unit's template still never causes +# the other to restart, and no restart happens at all until Phase 4 has +# been reached. diff --git a/ansible/roles/llm-inference-multimodel/tasks/verify.yml b/ansible/roles/llm-inference-multimodel/tasks/verify.yml index 7c23e54..31ab1c8 100644 --- a/ansible/roles/llm-inference-multimodel/tasks/verify.yml +++ b/ansible/roles/llm-inference-multimodel/tasks/verify.yml @@ -10,18 +10,18 @@ # systemd services" intent for durability, not just this-session. # ------------------------------------------------------------------------------ -- name: Enable and start llama-server-aux +- name: Enable llama-server-aux and start/restart based on Phase 2 unit-content change ansible.builtin.systemd: name: "{{ llm_aux_service_name }}" - state: started + state: "{{ 'restarted' if (llm_aux_unit_deployed.changed | default(false)) else 'started' }}" enabled: true daemon_reload: true become: true -- name: Enable and start llama-server-toolcall +- name: Enable llama-server-toolcall and start/restart based on Phase 2 unit-content change ansible.builtin.systemd: name: "{{ llm_toolcall_service_name }}" - state: started + state: "{{ 'restarted' if (llm_toolcall_unit_deployed.changed | default(false)) else 'started' }}" enabled: true daemon_reload: true become: true