Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

powerman: when status and status_all are defined, use status_all only on full pluglist #156

Merged
merged 5 commits into from
Mar 3, 2024
Merged

powerman: when status and status_all are defined, use status_all only on full pluglist #156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
db02fcc
t/simulators: add missing "goto" in vpcd simulator
chu11 Mar 2, 2024
0c1a1d7
etc/devices: remove untested status scripts
chu11 Mar 2, 2024
4e7ea64
powerman: do not always call "all" query script
chu11 Feb 27, 2024
9992f96
t: test which status script is used
chu11 Mar 2, 2024
ac7726c
man: add info on status scripts in powerman.dev(5)
chu11 Mar 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 0 additions & 60 deletions etc/devices/appro-gb2.dev
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,6 @@ specification "sr5110" {
}
expect "-iSCB> "
}
script status {
send "pmnode %s\r"
expect "node([0-9]+): (on|off|n/a)"
Comment on lines -47 to -49
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the commit message for this, it would probably be good to mention that the current behavior is changing so that the untested singletons would start to get used, so this change just preserves the existing behavior.

setplugstate $1 $2 on="on" off="off"
expect "-iSCB> "
}
script status_beacon {
send "pmled %s\r"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "-iSCB> "
}
script on {
send "power on %s\r"
expect "-iSCB> "
Expand Down Expand Up @@ -119,18 +107,6 @@ specification "sr5110_gpu" {
}
expect "-iSCB> "
}
script status {
send "pmnode %s\r"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "-iSCB> "
}
script status_beacon {
send "pmled %s\r"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "-iSCB> "
}
script on {
send "power on %s\r"
expect "-iSCB> "
Expand Down Expand Up @@ -195,18 +171,6 @@ specification "sr8116" {
}
expect "-iSCB> "
}
script status {
send "pmnode %s\r"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "-iSCB> "
}
script status_beacon {
send "pmled %s\r"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "-iSCB> "
}
script on {
send "power on %s\r"
expect "-iSCB> "
Expand Down Expand Up @@ -270,18 +234,6 @@ specification "sr8116_gpu" {
}
expect "-iSCB> "
}
script status {
send "pmnode %s\r"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "-iSCB> "
}
script status_beacon {
send "pmled %s\r"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "-iSCB> "
}
script on {
send "power on %s\r"
expect "-iSCB> "
Expand Down Expand Up @@ -345,18 +297,6 @@ specification "sr8104" {
}
expect "-iSCB> "
}
script status {
send "pmnode %s\r"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "-iSCB> "
}
script status_beacon {
send "pmled %s\r"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "-iSCB> "
}
script on {
send "power on %s\r"
expect "-iSCB> "
Expand Down
42 changes: 0 additions & 42 deletions etc/devices/appro-greenblade.dev
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,6 @@ specification "iscb-gb" {
expect "ok"
expect "iSCB-[0-9]+:[0-9]> "
}
script status {
send "pmnode %s\r\n"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "ok"
expect "iSCB-[0-9]+:[0-9]> "
}
script status_beacon {
send "pmled %s\r\n"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "ok"
expect "iSCB-[0-9]+:[0-9]> "
}
script on {
send "on %s\r\n"
expect "ok"
Expand Down Expand Up @@ -138,20 +124,6 @@ specification "iscb-gbgpu" {
expect "ok"
expect "iSCB-[0-9]+:[0-9]> "
}
script status {
send "pmnode %s\r\n"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "ok"
expect "iSCB-[0-9]+:[0-9]> "
}
script status_beacon {
send "pmled %s\r\n"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "ok"
expect "iSCB-[0-9]+:[0-9]> "
}
script on {
send "on %s\r\n"
expect "ok"
Expand Down Expand Up @@ -228,20 +200,6 @@ specification "iscb-hybrid" {
expect "ok"
expect "iSCB-[0-9]+:[0-9]> "
}
script status {
send "pmnode %s\r\n"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "ok"
expect "iSCB-[0-9]+:[0-9]> "
}
script status_beacon {
send "pmled %s\r\n"
expect "node([0-9]+): (on|off|n/a)"
setplugstate $1 $2 on="on" off="off"
expect "ok"
expect "iSCB-[0-9]+:[0-9]> "
}
script on {
send "on %s\r\n"
expect "ok"
Expand Down
6 changes: 0 additions & 6 deletions etc/devices/swpdu.dev
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,6 @@ specification "swpdu" {
}
expect "swpdu> "
}
script status {
send "status %s\r\n"
expect "port([0-9]+)[^\n]*(on|off|unknown|^n)"
setplugstate $1 $2 on="on" off="off"
expect "swpdu> "
}
script on {
send "on %s\r\n"
expect "swpdu> "
Expand Down
20 changes: 16 additions & 4 deletions man/powerman.dev.5.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,11 @@ Get device in a state so login script will work
(though hopefully disconnecting will do that too).
.TP
.I "status_all, status[%s]"
Obtain plug state for all plugs or only the specified plug.
Obtain plug state for all plugs or only the specified plug. In most
cases, only one script needs to be specified. In some hardware where
unpopulated plugs may be problematic, it may be beneficial to specify
both. If both scripts are specified, the status_all script will be
called only when all plug are requested.
.TP
.I "on_all, on_ranged[%s], on[%s]"
Power on all plugs, a range of plugs, or the specified plug.
Expand All @@ -107,11 +111,19 @@ and a probe into the node attached to a non-standby power source.
Obtain temperature reading for all plugs or only the specified plug.
Temperature is obtained by sampling a thermocouple in the node.
Results are reported as a text string - not interpreted by Powerman
beyond any regex chopping done by the script.
beyond any regex chopping done by the script. In most cases, only one
script needs to be specified. In some hardware where unpopulated
plugs may be problematic, it may be beneficial to specify both. If
both scripts are specified, the status_all script will be called only
when all plug are requested.
.TP
.I "status_beacon_all, status_beacon[%s]"
Obtain beacon state for all plugs or only the specified plug.
Some RPC's include a way to flash a light on a node.
Obtain beacon state for all plugs or only the specified plug. Some
RPC's include a way to flash a light on a node. In most cases, only
one script needs to be specified. In some hardware where unpopulated
plugs may be problematic, it may be beneficial to specify both. If
both scripts are specified, the status_all script will be called only
when all plugs are requested.
.TP
.I "beacon_on[%s]"
Flash beacon on the specified plug.
Expand Down
6 changes: 5 additions & 1 deletion src/powerman/device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,8 +683,12 @@ static int _enqueue_targeted_actions(Device * dev, int com, hostlist_t hl,
pluglist_iterator_destroy(itr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit message:

Solution: Only call the "all" script if it is a query and a singlet
version of the script does not exist.

Makes it sound like "all" is never called if the singlet exists. Maybe "only call the all query script if all plugs are requested OR the singlet version of the script is not defined."?

For device files where
unpopulated nodes may exist, it can define an all script and singlet
script for each scenario.

s/nodes/plugs/


/* Try _all version of script.
*
* - use if action is a query, unless no singlet version exists
* (i.e. if there is no singlet version, we have to use the _all
* version)
*/
if (all || _is_query_action(com)) {
if (all || (_is_query_action(com) && dev->scripts[com] == NULL)) {
int ncom = _get_all_script(dev, com);

if (ncom != -1) {
Expand Down
1 change: 1 addition & 0 deletions t/simulators/vpcd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ _prompt_loop(void)
continue;
}
printf("plug %d: %d\n", i, temp[i]);
goto ok;
}
if (strcmp(buf, "temp *") == 0) { /* temp * */
for (i = 0; i < NUM_PLUGS; i++)
Expand Down
66 changes: 66 additions & 0 deletions t/t0004-status-query.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,72 @@ test_expect_success 'stop powerman daemon and device server' '
wait &&
wait
'
test_expect_success 'create new powerman.conf with both status and status_all script' '
cat >powerman4.conf <<-EOT4
specification "vpc" {
timeout 5.0
plug name { "0" "1" "2" "3" "4" "5" "6" "7" "8"
"9" "10" "11" "12" "13" "14" "15" }
script login {
send "login\n"
expect "[0-9]* OK\n"
expect "[0-9]* vpc> "
}
script logout {
send "logoff\n"
expect "[0-9]* OK\n"
}
script status {
send "stat %s\n"
expect "plug ([0-9]+): (ON|OFF)\n"
setplugstate \$1 \$2 on="ON" off="OFF"
expect "[0-9]* OK\n"
expect "[0-9]* vpc> "
}
script status_all {
send "stat *\n"
foreachplug {
expect "plug ([0-9]+): (ON|OFF)\n"
setplugstate \$1 \$2 on="ON" off="OFF"
}
expect "[0-9]* OK\n"
expect "[0-9]* vpc> "
}
}
listen "$testaddr"
device "test0" "vpc" "$vpcd |&"
node "t[0-15]" "test0"
EOT4
'
test_expect_success 'start powerman daemon and wait for it to start' '
$powermand -c powerman4.conf &
echo $! >powermand4.pid &&
$powerman --retry-connect=100 --server-host=$testaddr -q >/dev/null
'
test_expect_success 'powerman -q works' '
$powerman -h $testaddr -q >all_query.out &&
makeoutput "" "t[0-15]" "" >all_query.exp &&
test_cmp all_query.exp all_query.out
'
test_expect_success 'powerman -q uses status_all script on all plugs' '
$powerman -h $testaddr -T -q >all_queryT.out &&
count=`grep "stat" all_queryT.out | wc -l` &&
test $count = 1
'
test_expect_success 'powerman -q t[1-15] works' '
$powerman -h $testaddr -q t[1-15] >most_query.out &&
makeoutput "" "t[1-15]" "" >most_query.exp &&
test_cmp most_query.exp most_query.out
'
test_expect_success 'powerman -q uses status script on not all plugs' '
$powerman -h $testaddr -T -q t[1-15] >most_queryT.out &&
count=`grep "stat" most_queryT.out | wc -l` &&
test $count = 15
'
test_expect_success 'stop powerman daemon' '
kill -15 $(cat powermand4.pid) &&
wait
'

test_done

Expand Down
Loading
Loading