Stop resolving benchmark binary location

This is a back-port of commit ba6a2b7 (PR #2860) from `main`.

Both `pbench-fio` and `pbench-uperf`:

 * No longer allow for the benchmark binary to be overriden by an
   environment variable -- this was an out-dated way for the unit tests
   to mock the respective benchmark behavior

 * No longer resolve and check that the benchmark binary is executable

There was an ordering problem with the old `benchmark_bin` variable
initial value and where it is used in the rest of the script (both for
`pbench-fio` and `pbench-uperf`). The existence check was not always
performed locally (no clients or servers specified which are local), but
the commands run remotely required `benchmark_bin` to be set during
creation of the commands for remote execution. By only checking for the
existence of the benchmark binary when performing the version check, and
allowing the shell to resolve the location of the benchmark binary at
run time, we avoid the interdependency altogether.

Mock commands for `fio` and `uperf` are provided on the `PATH` for
tests.  For the CLI tests, those mocks are removed so that we can verify
that help and usage text is emitted before the checks for the particular
command existence (which is shown by the failing `test-CL`).

We also add failing tests for `uperf` and `fio` behaviors:

 * `pbench-fio`
   * `test-22` -- missing `fio` command
   * `test-50` -- `fio -V` reports a bad version
 * `pbench-uperf`
   * `test-02` -- missing `uperf` command
   * `test-51` -- `uperf -V` reports a bad version

The existence check of the `fio` and `uperf` benchmark commands now
occurs after any help requests or command usage errors. This fixes
issue #2841 [1].

Finally, we correct the way `pbench-uperf` checked the status of the
`uperf` command execution of the benchmark by making sure the `local`
declaration is performed before the assigment, so that the return code
check is not overridden by the "status" of the `local` declaration.
This fixes issue #2842 [2].

[1] #2841
[2] #2842

Author: portante

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-dbench

@@ -24,7 +24,7 @@ warn_log "${script_name} is deprecated and will be removed in the next release"
 benchmark_rpm=$script_name
 export benchmark="dbench"
 if [[ -z "${benchmark_bin}" ]]; then
-	benchmark_bin="$(resolve_benchmark_bin "${benchmark}")"
+	benchmark_bin="$(command -v "${benchmark}")"
 	if [[ -z "${benchmark_bin}" ]]; then
 		error_log "[${script_name}] ${benchmark} executable not found on PATH=${PATH}"
 		exit 1

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-iozone

@@ -25,7 +25,7 @@ warn_log "${script_name} is deprecated and will be removed in the next release"
 benchmark_rpm=$script_name
 export benchmark="iozone"
 if [[ -z "${benchmark_bin}" ]]; then
-	benchmark_bin="$(resolve_benchmark_bin "${benchmark}")"
+	benchmark_bin="$(command -v "${benchmark}")"
 	if [[ -z "${benchmark_bin}" ]]; then
 		error_log "[${script_name}] ${benchmark} executable not found on PATH=${PATH}"
 		exit 1

agent/bench-scripts/pbench-netperf

@@ -39,7 +39,7 @@ warn_log "${script_name} is deprecated and will be removed in the next release"
 benchmark_rpm=$script_name
 export benchmark="netperf"
 if [[ -z "${benchmark_bin}" ]]; then
-        benchmark_bin="$(resolve_benchmark_bin "${benchmark}")"
+        benchmark_bin="$(command -v "${benchmark}")"
 	if [[ -z "${benchmark_bin}" ]]; then
 		error_log "[${script_name}] ${benchmark} executable not found on PATH=${PATH}"
 		exit 1

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/bm

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

agent/bench-scripts/test-bin/dbench

@@ -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}