From 501efee07e621eb626f5ed032210d49f30c04408 Mon Sep 17 00:00:00 2001
From: Marcel Partap <mpartap@gmx.net>
Date: Thu, 22 Oct 2020 20:53:41 +0200
Subject: [PATCH] synapse workers: supply systemd with actual worker PIDs
 (requires jq)

also, worker.yaml.j2:
  - hone worker_name
  - remove worker_pid_file entry (would only be used if worker_daemonize
    set to true; also, synapse only knows about the container namespace
    and thus can not provide the required host-view PID)
---
 .../matrix-synapse-worker-write-pid           | 30 +++++++++++++++++++
 .../tasks/workers/setup_install.yml           |  6 ++++
 .../tasks/workers/setup_uninstall.yml         |  5 ++++
 .../systemd/matrix-synapse-worker@.service.j2 | 12 +++++---
 .../templates/synapse/worker.yaml.j2          |  3 +-
 5 files changed, 50 insertions(+), 6 deletions(-)
 create mode 100644 roles/matrix-synapse/files/usr-local-bin/matrix-synapse-worker-write-pid

diff --git a/roles/matrix-synapse/files/usr-local-bin/matrix-synapse-worker-write-pid b/roles/matrix-synapse/files/usr-local-bin/matrix-synapse-worker-write-pid
new file mode 100644
index 000000000..02c5ba09b
--- /dev/null
+++ b/roles/matrix-synapse/files/usr-local-bin/matrix-synapse-worker-write-pid
@@ -0,0 +1,30 @@
+#!/bin/bash
+# Find a synapse worker's PID and write it to a file so systemd can manage it as a service
+
+# example invocation:
+# matrix-synapse-worker-write-pid user_dir:18700 /run/matrix-synapse-worker.user_dir:18700.pid
+
+docker_api_call() { curl --silent --unix-socket /var/run/docker.sock ${@}; }
+
+TARGETCONTAINER=matrix-synapse
+TARGETWORKER=${1}
+PIDFILE=${2}
+
+# get ID list of subprocesses executed in $TARGETCONTAINER, and for each..
+for EXECID in $(docker_api_call http://localhost/containers/${TARGETCONTAINER}/json | jq --raw-output '.ExecIDs[]')
+do
+  # fetch detailed process info
+  EXECINFO=$(docker_api_call http://localhost/exec/${EXECID}/json)
+
+  # extract config file path from last command argument
+  WORKERCONFIGFILE=$(echo ${EXECINFO} | jq --raw-output .ProcessConfig.arguments[-1])
+
+  # reconstruct worker name
+  WORKERNAME=${WORKERCONFIGFILE#*/worker.}
+  WORKERNAME=${WORKERNAME%.yaml}
+
+  # if name matches the target worker: write out most recent PID & quit
+  [ "${WORKERNAME}" = "${TARGETWORKER}" ] \
+    && echo ${EXECINFO} | jq --raw-output .Pid > ${PIDFILE} \
+    && exit 0
+done
diff --git a/roles/matrix-synapse/tasks/workers/setup_install.yml b/roles/matrix-synapse/tasks/workers/setup_install.yml
index 0031c236c..44d59495a 100644
--- a/roles/matrix-synapse/tasks/workers/setup_install.yml
+++ b/roles/matrix-synapse/tasks/workers/setup_install.yml
@@ -40,3 +40,9 @@
       {{ matrix_synapse_systemd_wanted_services_list +
       ['matrix-synapse-worker@' + item.worker + ':' + item.port|string + '.service'] }}
   with_items: "{{ matrix_synapse_workers_enabled_list }}"
+
+- name: Ensure matrix-synapse-worker-write-pid script is created
+  copy:
+    src: "{{ role_path }}/files/usr-local-bin/matrix-synapse-worker-write-pid"
+    dest: "{{ matrix_local_bin_path }}/matrix-synapse-worker-write-pid"
+    mode: 0750
diff --git a/roles/matrix-synapse/tasks/workers/setup_uninstall.yml b/roles/matrix-synapse/tasks/workers/setup_uninstall.yml
index d1e7e3b56..0571114c5 100644
--- a/roles/matrix-synapse/tasks/workers/setup_uninstall.yml
+++ b/roles/matrix-synapse/tasks/workers/setup_uninstall.yml
@@ -36,3 +36,8 @@
 - name: Ensure systemd noticed removal of worker service units
   service:
     daemon_reload: yes
+
+- name: Ensure matrix-synapse-worker-write-pid script is removed
+  file:
+    path: "{{ matrix_local_bin_path }}/matrix-synapse-worker-write-pid"
+    state: absent
diff --git a/roles/matrix-synapse/templates/synapse/systemd/matrix-synapse-worker@.service.j2 b/roles/matrix-synapse/templates/synapse/systemd/matrix-synapse-worker@.service.j2
index d14b2557f..2c82873d9 100644
--- a/roles/matrix-synapse/templates/synapse/systemd/matrix-synapse-worker@.service.j2
+++ b/roles/matrix-synapse/templates/synapse/systemd/matrix-synapse-worker@.service.j2
@@ -3,8 +3,8 @@
 # alongside the homeserver main process.
 # c.f. https://github.com/matrix-org/synapse/pull/4662
 [Unit]
-Description=Synapse Matrix Worker
-AssertPathExists={{matrix_synapse_config_dir_path }}/worker.%i.yaml
+Description=Matrix worker synapse.app.%i
+AssertPathExists={{ matrix_synapse_config_dir_path }}/worker.%i.yaml
 After=matrix-synapse.service
 BindsTo=matrix-synapse.service
 
@@ -23,9 +23,13 @@ ExecStart=/bin/sh -c "WORKER=%i; WORKER=$${WORKER%%:*}; \
 			matrix-synapse \
 			python -m synapse.app.$${WORKER} -c /data/homeserver.yaml -c /data/worker.%i.yaml"
 
+# wait for worker startup & write out PID of actual worker process so systemd can handle it
+ExecStartPost=/bin/sleep 5
+ExecStartPost=/usr/local/bin/matrix-synapse-worker-write-pid %i /run/matrix-synapse-worker.%i.pid
+
 ExecReload=/bin/kill -HUP $MAINPID
-ExecStop=/usr/bin/docker exec matrix-synapse pkill -f %i
-PIDFile=/matrix-run/{{ item.worker }}.port{{ item.port }}.pid
+ExecStop=/bin/kill $MAINPID
+PIDFile=/run/matrix-synapse-worker.%i.pid
 KillMode=process
 Restart=always
 RestartSec=10
diff --git a/roles/matrix-synapse/templates/synapse/worker.yaml.j2 b/roles/matrix-synapse/templates/synapse/worker.yaml.j2
index 319f5708a..0a282ba7c 100644
--- a/roles/matrix-synapse/templates/synapse/worker.yaml.j2
+++ b/roles/matrix-synapse/templates/synapse/worker.yaml.j2
@@ -1,6 +1,6 @@
 #jinja2: lstrip_blocks: "True"
 worker_app: synapse.app.{{ item.worker }}
-worker_name: {{ item.worker ~ '_' ~ item.port }}
+worker_name: {{ item.worker ~ ':' ~ item.port }}
 
 worker_replication_host: 127.0.0.1
 worker_replication_http_port: {{ matrix_synapse_replication_http_port }}
@@ -26,5 +26,4 @@ worker_main_http_uri: http://127.0.0.1:8008
 {% endif %}
 
 worker_daemonize: false
-worker_pid_file: /matrix-run/{{ item.worker }}.port{{ item.port }}.pid
 worker_log_config: /data/{{ matrix_server_fqn_matrix }}.log.config