v0.72.0: Fix handling of labeled hosts by pbench-postprocess-tools #3456
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ndokos
added
bug
Agent
tools
Fixes distributed-system-analysis#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 commit 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. The next commit deals with tests. PBENCH-1178
This commit adds a "functional" test for pbench-list-tools to verify the output when labeled hosts have registered tools. It also 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.
dbutenhof
previously approved these changes
Jun 7, 2023
dbutenhof
previously approved these changes
Jun 7, 2023
There was a problem hiding this comment.
OK; the apparent React fix (which I won't mention here as I don't really want the "mention" to show up in the React PR) has an approval but hasn't been submitted. Still, we might be able to retract the workaround soon ... but leaving it on the b0.72 branch is probably fine anyway.
webbnh
previously approved these changes
Jun 7, 2023
...s/samples/pbench-postprocess-tools/test-07.1/pbench/tools-v1-foobar/testhost.example.com/foo
Outdated
Show resolved
Hide resolved
lib/pbench/test/functional/agent/cli/commands/tools/test_tools_list.py
Outdated
Show resolved
Hide resolved
dbutenhof
previously approved these changes
Jun 8, 2023
lib/pbench/test/functional/agent/cli/commands/tools/test_tools_list.py
Outdated
Show resolved
Hide resolved
webbnh
reviewed
Jun 8, 2023
lib/pbench/test/functional/agent/cli/commands/tools/test_tools_list.py
Outdated
Show resolved
Hide resolved
webbnh
previously approved these changes
Jun 8, 2023
dbutenhof
reviewed
Jun 9, 2023
dbutenhof
previously approved these changes
Jun 9, 2023
Fix comment in pbench-postprocess-tools. Rework ugly code in list.py.
I think this is the last one.
|
I dropped the React commit (that's the only change). We'll see what Jenkins thinks. |
dbutenhof
approved these changes
Jun 9, 2023
ndokos
added a commit
to ndokos/pbench
that referenced
this pull request
Jun 26, 2023
…rt of distributed-system-analysis#3456) Fixes distributed-system-analysis#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
ndokos
added a commit
to ndokos/pbench
that referenced
this pull request
Jun 27, 2023
…rt of distributed-system-analysis#3456) Fixes distributed-system-analysis#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
ndokos
added a commit
that referenced
this pull request
Jul 3, 2023
…#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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3454
pbench-postprocess-toolsmishandles 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 butpbench-postprocess-toolslooks for a<host>element.pbench-postprocess-toolsparses the output ofpbench-list-toolsto get the tool info it needs, butpbench-list-toolsomits the label from its output.The first commit fixes
pbench-list-toolsto add the label to its output andpbench-postprocess-toolsto parse that output, derive the label and use it to construct the path of the tool output directory.The second commit deals with tests.
PBENCH-1178