fix: pin agent update gateway restart profile
This commit is contained in:
1 parent
885e267213
commit
14adbb61a4
5 files changed
+119
-37
No files matched your search
+7
-3
@@ -251,7 +251,7 @@ def _gateway_running_pid(gateway_status: Any, pid_path: Path | None) -> int | No
|
||||
return get_running_pid()
|
||||
|
||||
|
||||
def get_active_profile_gateway_running_pid() -> int | None:
|
||||
def get_active_profile_gateway_running_pid(profile: str | None = None) -> int | None:
|
||||
"""Return the confirmed active-profile PID without state fallbacks.
|
||||
|
||||
Update recovery needs process identity, not just a recent ``running`` state:
|
||||
@@ -260,10 +260,14 @@ def get_active_profile_gateway_running_pid() -> int | None:
|
||||
the default-root health path. Keep this fail-closed when status is unavailable.
|
||||
"""
|
||||
try:
|
||||
from api.profiles import get_active_hermes_home
|
||||
from api.profiles import get_active_hermes_home, get_hermes_home_for_profile
|
||||
|
||||
gateway_status = _gateway_status_module()
|
||||
gateway_pid_path = Path(get_active_hermes_home()) / _GATEWAY_PID_FILE
|
||||
if profile is None:
|
||||
gateway_home = get_active_hermes_home()
|
||||
else:
|
||||
gateway_home = get_hermes_home_for_profile(profile)
|
||||
gateway_pid_path = Path(gateway_home) / _GATEWAY_PID_FILE
|
||||
return _gateway_running_pid(gateway_status, gateway_pid_path)
|
||||
except Exception:
|
||||
return None
|
||||
|
||||
+27
-4
@@ -10,7 +10,13 @@ import sys
|
||||
import threading
|
||||
from pathlib import Path
|
||||
|
||||
from api.profiles import get_active_hermes_home
|
||||
from api.profiles import (
|
||||
_PROFILE_ID_RE,
|
||||
_is_root_profile,
|
||||
get_active_hermes_home,
|
||||
get_active_profile_name,
|
||||
get_hermes_home_for_profile,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -46,8 +52,23 @@ def _release_lock() -> None:
|
||||
pass
|
||||
|
||||
|
||||
def _gateway_restart_profile_context(profile: str | None = None) -> tuple[Path, str]:
|
||||
"""Return the HERMES_HOME and CLI profile arg for a gateway restart."""
|
||||
if profile is None:
|
||||
raw_profile = str(get_active_profile_name() or "default").strip()
|
||||
active_home = Path(get_active_hermes_home())
|
||||
else:
|
||||
raw_profile = str(profile or "default").strip()
|
||||
active_home = Path(get_hermes_home_for_profile(raw_profile))
|
||||
|
||||
if not raw_profile or not _PROFILE_ID_RE.fullmatch(raw_profile) or _is_root_profile(raw_profile):
|
||||
return active_home, "default"
|
||||
return active_home, raw_profile
|
||||
|
||||
|
||||
def restart_active_profile_gateway(
|
||||
*,
|
||||
profile: str | None = None,
|
||||
quick_timeout_seconds: float = 2.0,
|
||||
background_wait_seconds: float = 240.0,
|
||||
) -> dict:
|
||||
@@ -66,18 +87,20 @@ def restart_active_profile_gateway(
|
||||
}
|
||||
|
||||
try:
|
||||
active_home = get_active_hermes_home()
|
||||
active_home, cli_profile = _gateway_restart_profile_context(profile)
|
||||
env = os.environ.copy()
|
||||
env["HERMES_HOME"] = str(active_home)
|
||||
hermes_cmd = _resolve_hermes_command()
|
||||
cmd = [hermes_cmd, "--profile", cli_profile, "gateway", "restart"]
|
||||
|
||||
logger.info(
|
||||
"Restarting gateway service via CLI command: %s gateway restart (HERMES_HOME=%s)",
|
||||
"Restarting gateway service via CLI command: %s --profile %s gateway restart (HERMES_HOME=%s)",
|
||||
hermes_cmd,
|
||||
cli_profile,
|
||||
active_home,
|
||||
)
|
||||
proc = subprocess.Popen(
|
||||
[hermes_cmd, "gateway", "restart"],
|
||||
cmd,
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE,
|
||||
text=True,
|
||||
|
||||
+6
-4
@@ -26,6 +26,7 @@ from urllib.parse import urlparse
|
||||
|
||||
from api.agent_health import get_active_profile_gateway_running_pid
|
||||
from api.gateway_restart import restart_active_profile_gateway
|
||||
from api.profiles import get_active_profile_name
|
||||
from api.config import REPO_ROOT, STREAMS, STREAMS_LOCK
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -1816,8 +1817,9 @@ def _ensure_gateway_restart_for_agent_update() -> tuple[bool, dict]:
|
||||
- ok is False when restart did not complete and callers must abort success.
|
||||
- restart_payload contains helper status fields for response shaping.
|
||||
"""
|
||||
gateway_pid_before_restart = get_active_profile_gateway_running_pid()
|
||||
restart_result = restart_active_profile_gateway()
|
||||
target_profile = str(get_active_profile_name() or "default").strip() or "default"
|
||||
gateway_pid_before_restart = get_active_profile_gateway_running_pid(profile=target_profile)
|
||||
restart_result = restart_active_profile_gateway(profile=target_profile)
|
||||
status = str(restart_result.get("status") or "")
|
||||
if status in {"completed", "in_progress"}:
|
||||
return True, restart_result
|
||||
@@ -1829,7 +1831,7 @@ def _ensure_gateway_restart_for_agent_update() -> tuple[bool, dict]:
|
||||
# bounded delay so an already-applied Agent update is not reported as a
|
||||
# complete failure because of that transient process handoff.
|
||||
time.sleep(_AGENT_GATEWAY_RESTART_RETRY_DELAY_S)
|
||||
retry_result = restart_active_profile_gateway()
|
||||
retry_result = restart_active_profile_gateway(profile=target_profile)
|
||||
retry_status = str(retry_result.get("status") or "")
|
||||
if retry_status in {"completed", "in_progress"}:
|
||||
return True, {
|
||||
@@ -1848,7 +1850,7 @@ def _ensure_gateway_restart_for_agent_update() -> tuple[bool, dict]:
|
||||
# service. Only accept that recovery when the confirmed local PID changed;
|
||||
# a merely-alive old gateway has not loaded the updated Agent checkout.
|
||||
time.sleep(_AGENT_GATEWAY_RESTART_RETRY_DELAY_S)
|
||||
gateway_pid_after_retry = get_active_profile_gateway_running_pid()
|
||||
gateway_pid_after_retry = get_active_profile_gateway_running_pid(profile=target_profile)
|
||||
if (
|
||||
gateway_pid_before_restart is not None
|
||||
and gateway_pid_after_retry is not None
|
||||
|
||||
@@ -96,11 +96,35 @@ def test_restart_active_profile_gateway_success_uses_active_profile_home(monkeyp
|
||||
|
||||
assert result["status"] == "completed"
|
||||
assert result["message"] == "Gateway service restarted successfully"
|
||||
assert called["args"] == ["/mock/bin/hermes", "gateway", "restart"]
|
||||
assert called["args"] == ["/mock/bin/hermes", "--profile", "default", "gateway", "restart"]
|
||||
assert called["env"]["HERMES_HOME"] == "/mock/hermes/home"
|
||||
assert gateway_restart._GATEWAY_RESTART_LOCK.locked() is False
|
||||
|
||||
|
||||
def test_restart_active_profile_gateway_pins_explicit_default_profile(monkeypatch):
|
||||
gateway_restart._GATEWAY_RESTART_LOCK = threading.Lock()
|
||||
called = {}
|
||||
|
||||
def fake_popen(args, stdout=None, stderr=None, text=True, env=None):
|
||||
called["args"] = args
|
||||
called["env"] = env
|
||||
return MockPopen(args, stdout_text="ok", returncode=0, env=env)
|
||||
|
||||
monkeypatch.setattr(
|
||||
gateway_restart,
|
||||
"get_hermes_home_for_profile",
|
||||
lambda profile: "/mock/hermes/default" if profile == "default" else "/mock/hermes/profiles/work",
|
||||
)
|
||||
monkeypatch.setattr(gateway_restart.shutil, "which", lambda cmd: "/mock/bin/hermes")
|
||||
monkeypatch.setattr(gateway_restart.subprocess, "Popen", fake_popen)
|
||||
|
||||
result = gateway_restart.restart_active_profile_gateway(profile="default")
|
||||
|
||||
assert result["status"] == "completed"
|
||||
assert called["args"] == ["/mock/bin/hermes", "--profile", "default", "gateway", "restart"]
|
||||
assert called["env"]["HERMES_HOME"] == "/mock/hermes/default"
|
||||
|
||||
|
||||
def test_restart_active_profile_gateway_failure_preserves_empty_output_contract(monkeypatch):
|
||||
gateway_restart._GATEWAY_RESTART_LOCK = threading.Lock()
|
||||
|
||||
|
||||
@@ -729,7 +729,7 @@ class TestSuccessfulUpdateReturnsRestartScheduled:
|
||||
monkeypatch.setattr(upd, '_schedule_restart', lambda delay=2.0: None)
|
||||
monkeypatch.setattr(
|
||||
'api.updates.restart_active_profile_gateway',
|
||||
lambda: {'status': 'completed', 'message': 'Gateway service restarted successfully'},
|
||||
lambda **kwargs: {'status': 'completed', 'message': 'Gateway service restarted successfully'},
|
||||
)
|
||||
|
||||
result = upd.apply_update('agent')
|
||||
@@ -839,13 +839,13 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
restart_calls = []
|
||||
sleeps = []
|
||||
|
||||
def fake_restart():
|
||||
restart_calls.append('called')
|
||||
def fake_restart(*, profile=None):
|
||||
restart_calls.append(profile)
|
||||
return next(restart_results)
|
||||
|
||||
monkeypatch.setattr(upd, 'restart_active_profile_gateway', fake_restart)
|
||||
monkeypatch.setattr(upd.time, 'sleep', sleeps.append)
|
||||
monkeypatch.setattr(upd, 'get_active_profile_gateway_running_pid', lambda: 101)
|
||||
monkeypatch.setattr(upd, 'get_active_profile_gateway_running_pid', lambda *, profile=None: 101)
|
||||
|
||||
ok, result = upd._ensure_gateway_restart_for_agent_update()
|
||||
|
||||
@@ -853,7 +853,7 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
assert result['status'] == 'completed'
|
||||
assert result['retry_attempted'] is True
|
||||
assert 'bad file descriptor' in result['initial_failure']
|
||||
assert restart_calls == ['called', 'called']
|
||||
assert restart_calls == ['default', 'default']
|
||||
assert sleeps == [upd._AGENT_GATEWAY_RESTART_RETRY_DELAY_S]
|
||||
|
||||
def test_agent_gateway_restart_retry_busy_stays_fail_closed(self, monkeypatch):
|
||||
@@ -866,12 +866,12 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
sleeps = []
|
||||
gateway_pid_calls = []
|
||||
|
||||
monkeypatch.setattr(upd, 'restart_active_profile_gateway', lambda: next(restart_results))
|
||||
monkeypatch.setattr(upd, 'restart_active_profile_gateway', lambda **kwargs: next(restart_results))
|
||||
monkeypatch.setattr(upd.time, 'sleep', sleeps.append)
|
||||
monkeypatch.setattr(
|
||||
upd,
|
||||
'get_active_profile_gateway_running_pid',
|
||||
lambda: gateway_pid_calls.append(101) or 101,
|
||||
lambda *, profile=None: gateway_pid_calls.append(profile) or 101,
|
||||
)
|
||||
|
||||
ok, result = upd._ensure_gateway_restart_for_agent_update()
|
||||
@@ -881,7 +881,7 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
assert result['retry_attempted'] is True
|
||||
assert 'first' in result['initial_failure']
|
||||
assert sleeps == [upd._AGENT_GATEWAY_RESTART_RETRY_DELAY_S]
|
||||
assert gateway_pid_calls == [101]
|
||||
assert gateway_pid_calls == ['default']
|
||||
|
||||
def test_agent_gateway_restart_accepts_verified_process_replacement_after_retry_failure(self, monkeypatch):
|
||||
import api.updates as upd
|
||||
@@ -894,11 +894,11 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
sleeps = []
|
||||
gateway_pids = iter([101, 202])
|
||||
|
||||
def fake_restart():
|
||||
def fake_restart(*, profile=None):
|
||||
timeline.append('restart')
|
||||
return next(restart_results)
|
||||
|
||||
def fake_gateway_pid():
|
||||
def fake_gateway_pid(*, profile=None):
|
||||
pid = next(gateway_pids)
|
||||
timeline.append(f'pid:{pid}')
|
||||
return pid
|
||||
@@ -931,13 +931,13 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
restart_calls = []
|
||||
sleeps = []
|
||||
|
||||
def fake_restart():
|
||||
restart_calls.append('called')
|
||||
def fake_restart(*, profile=None):
|
||||
restart_calls.append(profile)
|
||||
return next(restart_results)
|
||||
|
||||
monkeypatch.setattr(upd, 'restart_active_profile_gateway', fake_restart)
|
||||
monkeypatch.setattr(upd.time, 'sleep', sleeps.append)
|
||||
monkeypatch.setattr(upd, 'get_active_profile_gateway_running_pid', lambda: 101)
|
||||
monkeypatch.setattr(upd, 'get_active_profile_gateway_running_pid', lambda *, profile=None: 101)
|
||||
|
||||
ok, result = upd._ensure_gateway_restart_for_agent_update()
|
||||
|
||||
@@ -946,12 +946,41 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
assert result['retry_attempted'] is True
|
||||
assert 'Restart failed: first' in result['message']
|
||||
assert 'Restart failed: retry' in result['message']
|
||||
assert restart_calls == ['called', 'called']
|
||||
assert restart_calls == ['default', 'default']
|
||||
assert sleeps == [
|
||||
upd._AGENT_GATEWAY_RESTART_RETRY_DELAY_S,
|
||||
upd._AGENT_GATEWAY_RESTART_RETRY_DELAY_S,
|
||||
]
|
||||
|
||||
def test_agent_gateway_restart_default_retry_cannot_use_sticky_named_profile(self, monkeypatch):
|
||||
import api.updates as upd
|
||||
|
||||
default_restart_results = iter([
|
||||
{'status': 'failed', 'message': 'Restart failed: default first'},
|
||||
{'status': 'failed', 'message': 'Restart failed: default retry'},
|
||||
])
|
||||
restart_profiles = []
|
||||
|
||||
def fake_restart(*, profile=None):
|
||||
effective_profile = profile or 'sticky-work'
|
||||
restart_profiles.append(effective_profile)
|
||||
if effective_profile == 'sticky-work':
|
||||
return {'status': 'completed', 'message': 'wrong profile restarted'}
|
||||
return next(default_restart_results)
|
||||
|
||||
monkeypatch.setattr(upd, 'get_active_profile_name', lambda: 'default')
|
||||
monkeypatch.setattr(upd, 'restart_active_profile_gateway', fake_restart)
|
||||
monkeypatch.setattr(upd.time, 'sleep', lambda seconds: None)
|
||||
monkeypatch.setattr(upd, 'get_active_profile_gateway_running_pid', lambda *, profile=None: 101)
|
||||
|
||||
ok, result = upd._ensure_gateway_restart_for_agent_update()
|
||||
|
||||
assert ok is False
|
||||
assert restart_profiles == ['default', 'default']
|
||||
assert result['status'] == 'failed'
|
||||
assert 'default first' in result['message']
|
||||
assert 'default retry' in result['message']
|
||||
|
||||
def test_apply_update_agent_requires_gateway_restart(self, tmp_path, monkeypatch):
|
||||
import api.updates as upd
|
||||
|
||||
@@ -973,8 +1002,8 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
return 'Already up to date.', True
|
||||
return '', True
|
||||
|
||||
def fake_gateway_restart():
|
||||
gateway_restarts.append('called')
|
||||
def fake_gateway_restart(*, profile=None):
|
||||
gateway_restarts.append(profile)
|
||||
return {'status': 'completed', 'message': 'Gateway service restarted successfully'}
|
||||
|
||||
monkeypatch.setattr(upd, '_run_git', fake_run)
|
||||
@@ -988,7 +1017,7 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
assert result['target'] == 'agent'
|
||||
assert result['restart_scheduled'] is True
|
||||
assert result['gateway_restart'] == 'completed'
|
||||
assert gateway_restarts == ['called']
|
||||
assert gateway_restarts == ['default']
|
||||
|
||||
def test_apply_update_agent_stash_conflict_success_invokes_gateway_restart(self, tmp_path, monkeypatch):
|
||||
import api.updates as upd
|
||||
@@ -1023,8 +1052,8 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
return 'Updating', True
|
||||
return '', True
|
||||
|
||||
def fake_gateway_restart():
|
||||
gateway_restarts.append('called')
|
||||
def fake_gateway_restart(*, profile=None):
|
||||
gateway_restarts.append(profile)
|
||||
return {'status': 'in_progress', 'message': 'Gateway service restart initiated (in progress)'}
|
||||
|
||||
monkeypatch.setattr(upd, '_run_git', fake_run)
|
||||
@@ -1039,7 +1068,7 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
assert result['target'] == 'agent'
|
||||
assert result['restart_scheduled'] is True
|
||||
assert result['gateway_restart'] == 'in_progress'
|
||||
assert gateway_restarts == ['called']
|
||||
assert gateway_restarts == ['default']
|
||||
|
||||
def test_apply_update_agent_without_gateway_restart_result_fails(self, tmp_path, monkeypatch):
|
||||
import api.updates as upd
|
||||
@@ -1066,8 +1095,8 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
monkeypatch.setattr(upd, 'REPO_ROOT', tmp_path)
|
||||
monkeypatch.setattr(upd, '_AGENT_DIR', tmp_path)
|
||||
monkeypatch.setattr(upd, '_schedule_restart', lambda delay=2.0: (_ for _ in ()).throw(AssertionError('must not restart')))
|
||||
monkeypatch.setattr('api.updates.restart_active_profile_gateway', lambda: (
|
||||
restart_calls.append('called'),
|
||||
monkeypatch.setattr('api.updates.restart_active_profile_gateway', lambda **kwargs: (
|
||||
restart_calls.append(kwargs.get('profile')),
|
||||
{'status': 'busy', 'message': 'Restart already in progress. Please wait a moment and try again.'},
|
||||
)[1])
|
||||
|
||||
@@ -1077,7 +1106,7 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
assert result['target'] == 'agent'
|
||||
assert result['gateway_restart'] == 'busy'
|
||||
assert 'hermes gateway restart' in result['message']
|
||||
assert restart_calls == ['called']
|
||||
assert restart_calls == ['default']
|
||||
|
||||
def test_apply_force_update_agent_uses_gateway_restart_status(self, tmp_path, monkeypatch):
|
||||
import api.updates as upd
|
||||
@@ -1101,7 +1130,7 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
monkeypatch.setattr(upd, 'REPO_ROOT', tmp_path)
|
||||
monkeypatch.setattr(upd, '_AGENT_DIR', tmp_path)
|
||||
monkeypatch.setattr(upd, '_schedule_restart', lambda delay=2.0: None)
|
||||
monkeypatch.setattr('api.updates.restart_active_profile_gateway', lambda: {'status': 'completed', 'message': 'Gateway service restarted successfully'})
|
||||
monkeypatch.setattr('api.updates.restart_active_profile_gateway', lambda **kwargs: {'status': 'completed', 'message': 'Gateway service restarted successfully'})
|
||||
|
||||
result = upd.apply_force_update('agent')
|
||||
assert result['ok'] is True
|
||||
@@ -1131,7 +1160,7 @@ class TestAgentUpdateRequiresGatewayRestart:
|
||||
monkeypatch.setattr(upd, '_schedule_restart', lambda delay=2.0: (_ for _ in ()).throw(AssertionError('must not restart')))
|
||||
monkeypatch.setattr(
|
||||
'api.updates.restart_active_profile_gateway',
|
||||
lambda: {'status': 'busy', 'message': 'Restart already in progress. Please wait a moment and try again.'},
|
||||
lambda **kwargs: {'status': 'busy', 'message': 'Restart already in progress. Please wait a moment and try again.'},
|
||||
)
|
||||
|
||||
result = upd.apply_force_update('agent')
|
||||
|
||||
Reference in new issue
Block a user