From ecabe5fdc6fa12f76814c79dfcb97f9704b32f1f Mon Sep 17 00:00:00 2001 From: Ramil Valitov Date: Fri, 11 Sep 2026 08:07:49 +0300 Subject: [PATCH 1/3] fix(hot-reload): mount the config directory so reloads actually reach the engine The engine container was given a single-file bind mount (-v "$CONFIG_DIR/config.toml:/etc/telemt.toml:ro"). Such a mount pins one inode, so once that path is replaced the container keeps reading the unlinked one -- `mount` reports the source as ".../config.toml//deleted". No SIGHUP, inotify event or content poll can then deliver the new bytes, so secret add/remove/rotate/toggle and limit changes silently did nothing until the container was recreated. That also let a removed or rotated secret keep working, which is what made the bug dangerous. Mount the directory instead and point the engine at the file inside it, at all nine docker run sites (primary and secondary instances). A directory mount tracks directory entries rather than pinning one inode, so replacing the file is picked up immediately. Instance configs are now written straight to their own file through a new optional destination argument to generate_telemt_config, instead of being generated into config.toml and then moved into place. The old mv both unlinked config.toml (permanently detaching a running primary) and briefly left it carrying an instance's port and metrics port, which the engine can now actually observe. reload_proxy_config additionally verifies that the running engine can see the bytes it wrote and falls back to a restart when it cannot, so this class of failure self-heals rather than passing unnoticed. A stopped container is never mistaken for an out-of-sync one, and the change is only announced as a hot reload when the reload signal was actually delivered. --- mtproxymax.sh | 128 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 93 insertions(+), 35 deletions(-) diff --git a/mtproxymax.sh b/mtproxymax.sh index c398b7f..b52ccfb 100644 --- a/mtproxymax.sh +++ b/mtproxymax.sh @@ -1216,8 +1216,11 @@ build_faketls_secret() { fi } -# Generate telemt config.toml +# Generate telemt config. Optional arg is the destination file (defaults to the +# primary config.toml). Instance configs are written straight to their own file +# instead of being routed through config.toml, which the running engine watches. generate_telemt_config() { + local dest="${1:-${CONFIG_DIR}/config.toml}" mkdir -p "$CONFIG_DIR" chmod 700 "$CONFIG_DIR" @@ -1471,7 +1474,7 @@ TOML_EOF fi chmod 644 "$tmp" - cp "$tmp" "${CONFIG_DIR}/config.toml" && rm -f "$tmp" + cp "$tmp" "$dest" && rm -f "$tmp" } # Get comma-separated quoted list of enabled labels for config @@ -9238,8 +9241,8 @@ run_proxy_container() { local _run_out _run_out=$(docker run -d "${_docker_args[@]}" \ --ulimit nofile=65535:65535 \ - -v "${CONFIG_DIR}/config.toml:/etc/telemt.toml:ro" \ - "$(get_docker_image)" /etc/telemt.toml 2>&1) || { + -v "${CONFIG_DIR}:/etc/telemt:ro" \ + "$(get_docker_image)" /etc/telemt/config.toml 2>&1) || { # Check if failure was caused by resource limits (CPU/Memory cgroup rejection in unprivileged LXC/containers) if [ -n "${PROXY_MEMORY}" ] || [ -n "${PROXY_CPUS}" ]; then if echo "$_run_out" | grep -E -iq "(cgroup|permission denied|OCI runtime create failed|memory|swap|cpus)"; then @@ -9254,8 +9257,8 @@ run_proxy_container() { docker rm -f "$CONTAINER_NAME" 2>/dev/null || true _run_out=$(docker run -d "${_docker_args[@]}" \ --ulimit nofile=65535:65535 \ - -v "${CONFIG_DIR}/config.toml:/etc/telemt.toml:ro" \ - "$(get_docker_image)" /etc/telemt.toml 2>&1) || true + -v "${CONFIG_DIR}:/etc/telemt:ro" \ + "$(get_docker_image)" /etc/telemt/config.toml 2>&1) || true fi fi @@ -9274,14 +9277,14 @@ run_proxy_container() { log_info "Retrying container launch after D-Bus & memory cache recovery..." _run_out=$(docker run -d "${_docker_args[@]}" \ --ulimit nofile=65535:65535 \ - -v "${CONFIG_DIR}/config.toml:/etc/telemt.toml:ro" \ - "$(get_docker_image)" /etc/telemt.toml 2>&1) || { + -v "${CONFIG_DIR}:/etc/telemt:ro" \ + "$(get_docker_image)" /etc/telemt/config.toml 2>&1) || { # If standard retry still fails, attempt fallback with explicit host cgroup namespace docker rm -f "$CONTAINER_NAME" 2>/dev/null || true _run_out=$(docker run -d "${_docker_args[@]}" \ --cgroupns host \ - -v "${CONFIG_DIR}/config.toml:/etc/telemt.toml:ro" \ - "$(get_docker_image)" /etc/telemt.toml 2>&1) || { + -v "${CONFIG_DIR}:/etc/telemt:ro" \ + "$(get_docker_image)" /etc/telemt/config.toml 2>&1) || { docker rm -f "$CONTAINER_NAME" 2>/dev/null || true log_error "Failed to start container after recovery attempts" echo -e " ${DIM}${_run_out}${NC}" @@ -9375,18 +9378,17 @@ _start_all_instances() { [ "${INSTANCE_ENABLED[$i]}" = "true" ] || continue local cname="mtproxymax-${INSTANCE_PORTS[$i]}" docker ps --format '{{.Names}}' 2>/dev/null | grep -q "^${cname}$" && continue - # Regenerate instance config dynamically + # Regenerate instance config dynamically (straight to its own file — never via config.toml) local inst_config="${CONFIG_DIR}/config-${INSTANCE_PORTS[$i]}.toml" PROXY_PORT="${INSTANCE_PORTS[$i]}" PROXY_METRICS_PORT="${INSTANCE_METRICS_PORTS[$i]}" - generate_telemt_config - mv "${CONFIG_DIR}/config.toml" "$inst_config" 2>/dev/null + generate_telemt_config "$inst_config" docker rm -f "$cname" &>/dev/null || true local _inst_out _inst_out=$(docker run -d --name "$cname" --restart unless-stopped --network host \ --ulimit nofile=65535:65535 --log-opt max-size=10m --log-opt max-file=3 \ - -v "${inst_config}:/etc/telemt.toml:ro" \ - "$(get_docker_image)" /etc/telemt.toml 2>&1) || { + -v "${CONFIG_DIR}:/etc/telemt:ro" \ + "$(get_docker_image)" "/etc/telemt/$(basename "$inst_config")" 2>&1) || { if echo "$_inst_out" | grep -E -q "(cgroup|Message recipient disconnected|systemd|dbus|EOF|timeout|system\.slice|runc|OCI runtime create failed)"; then [ -w /proc/sys/vm/drop_caches ] && { sync; echo 3 > /proc/sys/vm/drop_caches 2>/dev/null || true; } systemctl daemon-reload 2>/dev/null || true @@ -9395,14 +9397,13 @@ _start_all_instances() { docker rm -f "$cname" &>/dev/null || true docker run -d --name "$cname" --restart unless-stopped --network host \ --cgroupns host --log-opt max-size=10m --log-opt max-file=3 \ - -v "${inst_config}:/etc/telemt.toml:ro" \ - "$(get_docker_image)" /etc/telemt.toml &>/dev/null || true + -v "${CONFIG_DIR}:/etc/telemt:ro" \ + "$(get_docker_image)" "/etc/telemt/$(basename "$inst_config")" &>/dev/null || true fi } done PROXY_PORT="$_orig_port" PROXY_METRICS_PORT="$_orig_mport" - generate_telemt_config } start_proxy_container() { @@ -9432,6 +9433,33 @@ restart_proxy_container() { speed_limit_apply 2>/dev/null || true } +# Does the engine container actually see the config file we just wrote? +# +# The container gets a bind mount of CONFIG_DIR. A single-file mount pins one +# inode, so replacing the file leaves the engine reading an unlinked one (the +# mount source shows up as "...//deleted" in `mount`) and no reload signal can +# ever deliver the new bytes. Reading through /proc//root gives us the +# container's view without needing any binary inside the image. +_engine_config_in_sync() { + local cfg="$1" cname="$2" pid + [ -f "$cfg" ] || return 0 + pid=$(docker inspect -f '{{.State.Pid}}' "$cname" 2>/dev/null) || return 1 + [ -n "$pid" ] && [ "$pid" != "0" ] || return 1 + local rel="/etc/telemt/$(basename "$cfg")" + if [ -r "/proc/${pid}/root${rel}" ]; then + cmp -s "$cfg" "/proc/${pid}/root${rel}" 2>/dev/null + else + # Fallback for hosts where the container root is not readable + docker exec "$cname" cat "$rel" 2>/dev/null | cmp -s - "$cfg" + fi +} + +# Is a secondary instance container currently up? A stopped instance has no +# engine to reload and must never be treated as an out-of-sync one. +_instance_container_running() { + docker ps --format '{{.Names}}' 2>/dev/null | grep -q "^$1$" +} + # Hot-reload: rewrite config.toml and let the engine pick it up (no restart, no dropped connections) # Use this for secret/limit changes. Falls back to restart if container is not running. reload_proxy_config() { @@ -9441,7 +9469,16 @@ reload_proxy_config() { flush_traffic_to_disk 2>/dev/null || true # Signal primary container to reload config (inotify may miss bind-mount changes) - is_proxy_running && docker kill -s SIGHUP "$CONTAINER_NAME" 2>/dev/null || true + local _reload_ok=false + if is_proxy_running; then + if docker kill -s SIGHUP "$CONTAINER_NAME" 2>/dev/null; then + _reload_ok=true + else + log_warn "Could not signal the engine to reload; restart the proxy to apply this change" + fi + else + log_warn "Proxy is not running; this change applies on next start" + fi # Also reload secondary instances if any if [ -f "$INSTANCES_FILE" ]; then @@ -9452,18 +9489,42 @@ reload_proxy_config() { local inst_config="${CONFIG_DIR}/config-${INSTANCE_PORTS[$i]}.toml" PROXY_PORT="${INSTANCE_PORTS[$i]}" PROXY_METRICS_PORT="${INSTANCE_METRICS_PORTS[$i]}" - generate_telemt_config - mv "${CONFIG_DIR}/config.toml" "$inst_config" 2>/dev/null - docker kill -s SIGHUP "mtproxymax-${INSTANCE_PORTS[$i]}" 2>/dev/null || true + generate_telemt_config "$inst_config" + _instance_container_running "mtproxymax-${INSTANCE_PORTS[$i]}" || continue + docker kill -s SIGHUP "mtproxymax-${INSTANCE_PORTS[$i]}" 2>/dev/null \ + || log_warn "Instance ${INSTANCE_PORTS[$i]}: could not signal the engine to reload" done PROXY_PORT="$_orig_port" PROXY_METRICS_PORT="$_orig_mport" - # Regenerate primary config (was overwritten by last instance) - generate_telemt_config fi speed_limit_apply 2>/dev/null || true - log_info "Config reloaded (hot-reload, no restart)" + + # Verify the running engine can actually see the bytes we wrote. Without this a + # detached mount shows up as a silent no-op: secrets that were removed keep + # working and new ones never connect. + local _stale=false + if is_proxy_running && ! _engine_config_in_sync "${CONFIG_DIR}/config.toml" "$CONTAINER_NAME"; then + _stale=true + fi + if [ -f "$INSTANCES_FILE" ]; then + local _j + for _j in "${!INSTANCE_PORTS[@]}"; do + [ "${INSTANCE_ENABLED[$_j]}" = "true" ] || continue + _instance_container_running "mtproxymax-${INSTANCE_PORTS[$_j]}" || continue + _engine_config_in_sync "${CONFIG_DIR}/config-${INSTANCE_PORTS[$_j]}.toml" "mtproxymax-${INSTANCE_PORTS[$_j]}" \ + || _stale=true + done + fi + + if [ "$_stale" = "true" ]; then + log_warn "Engine cannot see the updated config (stale bind mount); restarting the proxy to apply the change" + restart_proxy_container 2>/dev/null || true + return 0 + fi + + [ "$_reload_ok" = "true" ] && log_info "Config reloaded (hot-reload, no restart)" + return 0 } # Parse ISO 8601 timestamp to epoch (portable: GNU date, busybox date, Python fallback) @@ -13471,12 +13532,9 @@ instance_add() { local _orig_port="$PROXY_PORT" _orig_mport="$PROXY_METRICS_PORT" PROXY_PORT="$port" PROXY_METRICS_PORT="$mport" - generate_telemt_config - mv "${CONFIG_DIR}/config.toml" "$inst_config" 2>/dev/null + generate_telemt_config "$inst_config" PROXY_PORT="$_orig_port" PROXY_METRICS_PORT="$_orig_mport" - # Regenerate primary config - generate_telemt_config # Start container local cname="mtproxymax-${port}" @@ -13490,8 +13548,8 @@ instance_add() { ) local _inst_add_out _inst_add_out=$(docker run -d "${_docker_args[@]}" \ - -v "${inst_config}:/etc/telemt.toml:ro" \ - "$(get_docker_image)" /etc/telemt.toml 2>&1) || { + -v "${CONFIG_DIR}:/etc/telemt:ro" \ + "$(get_docker_image)" "/etc/telemt/$(basename "$inst_config")" 2>&1) || { if echo "$_inst_add_out" | grep -E -q "(cgroup|Message recipient disconnected|systemd|dbus|EOF|timeout|system\.slice|runc|OCI runtime create failed)"; then [ -w /proc/sys/vm/drop_caches ] && { sync; echo 3 > /proc/sys/vm/drop_caches 2>/dev/null || true; } systemctl daemon-reload 2>/dev/null || true @@ -13499,12 +13557,12 @@ instance_add() { sleep 2 docker rm -f "$cname" &>/dev/null || true docker run -d "${_docker_args[@]}" \ - -v "${inst_config}:/etc/telemt.toml:ro" \ - "$(get_docker_image)" /etc/telemt.toml &>/dev/null || { + -v "${CONFIG_DIR}:/etc/telemt:ro" \ + "$(get_docker_image)" "/etc/telemt/$(basename "$inst_config")" &>/dev/null || { docker run -d --name "$cname" --restart unless-stopped --network host \ --cgroupns host --log-opt max-size=10m --log-opt max-file=3 \ - -v "${inst_config}:/etc/telemt.toml:ro" \ - "$(get_docker_image)" /etc/telemt.toml &>/dev/null || true + -v "${CONFIG_DIR}:/etc/telemt:ro" \ + "$(get_docker_image)" "/etc/telemt/$(basename "$inst_config")" &>/dev/null || true } fi } From 283d4d1057eb7619f95e7cf83277a2b6d192fbce Mon Sep 17 00:00:00 2001 From: Ramil Valitov Date: Fri, 11 Sep 2026 08:08:01 +0300 Subject: [PATCH 2/3] test(hot-reload): lock in the config bind-mount and reload-reporting invariants Covers the reload path end to end: the container mounts the config directory rather than a single file, a reload with an instance enabled leaves the primary config.toml inode and content untouched, instance configs are written to their own file, a failed reload signal never claims a hot reload, a detached config triggers a restart, and stopped containers are never treated as out-of-sync. Ten of the sixteen assertions fail against the previous implementation. --- tests/test_hot_reload_inode.sh | 194 +++++++++++++++++++++++++++++++++ 1 file changed, 194 insertions(+) create mode 100644 tests/test_hot_reload_inode.sh diff --git a/tests/test_hot_reload_inode.sh b/tests/test_hot_reload_inode.sh new file mode 100644 index 0000000..852c9e2 --- /dev/null +++ b/tests/test_hot_reload_inode.sh @@ -0,0 +1,194 @@ +#!/bin/bash +# Regression tests for the hot-reload path. +# +# The engine container reads its config through a bind mount. A single-file mount +# pins one inode, so replacing that file leaves the engine reading an unlinked +# one — every reload silently becomes a no-op until the container is recreated. +# These tests lock in that we mount the directory, that instance configs are +# written straight to their own file instead of being routed through config.toml, +# and that a reload which cannot take effect says so instead of claiming success. +set -o pipefail + +if [ "${BASH_VERSINFO[0]:-0}" -lt 4 ]; then + echo "SKIP: bash 4+ required (got ${BASH_VERSION:-unknown})" >&2 + exit 0 +fi + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +TEST_ROOT=$(mktemp -d) || { echo "SKIP: cannot create temp dir" >&2; exit 0; } +trap 'rm -rf "$TEST_ROOT"' EXIT + +export INSTALL_DIR="$TEST_ROOT" +export CONFIG_DIR="$TEST_ROOT/mtproxy" +export SETTINGS_FILE="$TEST_ROOT/settings.conf" +export SECRETS_FILE="$TEST_ROOT/secrets.conf" +export INSTANCES_FILE="$TEST_ROOT/instances.conf" +export STATS_DIR="$TEST_ROOT/relay_stats" +mkdir -p "$CONFIG_DIR" "$STATS_DIR" + +MTPROXYMAX_SOURCE_ONLY=true source "${SCRIPT_DIR}/../mtproxymax.sh" +set +e + +TESTS_RUN=0 +TESTS_FAILED=0 + +assert_eq() { + local name="$1" want="$2" got="$3" + TESTS_RUN=$((TESTS_RUN + 1)) + if [ "$got" = "$want" ]; then + printf ' PASS %s\n' "$name" + else + TESTS_FAILED=$((TESTS_FAILED + 1)) + printf ' FAIL %s (got=%q want=%q)\n' "$name" "$got" "$want" + fi +} + +# ── Test doubles ───────────────────────────────────────────── +PROXY_RUNNING=true +KILL_FAILS=false +DOCKER_RUN_LOG="${TEST_ROOT}/docker_run.log" +INSTANCE_UP=false +LOG_INFO="" +LOG_WARN="" +RESTARTS=0 +: > "$DOCKER_RUN_LOG" + +docker() { + case "$1" in + # `docker run` is invoked inside $( ) — a subshell — so the argv has to be + # recorded in a file for the assertions to see it. + run) echo "$*" >> "$DOCKER_RUN_LOG"; return 0 ;; + kill) [ "$KILL_FAILS" = "true" ] && return 1; return 0 ;; + ps) [ "$INSTANCE_UP" = "true" ] && echo "mtproxymax-8443"; return 0 ;; + inspect) echo "0"; return 0 ;; + exec) return 1 ;; + *) return 0 ;; + esac +} + +is_proxy_running() { [ "$PROXY_RUNNING" = "true" ]; } +flush_traffic_to_disk() { return 0; } +speed_limit_apply() { return 0; } +restart_proxy_container() { RESTARTS=$((RESTARTS + 1)); return 0; } +log_info() { LOG_INFO="$*"; } +log_warn() { LOG_WARN="$*"; } +log_error() { LOG_WARN="$*"; } + +# In-sync check is stubbed: it reads /proc//root of a real container. +PRIMARY_IN_SYNC=true +INSTANCE_IN_SYNC=true +_engine_config_in_sync() { + case "$2" in + "$CONTAINER_NAME") [ "$PRIMARY_IN_SYNC" = "true" ] ;; + *) [ "$INSTANCE_IN_SYNC" = "true" ] ;; + esac +} + +# Ignore the generation timestamp when comparing configs +_strip_ts() { grep -v '^# Generated:' "$1" 2>/dev/null; } + +PRIMARY_PORT=443 +PROXY_PORT="$PRIMARY_PORT" +PROXY_METRICS_PORT=9090 + +echo "Hot-reload bind-mount tests" + +# ── 1. Source guard: never bind-mount the config as a single file ──────────── +SINGLE_FILE_MOUNTS=$(grep -c 'config\.toml:/etc/telemt' "${SCRIPT_DIR}/../mtproxymax.sh") +assert_eq "no single-file config bind mount remains" 0 "$SINGLE_FILE_MOUNTS" + +# ── 2. Instance configs are written directly, and instances mount the dir ──── +cat > "$INSTANCES_FILE" <<'EOF' +# MTProxyMax Instances — Format: PORT|METRICS_PORT|ENABLED|LABEL +8443|9091|true|inst1 +EOF + +: > "$DOCKER_RUN_LOG" +_start_all_instances 2>/dev/null + +INST_CFG="${CONFIG_DIR}/config-8443.toml" +if [ -f "$INST_CFG" ]; then + assert_eq "instance config written to its own file" "yes" "yes" +else + assert_eq "instance config written to its own file" "yes" "no" +fi +assert_eq "instance config carries the instance port" 1 \ + "$(grep -c '^port = 8443' "$INST_CFG" 2>/dev/null)" +assert_eq "instance container mounts the config directory" 1 \ + "$(grep -c -- "-v ${CONFIG_DIR}:/etc/telemt:ro" "$DOCKER_RUN_LOG")" +assert_eq "instance container is pointed at its own config" 1 \ + "$(grep -c '/etc/telemt/config-8443.toml' "$DOCKER_RUN_LOG")" + +# ── 3. Reloading never replaces the primary config file ───────────────────── +INSTANCE_UP=true +generate_telemt_config 2>/dev/null +BEFORE_INODE=$(stat -c %i "${CONFIG_DIR}/config.toml" 2>/dev/null) +cp "${CONFIG_DIR}/config.toml" "${TEST_ROOT}/before.toml" 2>/dev/null + +reload_proxy_config 2>/dev/null + +AFTER_INODE=$(stat -c %i "${CONFIG_DIR}/config.toml" 2>/dev/null) +assert_eq "primary config inode survives a reload with instances" "$BEFORE_INODE" "$AFTER_INODE" +if diff <(_strip_ts "${TEST_ROOT}/before.toml") <(_strip_ts "${CONFIG_DIR}/config.toml") >/dev/null 2>&1; then + assert_eq "primary config is not overwritten by instance content" "yes" "yes" +else + assert_eq "primary config is not overwritten by instance content" "yes" "no" +fi + +# ── 4. Reload outcome is reported honestly ────────────────────────────────── +LOG_INFO="" +LOG_WARN="" +KILL_FAILS=true +reload_proxy_config 2>/dev/null +assert_eq "failed signal does not claim a reload" "" "$LOG_INFO" +if [ -n "$LOG_WARN" ]; then + assert_eq "failed signal warns the user" "yes" "yes" +else + assert_eq "failed signal warns the user" "yes" "no" +fi + +LOG_INFO="" +LOG_WARN="" +KILL_FAILS=false +reload_proxy_config 2>/dev/null +assert_eq "successful signal reports the hot reload" \ + "Config reloaded (hot-reload, no restart)" "$LOG_INFO" + +# ── 5. A detached config triggers a restart instead of a silent no-op ─────── +LOG_INFO="" +LOG_WARN="" +RESTARTS=0 +PRIMARY_IN_SYNC=false +reload_proxy_config 2>/dev/null +assert_eq "detached primary config falls back to a restart" 1 "$RESTARTS" +assert_eq "detached primary config does not claim a hot reload" "" "$LOG_INFO" + +LOG_INFO="" +RESTARTS=0 +PRIMARY_IN_SYNC=true +INSTANCE_IN_SYNC=false +reload_proxy_config 2>/dev/null +assert_eq "detached instance config falls back to a restart" 1 "$RESTARTS" +INSTANCE_IN_SYNC=true + +# ── 6. A stopped container is never mistaken for an out-of-sync one ───────── +LOG_INFO="" +LOG_WARN="" +RESTARTS=0 +PROXY_RUNNING=false +INSTANCE_UP=false +PRIMARY_IN_SYNC=false +INSTANCE_IN_SYNC=false +reload_proxy_config 2>/dev/null +assert_eq "stopped proxy is not reported as reloaded" "" "$LOG_INFO" +assert_eq "stopped proxy is not restarted" 0 "$RESTARTS" + +# A stopped instance must not drag the whole proxy into a restart either +PROXY_RUNNING=true +PRIMARY_IN_SYNC=true +RESTARTS=0 +reload_proxy_config 2>/dev/null +assert_eq "stopped instance does not trigger a restart" 0 "$RESTARTS" + +printf '\n%d tests, %d failures\n' "$TESTS_RUN" "$TESTS_FAILED" +[ "$TESTS_FAILED" -eq 0 ] From 9e2512af011319c31c9cc0e9cd6f351c7a06c05c Mon Sep 17 00:00:00 2001 From: Ramil Valitov Date: Fri, 11 Sep 2026 14:39:23 +0300 Subject: [PATCH 3/3] fix(hot-reload): write the config in place instead of copying over it A field A/B on the affected box settles the trigger. With config.toml as the source of the container's bind mount, so that the file is itself a mount point: cp onto the bind-mounted config.toml -> inode replaced -> detached cp onto an ordinary file -> inode preserved append, `>` redirect, dd conv=notrunc -> inode preserved So `cp` takes an unlink-and-recreate path when the destination is a mount point instead of truncating in place. That inode swap is what produced ".../config.toml//deleted" in mountinfo and left the engine reading the unlinked original, making every reload a silent no-op until the container was recreated -- and making secret removal and rotation appear to do nothing. Write through a shell redirect, which can only truncate. Guard against a failed generation clobbering a good config with an empty file, and set the mode explicitly because `>` creates with the umask when the destination does not exist yet. This complements the directory mount in the previous commit rather than replacing it: the in-place write removes the trigger we proved, while the directory mount keeps the engine working when anything else replaces the file (editors, sed -i, restore from backup, config management) -- the same A/B implies those detach it just as cp did. Why busybox cp unlinks a mount point is still unexplained and is left to the maintainer; a strace settles it. The fix does not depend on the answer. --- mtproxymax.sh | 11 ++++++++++- tests/test_hot_reload_inode.sh | 35 ++++++++++++++++++++++++---------- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/mtproxymax.sh b/mtproxymax.sh index b52ccfb..e7f02a3 100644 --- a/mtproxymax.sh +++ b/mtproxymax.sh @@ -1474,7 +1474,16 @@ TOML_EOF fi chmod 644 "$tmp" - cp "$tmp" "$dest" && rm -f "$tmp" + # Write in place. `cp` onto a file that is itself the source of a bind mount + # (i.e. the file is a mount point) unlinks and recreates it instead of + # truncating it, so the inode changes and every container mounting that file + # keeps reading the unlinked original -- each reload then silently does + # nothing until the container is recreated. A shell redirect can only + # truncate. The guard stops a failed generation from clobbering a good + # config with an empty file, and the mode is set explicitly because `>` + # creates with the umask when the destination does not exist yet. + [ -f "$tmp" ] || { log_error "Config generation produced no output"; return 1; } + cat "$tmp" > "$dest" && chmod 644 "$dest" && rm -f "$tmp" } # Get comma-separated quoted list of enabled labels for config diff --git a/tests/test_hot_reload_inode.sh b/tests/test_hot_reload_inode.sh index 852c9e2..25fe3a0 100644 --- a/tests/test_hot_reload_inode.sh +++ b/tests/test_hot_reload_inode.sh @@ -2,11 +2,12 @@ # Regression tests for the hot-reload path. # # The engine container reads its config through a bind mount. A single-file mount -# pins one inode, so replacing that file leaves the engine reading an unlinked -# one — every reload silently becomes a no-op until the container is recreated. -# These tests lock in that we mount the directory, that instance configs are -# written straight to their own file instead of being routed through config.toml, -# and that a reload which cannot take effect says so instead of claiming success. +# pins one inode, and `cp` onto a file that is itself a mount point replaces that +# inode rather than truncating it, so the engine is left reading an unlinked file +# — every reload silently becomes a no-op until the container is recreated. +# These tests lock in that we mount the directory, write the config in place, +# write instance configs straight to their own file instead of routing them +# through config.toml, and report a reload honestly when it cannot take effect. set -o pipefail if [ "${BASH_VERSINFO[0]:-0}" -lt 4 ]; then @@ -93,10 +94,15 @@ PROXY_METRICS_PORT=9090 echo "Hot-reload bind-mount tests" -# ── 1. Source guard: never bind-mount the config as a single file ──────────── +# ── 1. Source guards: the two things that made reloads a silent no-op ─────── SINGLE_FILE_MOUNTS=$(grep -c 'config\.toml:/etc/telemt' "${SCRIPT_DIR}/../mtproxymax.sh") assert_eq "no single-file config bind mount remains" 0 "$SINGLE_FILE_MOUNTS" +# `cp` onto a mount point replaces the inode (busybox cp does this), which +# detaches the container. The config must be written through a redirect. +COPY_OVER=$(grep -c 'cp "$tmp" "$dest"' "${SCRIPT_DIR}/../mtproxymax.sh") +assert_eq "config is never copied over itself" 0 "$COPY_OVER" + # ── 2. Instance configs are written directly, and instances mount the dir ──── cat > "$INSTANCES_FILE" <<'EOF' # MTProxyMax Instances — Format: PORT|METRICS_PORT|ENABLED|LABEL @@ -119,9 +125,18 @@ assert_eq "instance container mounts the config directory" 1 \ assert_eq "instance container is pointed at its own config" 1 \ "$(grep -c '/etc/telemt/config-8443.toml' "$DOCKER_RUN_LOG")" -# ── 3. Reloading never replaces the primary config file ───────────────────── +# ── 3. Regenerating writes in place and produces a complete config ────────── INSTANCE_UP=true generate_telemt_config 2>/dev/null +GEN_INODE=$(stat -c %i "${CONFIG_DIR}/config.toml" 2>/dev/null) +generate_telemt_config 2>/dev/null +assert_eq "regeneration keeps the config inode" "$GEN_INODE" \ + "$(stat -c %i "${CONFIG_DIR}/config.toml" 2>/dev/null)" +assert_eq "in-place write produces a complete config" 1 \ + "$(grep -c '^\[access.users\]$' "${CONFIG_DIR}/config.toml" 2>/dev/null)" + +# ── 4. Reloading never replaces the primary config file ───────────────────── +generate_telemt_config 2>/dev/null BEFORE_INODE=$(stat -c %i "${CONFIG_DIR}/config.toml" 2>/dev/null) cp "${CONFIG_DIR}/config.toml" "${TEST_ROOT}/before.toml" 2>/dev/null @@ -135,7 +150,7 @@ else assert_eq "primary config is not overwritten by instance content" "yes" "no" fi -# ── 4. Reload outcome is reported honestly ────────────────────────────────── +# ── 5. Reload outcome is reported honestly ────────────────────────────────── LOG_INFO="" LOG_WARN="" KILL_FAILS=true @@ -154,7 +169,7 @@ reload_proxy_config 2>/dev/null assert_eq "successful signal reports the hot reload" \ "Config reloaded (hot-reload, no restart)" "$LOG_INFO" -# ── 5. A detached config triggers a restart instead of a silent no-op ─────── +# ── 6. A detached config triggers a restart instead of a silent no-op ─────── LOG_INFO="" LOG_WARN="" RESTARTS=0 @@ -171,7 +186,7 @@ reload_proxy_config 2>/dev/null assert_eq "detached instance config falls back to a restart" 1 "$RESTARTS" INSTANCE_IN_SYNC=true -# ── 6. A stopped container is never mistaken for an out-of-sync one ───────── +# ── 7. A stopped container is never mistaken for an out-of-sync one ───────── LOG_INFO="" LOG_WARN="" RESTARTS=0