Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/report-script-result into
lp:maas.
Commit message:
Updated protocol for signaling commissioning-script results to the metadata API.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/report-script-result/+merge/140650
Discussed this with Raphaël. There were some design mismatches in how a
commissioning script (such as the lshw script) would report its output files
and its return code to the signal() metadata API call.
The new design is as follows. Many commissioning scripts may be run in the
process of commissioning one node. Each now produces at least one, or at most
two output files (we'll see about additional files if we ever really need
them): stdout and stderr. And of course each script run returns a numeric Unix
return code. All of these outputs of a single script are now reported at once,
in one signal() call. The stderr file is omitted if there was no error output.
We still want the stdout file even if it's empty, because the storage of
return values is associated with these files. There has to be at least one.
Signalling the result of one script still has the status "WORKING," indicating
that it is a kind of progress report. We may still want to change that; it's a
simple matter of changing that string in signal_result() in the shell code.
But there's also a new parameter: script_result. This stores the script's
numerical return value.
The old code reported return values individually per script it ran (in an
uncomfortably loosely-structured way) and then gathered up and sent all the
output files. Nothing very firmly associated scripts with files, except
stdout/stderr files have names predictably tied to the script. In the new
setup, that is still the case but stdout/stderr are the only files.
Now, about implementation.
I was honestly a little worried that this line would cause trouble:
local files="--stdout=$stdout"
if [ ... ]
then
files="$files --stderr=$stderr"
fi
Now wouldn't $files expand to a single string that might contain spaces? Turns
out it doesn't. I wrote a simple python script that accepted some of the same
options, and defined a version of signal_result() in my shell that called this
python script. Then I ran some basic manual tests with that experimental
setup, in lieu of unit tests. It did indeed pass two --file=$x options,
exactly as hoped. What if a filename contains spaces? Kibo help us. Unix
shell quoting always scares me.
Jeroen
--
https://code.launchpad.net/~jtv/maas/report-script-result/+merge/140650
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jtv/maas/report-script-result into lp:maas.
=== modified file 'src/metadataserver/commissioning/snippets/maas_signal.py'
--- src/metadataserver/commissioning/snippets/maas_signal.py 2012-12-18 12:31:47 +0000
+++ src/metadataserver/commissioning/snippets/maas_signal.py 2012-12-19 12:41:21 +0000
@@ -105,6 +105,9 @@
help="Power type.", choices=POWER_TYPES, default=None)
parser.add_argument("--power-parameters", dest='power_parms',
help="Power parameters.", default=None)
+ parser.add_argument(
+ "--script-result", metavar="retval", type=int, dest='script_result',
+ help="Return code of a commissioning script.")
parser.add_argument("status",
help="Status", choices=VALID_STATUS, action='store')
@@ -132,7 +135,11 @@
params = {
"op": "signal",
"status": args.status,
- "error": args.message}
+ "error": args.message,
+ }
+
+ if args.script_result:
+ params['script_result'] = args.script_result
for ent in args.posts:
try:
=== modified file 'src/metadataserver/commissioning/user_data.template'
--- src/metadataserver/commissioning/user_data.template 2012-12-18 12:31:47 +0000
+++ src/metadataserver/commissioning/user_data.template 2012-12-19 12:41:21 +0000
@@ -77,10 +77,29 @@
return 1
}
+# Invoke the "signal()" API call to report progress.
+# Usage: signal <status> <message>
signal() {
maas-signal "--config=${CRED_CFG}" "$@"
}
+# Report result of a commissioning script: output file, error output
+# file if there was any error output, and return code.
+# Usage: signal <return-value> <message> <stdout-file> <stderr-file>
+signal_result() {
+ local result=$1 message="$2" stdout="$3" stderr="$4"
+ local files="--file=$stdout"
+ if [ -f "$stderr" -a -s "$stderr" ]
+ then
+ files="$files --file=$stderr"
+ fi
+ maas-signal \
+ "--config=${CRED_CFG}" \
+ "--script-result=$result" \
+ $files \
+ WORKING "$message"
+}
+
fail() {
[ -z "$CRED_CFG" ] || signal FAILED "$1"
echo "FAILED: $1" 1>&2;
@@ -153,10 +172,13 @@
for script in "${SCRIPTS_D}/"*; do
[ -f "$script" -a -f "$script" ] || continue
name=${script##*/}
- signal WORKING "starting ${script##*/} [$cur/$total]"
+ signal WORKING "starting ${name} [$cur/$total]"
"$script" > "${OUT_D}/${name}.out" 2> "${OUT_D}/${name}.err"
ret=$?
- signal WORKING "finished $name [$cur/$total]: $ret"
+ signal_result \
+ "$ret" "finished $name [$cur/$total]: $ret" \
+ "${OUT_D}/${name}.out" \
+ "${OUT_D}/${name}.err"
if [ $ret -eq 0 ]; then
numpass=$(($numpass+1))
failed="${failed} ${name}"
@@ -164,20 +186,13 @@
cur=$(($cur+1))
done
- # Get a list of all files created, ignoring empty ones.
- local fargs=""
- for file in "${OUT_D}/"*; do
- [ -f "$file" -a -s "$file" ] || continue
- fargs="$fargs --file=${file##*/}"
- done
-
if [ $numpass -eq $total ]; then
( cd "${OUT_D}" &&
- signal $fargs OK "finished [$numpass/$total]" )
+ signal OK "finished [$numpass/$total]" )
return 0
else
( cd "${OUT_D}" &&
- signal $fargs OK "failed [$numpass/$total] ($failed)" )
+ signal OK "failed [$numpass/$total] ($failed)" )
return $(($count-$numpass))
fi
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
