Correct pbench-fio and pbench-uperf handling of benchmark existence and version checks (take 2)

agent/base

@@ -291,20 +291,18 @@ function generate_inventory {
     done
 }
 
-function resolve_benchmark_bin {
-    # Encapsulation of method for resolving the actual benchmark
-    # binary.
-    which --skip-alias --skip-functions "${1}"
-}
-
 function vercmp {
     # Compare two package versions via `rpmdev-vercmp`.
     local _package="${1}"
     local _match="${2}"
     local _exp_ver="${3}"
     local _ins_ver="${4}"
 
-    local _stderr=$(rpmdev-vercmp "${_ins_ver}" "${_exp_ver}" 2>&1 > /dev/null)
+    # Note we declare `_stderr` local without an initializer because the return
+    # code of rpmdev-vercmp would be overwritten by the return code of the
+    # "local" declaration.
+    local _stderr
+    _stderr=$(rpmdev-vercmp "${_ins_ver}" "${_exp_ver}" 2>&1 > /dev/null)
     local _res=${?}
     if [[ ${_res} -ne 0 && ${_res} -ne 11 && ${_res} -ne 12 ]]; then
         error_log "rpmdev-vercmp - ${_stderr}"

agent/bench-scripts/pbench-fio

@@ -11,16 +11,7 @@ pbench_bin="`cd ${script_path}/..; /bin/pwd`"
 . "$pbench_bin"/base
 
 export benchmark="fio"
-benchmark_rpm=${benchmark}
 export benchmark_run_dir=""
-# allow unit tests to override
-if [[ -z "${benchmark_bin}" ]]; then
-	benchmark_bin="$(resolve_benchmark_bin "${benchmark}")"
-	if [[ -z "${benchmark_bin}" ]]; then
-		error_log "[${script_name}] ${benchmark} executable not found on PATH=${PATH}"
-		exit 1
-	fi
-fi
 
 job_file="${script_path}/templates/fio.job"
 
@@ -168,6 +159,7 @@ function usage {
 }
 
 function fio_process_options() {
+	local opts
 	opts=$(getopt -q -o b:c:d:hj:r:s:t: --longoptions "help,max-stddev:,max-failures:,samples:,direct:,sync:,pre-check,clients:,client-file:,iodepth:,ioengine:,config:,jobs-per-dev:,job-mode:,rate-iops:,ramptime:,runtime:,test-types:,block-sizes:,file-size:,targets:,tool-group:,postprocess-only:,run-dir:,numjobs:,job-file:,sysinfo:,pre-iteration-script:,histogram-interval-sec:,histogram-interval-msec:,unique-ports" -n "getopt.sh" -- "$@")
 	if [ $? -ne 0 ]; then
 		printf -- "${script_name} $*\n"
@@ -177,6 +169,7 @@ function fio_process_options() {
 		exit 1
 	fi
 	eval set -- "$opts"
+	local arg
 	while true; do
 		arg="${1}"
 		shift
@@ -479,7 +472,12 @@ function local_pre_check() {
 
 	# First check for the supported version of fio in use.
 
-	local _stdout=$(${benchmark_bin} --version)
+	local _stdout
+	_stdout=$(${benchmark} --version 2>&1)
+	if [[ ${?} -ne 0 ]]; then
+		error_log "[${script_name}] ${benchmark} failed reporting its version: '${_stdout}'"
+		exit 1
+	fi
 	local ver_installed=${_stdout##*-}
 
 	vercmp "${benchmark}" "${match}" "${ver}" "${ver_installed}"
@@ -528,8 +526,7 @@ function fio_pre_check() {
 	fi
 	if [[ ${do_local_pre_check} -eq 1 ]]; then
 		debug_log "Running pre-check locally"
-		local_pre_check "${devs}" "${ver}" "${match}"
-		rc=${?}
+		local_pre_check "${devs}" "${ver}" "${match}" || rc=${?}
 	fi
 	if [[ ${rc} -ne 0 ]]; then
 		error_log "pre-check(s) failed, exiting"
@@ -598,7 +595,7 @@ function fio_run_job() {
 
 	if [ -n "${clients}" ]; then
 		for client in "${client_names[@]}"; do
-			local bash_c_cmd="'${benchmark_bin}' --server=,${client_ports[${client}]} > client-result.txt 2> client-result.err"
+			local bash_c_cmd="${benchmark} --server=,${client_ports[${client}]} > client-result.txt 2> client-result.err"
 			local screen_it="screen -L -Logfile fio-server.screen.log -dmS fio-server bash -c ${bash_c_cmd}"
 			if pbench-is-local ${client}; then
 				debug_log "killing any old local fio processes and starting new a fio process for the local client"
@@ -653,9 +650,9 @@ function fio_run_job() {
 	pbench-start-tools --group=$tool_group --dir=$benchmark_results_dir
 
 	# create a command file and keep it with the results for debugging later, or user can run outside of pbench
-	echo "$benchmark_bin $client_opts $bench_opts" >$benchmark_results_dir/fio.cmd
+	echo "$benchmark $client_opts $bench_opts" >$benchmark_results_dir/fio.cmd
 	chmod +x $benchmark_results_dir/fio.cmd
-	debug_log "$benchmark: Going to run [$benchmark_bin $bench_opts $client_opts]"
+	debug_log "$benchmark: Going to run [$benchmark $bench_opts $client_opts]"
 	pushd $benchmark_results_dir >/dev/null
 	$benchmark_results_dir/fio.cmd >$benchmark_results_dir/fio-result.txt
 	local _fio_exit_code=$?
@@ -896,6 +893,7 @@ function fio_run_benchmark() {
 }
 
 fio_process_options "$@"
+
 if [ "$postprocess_only" = "n" ]; then
 	ver="$(pbench-config version ${benchmark})"
 	if [[ -z "${ver}" ]]; then

agent/bench-scripts/pbench-uperf

@@ -25,13 +25,6 @@ pbench_bin="`cd ${script_path}/..; /bin/pwd`"
 . "$pbench_bin"/base
 
 export benchmark="uperf"
-if [[ -z "${benchmark_bin}" ]]; then
-	benchmark_bin="$(resolve_benchmark_bin "${benchmark}")"
-	if [[ -z "${benchmark_bin}" ]]; then
-		error_log "${script_name}: ${benchmark} executable not found on PATH=${PATH}"
-		exit 1
-	fi
-fi
 
 # Every bench-script follows a similar sequence:
 # 1) process bench script arguments
@@ -204,7 +197,13 @@ function local_pre_check {
 	local _ver="${1}"
 	local _match="${2}"
 
-	local ver_installed=$(${benchmark_bin} -V | awk 'NR==1 {print $3}')
+	local _stdout
+	_stdout=$(${benchmark} -V 2>&1)
+	if [[ ${?} -ne 0 ]]; then
+		error_log "${script_name}: ${benchmark} failed reporting version: '${_stdout}'"
+		return 1
+	fi
+	local ver_installed=$(awk 'NR==1 {print $3}' <<< ${_stdout})
 	vercmp "${benchmark}" "${_match}" "${_ver}" "${ver_installed}"
 	return ${?}
 }
@@ -458,6 +457,11 @@ if [[ "${postprocess_only}" != "y" ]]; then
 		# At least one client or server is a local host, so perform the
 		# pre-check.
 		local_pre_check "${ver}" "${match}"
+		res=${?}
+		if [[ ${res} -ne 0 ]]; then
+			error_log "[${script_name}] The local pre-check failed"
+			exit ${res}
+		fi
 	fi
 
 	let err_cnt=0
@@ -484,7 +488,7 @@ if [[ "${postprocess_only}" != "y" ]]; then
 		fi
 	done
 	if [[ ${err_cnt} -gt 0 ]]; then
-		error_log "The following pre-checks on clients and servers failed:"
+		error_log "[${script_name}] The following pre-checks on clients and servers failed:"
 		error_log "    clients: ${err_clients}"
 		error_log "    servers: ${err_servers}"
 		exit 1
@@ -626,7 +630,7 @@ for protocol in ${protocols//,/ }; do
 										# vsock protocol requires socket-type to be specified
 										vsock_extra_opts=" -S vsock"
 									fi
-									benchmark_server_cmd="${benchmark_bin} -v -s -P ${server_port}${vsock_extra_opts} > ${server_log} 2>&1"
+									benchmark_server_cmd="${benchmark} -v -s -P ${server_port}${vsock_extra_opts} > ${server_log} 2>&1"
 									# adjust server command for NUMA binding
 									server_node=`echo "$server_nodes," | cut -d, -f$server_nr`
 									if [ ! -z "$server_node" ]; then
@@ -643,7 +647,7 @@ for protocol in ${protocols//,/ }; do
 									if [ "$log_response_times" == "y" ]; then
 										resp_opt=" -X $benchmark_results_dir/$uperf_identifier--response-times.txt"
 									fi
-									benchmark_client_cmd="${benchmark_bin} -v -m $xml_file -R -a -i 1 ${resp_opt}${vsock_extra_opts} -P $server_port >$result_file 2>&1"
+									benchmark_client_cmd="${benchmark} -v -m $xml_file -R -a -i 1 ${resp_opt}${vsock_extra_opts} -P $server_port >$result_file 2>&1"
 									# adjust client command for NUMA binding
 									client_node=`echo "$client_nodes," | cut -d, -f$server_nr`
 									if [ ! -z "$client_node" ]; then

agent/bench-scripts/test-bin/bm

@@ -0,0 +1 @@
+mock-cmd

agent/bench-scripts/test-bin/fio

@@ -0,0 +1,24 @@
+#!/bin/bash
+
+echo "$0 $*" >> $_testlog
+
+# Mock fio command behaviors.
+if [[ "${1%%=*}" == "--client" ]]; then 
+	client_file=${1##*=}
+	for line in $(< ${client_file}); do
+		ip_name="${line%%,*}"
+		client="${ip_name#*:}"
+		if [[ "${client%%.*}" == "hist" ]]; then 
+			# When we are mocking out a benchmark command for pbench-fio,
+			# for each client which begins with "hist" create an empty
+			# fio latency histogram log file so that the invocation of
+			# our latency visualizations can be exercised.
+			touch ./fio_clat_hist.empty.log.${client}
+		fi
+	done
+elif [[ "${1}" == "--version" ]]; then 
+	echo "fio-${_BM_FIO_VER:-3.42}"
+	exit 0
+fi
+
+exit ${_BM_FIO_STS:-0}

agent/bench-scripts/test-bin/mock-cmd

@@ -8,13 +8,6 @@ fi
 
 prog="$(basename "${0}")"
 args=( "${@}" )
-lastidx=$((${#args[@]} - 1))
-# avoid "bad array subscript" error
-if [[ "${lastidx}" -lt 0 ]] ;then
-    exit 0
-fi
-
-last=${args[${lastidx}]}
 if [[ "${prog}" == "pbench-tool-meister-start" ]]; then
     echo [pbench] > ${benchmark_run_dir}/metadata.log
 elif [[ "${prog}" == "cat" && "${args[0]}" == "/proc/cmdline" ]]; then

agent/bench-scripts/test-bin/uperf

@@ -0,0 +1,11 @@
+#!/bin/bash
+
+echo "$0 $*" >> $_testlog
+
+# Mock uperf command behaviors.
+if [[ "${1}" == "-V" ]]; then 
+	echo "Uperf Version ${_BM_UPERF_VER:-1.0.42}"
+	echo "... garbage to be ignored ..."
+fi
+
+exit ${_BM_UPERF_STS:-0}

agent/bench-scripts/tests/pbench-fio/test-03.txt

@@ -15,5 +15,5 @@
 [debug][1900-01-01T00:00:00.000000] found fio-3.42
 --- pbench.log file contents
 +++ test-execution.log file contents
-/var/tmp/pbench-test-bench/opt/pbench-agent/unittest-scripts/bm --version
+/var/tmp/pbench-test-bench/opt/pbench-agent/unittest-scripts/fio --version
 --- test-execution.log file contents