Skip to content

Commit 775da21

Browse files
ndokosndokos
authored
Fix handling of labeled hosts by pbench-postprocess-tools (fwd port of #3456) (#3472)
* Fix handling of labeled hosts by pbench-postprocess-tools (forward port of #3456) 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 1.0 PBENCH-1178
1 parent bfdc613 commit 775da21

File tree

9 files changed

+153
-76
lines changed

9 files changed

+153
-76
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+
1.0

‎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
@@ -87,11 +87,14 @@ fi
8787
let failures=0
8888
pbench-list-tools --group ${group} --with-option 2>/dev/null | while read hostentry ;do
8989
# Parse an entry from the output of `pbench-list-tools` above.
90-
# The format is: "group: <group>; host: <host>; tools: <tool> <options, <tool> <options>, ...
90+
# The format is: "group: <group>; host: <host> [, label: <label>]; tools: <tool> <options>, <tool> <options>, ...
9191
IFS=';' read group_spec host_spec tools_spec <<<"${hostentry}"
92-
IFS=':' read dummy host junk <<<"${host_spec}"
92+
IFS=',' read hostpart labelpart <<<"${host_spec}"
93+
IFS=':' read dummy host junk <<<"${hostpart}"
94+
IFS=':' read dummy label junk <<<"${labelpart}"
9395
IFS=':' read dummy tools_entry junk <<<"${tools_spec}"
9496
host=${host# *}
97+
label=${label# *}
9598
# Associative array: the keys are the tool names, and the values are the options
9699
declare -A tools=()
97100
IFS=',' read -a otools <<<"${tools_entry# *}"
@@ -100,8 +103,11 @@ pbench-list-tools --group ${group} --with-option 2>/dev/null | while read hosten
100103
tools[${atool[0]}]=${atool[@]:1}
101104
done
102105

103-
# FIXME: add support for label applied to the hostname directory.
104-
host_tool_output_dir="${tool_output_dir}/${host}"
106+
if [[ -z "${label}" ]] ;then
107+
host_tool_output_dir="${tool_output_dir}/${host}"
108+
else
109+
host_tool_output_dir="${tool_output_dir}/${label}:${host}"
110+
fi
105111
if [[ -d "${host_tool_output_dir}" ]]; then
106112
for tool_name in ${!tools[@]} ;do
107113
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: 54 additions & 62 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 = hostitems["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:
37-
tools_with_options = [
40+
tools_with_options = (
3841
f"{t} {' '.join(v)}" for t, v in sorted(tools.items())
39-
]
40-
s = ", ".join(tools_with_options)
41-
print(f"group: {group}; host: {host}; tools: {s}")
42+
)
43+
tool_string = ", ".join(tools_with_options)
44+
print(f"group: {group}; {host_string}; tools: {tool_string}")
4245
printed = True
4346
return printed
4447

@@ -54,76 +57,65 @@ def execute(self) -> int:
5457
groups = self.groups
5558

5659
opts = self.context.with_option
60+
toolname = self.context.name
61+
found = False
5762
tool_info = {}
5863

59-
if not self.context.name:
60-
for group in groups:
61-
tool_info[group] = {}
62-
try:
63-
tg_dir = self.gen_tools_group_dir(group).glob("*/**")
64-
except BadToolGroup:
65-
self.logger.error("Bad tool group: %s", group)
66-
return 1
67-
68-
for path in tg_dir:
69-
host = path.name
70-
tool_info[group][host] = {}
71-
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-
)
64+
for group in groups:
65+
tool_info[group] = {}
66+
try:
67+
tg_dir = self.gen_tools_group_dir(group)
68+
except BadToolGroup:
69+
self.logger.error("Bad tool group: %s", group)
70+
return 1
7771

78-
if tool_info:
79-
found = self.print_results(tool_info, self.context.with_option)
80-
if not found:
81-
msg = "No tools found"
82-
if self.context.group:
83-
msg += f' in group "{self.context.group[0]}"'
84-
self.logger.warn(msg)
85-
else:
86-
self.logger.warn("No tool groups found")
72+
for path in tg_dir.iterdir():
73+
# skip __trigger__ if present
74+
if not path.is_dir():
75+
continue
8776

88-
return 0
77+
host = path.name
78+
tool_info[group][host] = {"label": None, "tools": {}}
8979

90-
else:
91-
# List the groups which include this tool
92-
tool = self.context.name
93-
found = False
94-
for group in groups:
95-
try:
96-
tg_dir = self.gen_tools_group_dir(group)
97-
except BadToolGroup:
98-
self.logger.error("Bad tool group: %s", group)
99-
return 1
100-
101-
tool_info[group] = {}
102-
for path in tg_dir.iterdir():
103-
# skip files like __label__ and __trigger__
104-
if not path.is_dir():
105-
continue
106-
107-
host = path.name
108-
tool_info[group][host] = {}
109-
# Check to see if the tool is in any of the hosts.
110-
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-
)
116-
found = True
80+
toolsdict = tool_info[group][host]["tools"]
81+
if toolname:
82+
# Check if the tool is in any of the hosts.
83+
found = toolname in self.tools(path)
84+
toolslist = [toolname] if found else []
85+
else:
86+
# no tool name was specified
87+
toolslist = sorted(self.tools(path))
11788

89+
label = path / "__label__"
90+
v = label.read_text().rstrip("\n") if label.exists() else None
91+
tool_info[group][host]["label"] = v
92+
93+
for tool in toolslist:
94+
v = (path / tool).read_text().rstrip("\n").split("\n")
95+
toolsdict[tool] = sorted(v) if opts else ""
96+
97+
if toolname:
11898
if found:
11999
self.print_results(tool_info, self.context.with_option)
120100
return 0
121101
else:
122-
msg = f'Tool "{self.context.name}" not found in '
102+
msg = f'Tool "{toolname}" not found in '
123103
msg += self.context.group[0] if self.context.group else "any group"
124104
self.logger.error(msg)
125105
return 1
126106

107+
elif tool_info:
108+
found = self.print_results(tool_info, self.context.with_option)
109+
if not found:
110+
msg = "No tools found"
111+
if self.context.group:
112+
msg += f' in group "{self.context.group[0]}"'
113+
self.logger.warn(msg)
114+
else:
115+
self.logger.warn("No tool groups found")
116+
117+
return 0
118+
127119

128120
def _group_option(f):
129121
"""Group name option"""

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,41 @@ 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+
# Note that this fixture depends on `tools_on_multiple_hosts'.
266+
# It embelishes the structure produced by
267+
# `tools_on_multiple_hosts' by adding labels to some host
268+
# entries in the tool-like directory structure that
269+
# `tools_on_multiple_hosts' establishes. Think of it as a
270+
# double for-loop, like the one in `tools_on_multiple_hosts',
271+
# only unrolled.
272+
273+
# row 1
274+
group = "default"
275+
276+
# column 1
277+
host = "th1.example.com"
278+
label = pbench_run / f"tools-v1-{group}" / host / "__label__"
279+
label.write_text("foo")
280+
281+
# column 2
282+
host = "th2.example.com"
283+
label = pbench_run / f"tools-v1-{group}" / host / "__label__"
284+
label.write_text("bar")
285+
286+
# row 2
287+
group = "test"
288+
289+
# column 1
290+
host = "th1.example.com"
291+
label = pbench_run / f"tools-v1-{group}" / host / "__label__"
292+
label.write_text("bar")
293+
294+
# column 2
295+
# th2 has no label
296+
262297
# Issue #2346
263298
def test_existing_group_options(self, single_group_tools, agent_config):
264299
command = ["pbench-list-tools", "--with-option"]
@@ -309,3 +344,12 @@ def test_multiple_hosts_with_options(self, tools_on_multiple_hosts, agent_config
309344
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'"
310345
in out
311346
)
347+
348+
def test_multiple_hosts_with_labels(self, labels_on_multiple_hosts, agent_config):
349+
command = ["pbench-list-tools", "--with-option"]
350+
out, err, exitcode = pytest.helpers.capture(command)
351+
assert EMPTY == err and exitcode == 0
352+
assert (
353+
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'"
354+
in out
355+
)

0 commit comments

Comments
 (0)