Skip to content

Commit 9d90a97

Browse files
committed
Ensure use of localhost when stopping Redis
This is a back-port of commit c84c6ed (PR #2863) from `main`. When `pbench-tool-meister-stop` is invoked where the Redis server was created locally by `pbench-tool-meister-start`, we always want to use the `localhost` IP address to talk to it. Also added unit tests for the `tool_meister_stop.py` module's `RedisServer` class, and corrected the `pbench-tool-meister-stop` CLI param default value for the `--redis-server` switch. Fixes issue #2861 [1]. [1] #2861
1 parent acae28b commit 9d90a97

File tree

2 files changed

+46
-1
lines changed

2 files changed

+46
-1
lines changed

lib/pbench/agent/tool_meister_stop.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@ def __init__(self, spec: str, benchmark_run_dir: Path, def_host_name: str):
6565
)
6666
except FileNotFoundError:
6767
pass
68+
else:
69+
# The redis.pid file exists in the "tm" directory, which means this
70+
# Redis server is locally managed. Communication with it will
71+
# always be through the "localhost" interface.
72+
self.host = "localhost"
6873

6974
def locally_managed(self) -> bool:
7075
return self.pid_file is not None
@@ -286,7 +291,7 @@ def main():
286291
parser.add_argument(
287292
"--redis-server",
288293
dest="redis_server",
289-
default=os.environ.get("PBENCH_REDIS_SERVER", None),
294+
default=os.environ.get("PBENCH_REDIS_SERVER", ""),
290295
help=(
291296
"Use an existing Redis server specified by <hostname>:<port>;"
292297
" implies the use of an existing Tool Data Sink and Tool Meisters"
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
"""Tests for the Tool Meister "stop" module.
2+
"""
3+
from pbench.agent.tool_meister_stop import RedisServer
4+
5+
6+
class TestRedisServer:
7+
"""Verify the RedisServer class use by Tool Meister "stop"."""
8+
9+
def test_locally_managed(self, tmp_path):
10+
# Locally managed means we have a run directory, ...
11+
rundir = tmp_path / "run-dir"
12+
rundir.mkdir()
13+
# ... with a "tm" sub-directory, ...
14+
tmdir = rundir / "tm"
15+
tmdir.mkdir()
16+
# ... containing a "redis.pid" file.
17+
pidfile = tmdir / "redis.pid"
18+
pidfile.write_text("12345")
19+
20+
rs = RedisServer("", rundir, "notme.example.com")
21+
assert (
22+
rs.locally_managed()
23+
), "RedisServer incorrectly inferred a non-locally managed instance from a run directory with a 'tm/redis.pid' file"
24+
assert (
25+
rs.host == "localhost"
26+
), f"Expected 'RedisServer.host' to be 'localhost', got '{rs.host}'"
27+
28+
def test_not_locally_managed(self, tmp_path):
29+
# Empty benchmark run directory indicates not locally managed.
30+
rundir = tmp_path / "empty-run-dir"
31+
rundir.mkdir()
32+
33+
rs_host = "redis.example.com"
34+
rs = RedisServer(f"{rs_host}:4343", rundir, "notme.example.com")
35+
assert (
36+
not rs.locally_managed()
37+
), "RedisServer incorrectly inferred a locally managed instance from an empty run directory"
38+
assert (
39+
rs.host == "redis.example.com"
40+
), f"Expected 'RedisServer.host' to be '{rs_host}', got '{rs.host}'"

0 commit comments

Comments
 (0)