Skip to content

Commit 7d727fe

Browse files
ndokosndokos
authored
v0.72.0: Fix handling of labeled hosts by pbench-postprocess-tools (#3456)
* v0.72: Fix handling of labeled hosts by pbench-postprocess-tools Fixes #3454 pbench-postprocess-tools mishandles hosts with labels (added by tool registration commands): it ignores the label and complains that it cannot find the tool output directory. The tool output directory path contains '<label>:<host>' as one element in the path but pbench-postprocess-tools looks for a '<host>' element. pbench-postprocess-tools parses the output of pbench-list-tools to get the tool info it needs, but pbench-list-tools omits the label from its output. This PR fixes pbench-list-tools to add the label to its output and pbench-postprocess-tools to parse that output, derive the label and use it to construct the path of the tool output directory. It also adds a "functional" test for pbench-list-tools to verify the output when labeled hosts have registered tools and fixes an incorrect legacy test (test-07) for pbench-postprocess-tools: the test was using a labeled host, but it was testing the wrong tool output directory (without the label). It adds a similar legacy test (test-07.1) to test the unlabeled host case. Bump the version to 0.72.1 PBENCH-1178
1 parent 4391fbc commit 7d727fe

File tree

9 files changed

+119
-29
lines changed

9 files changed

+119
-29
lines changed

‎agent/VERSION‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
0.72.0
1+
0.72.1

‎agent/util-scripts/gold/pbench-postprocess-tools/test-07.1.txt‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
+++ Running test-07.1 pbench-postprocess-tools --group=foobar --dir=/var/tmp/pbench-test-utils/pbench/42-iter/sample42
2+
--- Finished test-07.1 pbench-postprocess-tools (status=0)
3+
iostat: post-processing data
4+
+++ pbench tree state
5+
/var/tmp/pbench-test-utils/pbench
6+
/var/tmp/pbench-test-utils/pbench/42-iter
7+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42
8+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar
9+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com
10+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com/iostat
11+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com/iostat/csv
12+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com/iostat/iostat-postprocess.err
13+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com/iostat/iostat-postprocess.out
14+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com/iostat/iostat-stdout.txt
15+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com/postprocess.log
16+
/var/tmp/pbench-test-utils/pbench/tmp
17+
/var/tmp/pbench-test-utils/pbench/tools-v1-foobar
18+
/var/tmp/pbench-test-utils/pbench/tools-v1-foobar/testhost.example.com
19+
/var/tmp/pbench-test-utils/pbench/tools-v1-foobar/testhost.example.com/foo
20+
/var/tmp/pbench-test-utils/pbench/tools-v1-foobar/testhost.example.com/iostat
21+
=== /var/tmp/pbench-test-utils/pbench/tools-v1-foobar/testhost.example.com/foo:
22+
--interval=10
23+
=== /var/tmp/pbench-test-utils/pbench/tools-v1-foobar/testhost.example.com/iostat:
24+
--interval=10
25+
--- pbench tree state

‎agent/util-scripts/gold/pbench-postprocess-tools/test-07.txt‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ iostat: post-processing data
66
/var/tmp/pbench-test-utils/pbench/42-iter
77
/var/tmp/pbench-test-utils/pbench/42-iter/sample42
88
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar
9-
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com
10-
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com/iostat
11-
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com/iostat/csv
12-
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com/iostat/iostat-postprocess.err
13-
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com/iostat/iostat-postprocess.out
14-
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com/iostat/iostat-stdout.txt
15-
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/testhost.example.com/postprocess.log
9+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/bar:testhost.example.com
10+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/bar:testhost.example.com/iostat
11+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/bar:testhost.example.com/iostat/csv
12+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/bar:testhost.example.com/iostat/iostat-postprocess.err
13+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/bar:testhost.example.com/iostat/iostat-postprocess.out
14+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/bar:testhost.example.com/iostat/iostat-stdout.txt
15+
/var/tmp/pbench-test-utils/pbench/42-iter/sample42/tools-foobar/bar:testhost.example.com/postprocess.log
1616
/var/tmp/pbench-test-utils/pbench/tmp
1717
/var/tmp/pbench-test-utils/pbench/tools-v1-foobar
1818
/var/tmp/pbench-test-utils/pbench/tools-v1-foobar/testhost.example.com

‎agent/util-scripts/pbench-postprocess-tools‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,14 @@ pbench-list-tools --group ${group} --with-option >${tmp} 2>/dev/null
9191
let failures=0
9292
while read hostentry ;do
9393
# Parse an entry from the output of `pbench-list-tools` above.
94-
# The format is: "group: <group>; host: <host>; tools: <tool> <options, <tool> <options>, ...
94+
# The format is: "group: <group>; host: <host> [, label: <label>]; tools: <tool> <options, <tool> <options>, ...
9595
IFS=';' read group_spec host_spec tools_spec <<<"${hostentry}"
96-
IFS=':' read dummy host junk <<<"${host_spec}"
96+
IFS=',' read hostpart labelpart <<<"${host_spec}"
97+
IFS=':' read dummy host junk <<<"${hostpart}"
98+
IFS=':' read dummy label junk <<<"${labelpart}"
9799
IFS=':' read dummy tools_entry junk <<<"${tools_spec}"
98100
host=${host# *}
101+
label=${label# *}
99102
# Associative array: the keys are the tool names, and the values are the options
100103
declare -A tools=()
101104
IFS=',' read -a otools <<<"${tools_entry# *}"
@@ -104,8 +107,11 @@ while read hostentry ;do
104107
tools[${atool[0]}]=${atool[@]:1}
105108
done
106109

107-
# FIXME: add support for label applied to the hostname directory.
108-
host_tool_output_dir="${tool_output_dir}/${host}"
110+
if [[ -z "${label}" ]] ;then
111+
host_tool_output_dir="${tool_output_dir}/${host}"
112+
else
113+
host_tool_output_dir="${tool_output_dir}/${label}:${host}"
114+
fi
109115
if [[ -d "${host_tool_output_dir}" ]]; then
110116
for tool_name in ${!tools[@]} ;do
111117
if [[ ! -x "${pbench_bin}/tool-scripts/${tool_name}" ]]; then

‎agent/util-scripts/samples/pbench-postprocess-tools/test-07.1/pbench/tools-v1-foobar/testhost.example.com/foo‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
--interval=10

‎agent/util-scripts/samples/pbench-postprocess-tools/test-07.1/pbench/tools-v1-foobar/testhost.example.com/iostat‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
--interval=10

‎agent/util-scripts/unittests‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,7 @@ declare -A tools=(
366366
[test-05]="pbench-start-tools"
367367
[test-06]="pbench-stop-tools"
368368
[test-07]="pbench-postprocess-tools"
369+
[test-07.1]="pbench-postprocess-tools"
369370
[test-08]="pbench-kill-tools"
370371
[test-09]="pbench-kill-tools"
371372
[test-10]="pbench-kill-tools"
@@ -451,6 +452,7 @@ declare -A options=(
451452
[test-05]="--group=default --dir=42-iter/sample42"
452453
[test-06]="--group=default --dir=42-iter/sample42"
453454
[test-07]="--group=foobar --dir=${_testdir}/42-iter/sample42"
455+
[test-07.1]="--group=foobar --dir=${_testdir}/42-iter/sample42"
454456
[test-08]="--group=barfoo --dir=42-iter/sample42"
455457
[test-09]="--group=barfoo"
456458
[test-10]="--dir=barfoo"
@@ -566,7 +568,10 @@ declare -A expected_status=(
566568
declare -A pre_hooks=(
567569
[test-05]='ln -s mock-pbench-tool-meister-client ${_testopt}/unittest-scripts/pbench-tool-meister-client'
568570
[test-06]='ln -s mock-pbench-tool-meister-client ${_testopt}/unittest-scripts/pbench-tool-meister-client; mkdir -p ${_testdir}/42-iter/sample42/tools-default/testhost.example.com/iostat; touch ${_testdir}/42-iter/sample42/tools-default/testhost.example.com/iostat/iostat-stdout.txt'
569-
[test-07]='mkdir -p ${_testdir}/42-iter/sample42/tools-foobar/testhost.example.com/iostat; touch ${_testdir}/42-iter/sample42/tools-foobar/testhost.example.com/iostat/iostat-stdout.txt'
571+
# with labeled host
572+
[test-07]='mkdir -p ${_testdir}/42-iter/sample42/tools-foobar/bar:testhost.example.com/iostat; touch ${_testdir}/42-iter/sample42/tools-foobar/bar:testhost.example.com/iostat/iostat-stdout.txt'
573+
# with unlabeled host
574+
[test-07.1]='mkdir -p ${_testdir}/42-iter/sample42/tools-foobar/testhost.example.com/iostat; touch ${_testdir}/42-iter/sample42/tools-foobar/testhost.example.com/iostat/iostat-stdout.txt'
570575
[test-11.16]='mkdir ${_testdir}/tmp; (echo foo; echo bar) > ${_testdir}/tmp/good-remote-file'
571576
[test-11.17]='mkdir ${_testdir}/tmp; touch ${_testdir}/tmp/empty-remote-file'
572577
[test-19]='ln -s mock-pbench-tool-meister-client ${_testopt}/unittest-scripts/pbench-tool-meister-client'
@@ -623,7 +628,10 @@ function sort_tmlogs {
623628
declare -A post_hooks=(
624629
[test-05]='rm ${_testopt}/unittest-scripts/pbench-tool-meister-client'
625630
[test-06]='rm ${_testopt}/unittest-scripts/pbench-tool-meister-client'
626-
[test-07]='cat ${_testdir}/42-iter/sample42/tools-foobar/testhost.example.com/postprocess.log >> ${_testout} 2>&1'
631+
# with labeled host
632+
[test-07]='cat ${_testdir}/42-iter/sample42/tools-foobar/bar:testhost.example.com/postprocess.log >> ${_testout} 2>&1'
633+
# with unlabeled host
634+
[test-07.1]='cat ${_testdir}/42-iter/sample42/tools-foobar/testhost.example.com/postprocess.log >> ${_testout} 2>&1'
627635
[test-19]='rm ${_testopt}/unittest-scripts/pbench-tool-meister-client'
628636
[test-53]='sort_testlog; sort_tmlogs; filter_tmerrs'
629637
[test-54]='sed -Ei "s/^optional arguments:/options:/" ${_testout}'

‎lib/pbench/cli/agent/commands/tools/list.py‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,19 @@ def print_results(toolinfo: dict, with_option: bool) -> bool:
2929
"""
3030
printed = False
3131
for group, gval in sorted(toolinfo.items()):
32-
for host, tools in sorted(gval.items()):
32+
for host, hostitems in sorted(gval.items()):
33+
label = toolinfo[group][host]["label"]
34+
host_string = f"host: {host}" + (f", label: {label}" if label else "")
35+
tools = hostitems["tools"]
3336
if tools:
3437
if not with_option:
35-
s = ", ".join(sorted(tools.keys()))
38+
tool_string = ", ".join(sorted(tools.keys()))
3639
else:
3740
tools_with_options = [
3841
f"{t} {' '.join(v)}" for t, v in sorted(tools.items())
3942
]
40-
s = ", ".join(tools_with_options)
41-
print(f"group: {group}; host: {host}; tools: {s}")
43+
tool_string = ", ".join(tools_with_options)
44+
print(f"group: {group}; {host_string}; tools: {tool_string}")
4245
printed = True
4346
return printed
4447

@@ -68,12 +71,16 @@ def execute(self) -> int:
6871
for path in tg_dir:
6972
host = path.name
7073
tool_info[group][host] = {}
74+
75+
label = path / "__label__"
76+
v = label.read_text().rstrip("\n") if label.exists() else None
77+
tool_info[group][host]["label"] = v
78+
79+
tool_info[group][host]["tools"] = {}
80+
7181
for tool in sorted(self.tools(path)):
72-
tool_info[group][host][tool] = (
73-
sorted((path / tool).read_text().rstrip("\n").split("\n"))
74-
if opts
75-
else ""
76-
)
82+
v = sorted((path / tool).read_text().rstrip("\n").split("\n"))
83+
tool_info[group][host]["tools"][tool] = v if opts else ""
7784

7885
if tool_info:
7986
found = self.print_results(tool_info, self.context.with_option)
@@ -106,13 +113,13 @@ def execute(self) -> int:
106113

107114
host = path.name
108115
tool_info[group][host] = {}
109-
# Check to see if the tool is in any of the hosts.
116+
tool_info[group][host]["label"] = None
117+
tool_info[group][host]["tools"] = {}
118+
119+
# Check if the tool is in any of the hosts.
110120
if tool in self.tools(path):
111-
tool_info[group][host][tool] = (
112-
sorted((path / tool).read_text().rstrip("\n").split("\n"))
113-
if opts
114-
else ""
115-
)
121+
v = sorted((path / tool).read_text().rstrip("\n").split("\n"))
122+
tool_info[group][host]["tools"][tool] = v if opts else ""
116123
found = True
117124

118125
if found:

‎lib/pbench/test/functional/agent/cli/commands/tools/test_tools_list.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,39 @@ def tools_on_multiple_hosts(self, pbench_run):
259259
tool = p / "perf"
260260
tool.write_text("--record-opts='-a --freq=100'")
261261

262+
# Issue #3454
263+
@pytest.fixture
264+
def labels_on_multiple_hosts(self, pbench_run, tools_on_multiple_hosts):
265+
# This fixture is meant to be called after the previous one
266+
# (tools_on_multiple_hosts). The previous one establishes a
267+
# tool-like directory structure; this one just embellishes it
268+
# with labels on some host entries. Think of it as a
269+
# double for-loop, like the one above, only unrolled.
270+
271+
# row 1
272+
group = "default"
273+
274+
# column 1
275+
host = "th1.example.com"
276+
label = pbench_run / f"tools-v1-{group}" / host / "__label__"
277+
label.write_text("foo")
278+
279+
# column 2
280+
host = "th2.example.com"
281+
label = pbench_run / f"tools-v1-{group}" / host / "__label__"
282+
label.write_text("bar")
283+
284+
# row 2
285+
group = "test"
286+
287+
# column 1
288+
host = "th1.example.com"
289+
label = pbench_run / f"tools-v1-{group}" / host / "__label__"
290+
label.write_text("bar")
291+
292+
# column 2
293+
# th2 has no label
294+
262295
# Issue #2346
263296
def test_existing_group_options(self, single_group_tools, agent_config):
264297
command = ["pbench-list-tools", "--with-option"]
@@ -309,3 +342,12 @@ def test_multiple_hosts_with_options(self, tools_on_multiple_hosts, agent_config
309342
b"group: default; host: th1.example.com; tools: iostat --interval=30, mpstat --interval=300, sar --interval=10\ngroup: default; host: th2.example.com; tools: iostat --interval=30, mpstat --interval=300, perf --record-opts='-a --freq=100'\ngroup: test; host: th1.example.com; tools: iostat --interval=30, mpstat --interval=300, sar --interval=10\ngroup: test; host: th2.example.com; tools: iostat --interval=30, mpstat --interval=300, perf --record-opts='-a --freq=100'"
310343
in out
311344
)
345+
346+
def test_multiple_hosts_with_labels(self, labels_on_multiple_hosts, agent_config):
347+
command = ["pbench-list-tools", "--with-option"]
348+
out, err, exitcode = pytest.helpers.capture(command)
349+
assert EMPTY == err and exitcode == 0
350+
assert (
351+
b"group: default; host: th1.example.com, label: foo; tools: iostat --interval=30, mpstat --interval=300, sar --interval=10\ngroup: default; host: th2.example.com, label: bar; tools: iostat --interval=30, mpstat --interval=300, perf --record-opts='-a --freq=100'\ngroup: test; host: th1.example.com, label: bar; tools: iostat --interval=30, mpstat --interval=300, sar --interval=10\ngroup: test; host: th2.example.com; tools: iostat --interval=30, mpstat --interval=300, perf --record-opts='-a --freq=100'"
352+
in out
353+
)

0 commit comments

Comments
 (0)