Alan Pevec <apevec at redhat.com> wrote:> service (libvirtd in this case) was assumed to be ready immediately after
startup, which wasn't guaranteed.
> I generalized waiting loop w/ timeout we had for postgres to a shell
function, so it can be used where needed.
>
> SYNOPSIS: wait_for_service 'test command' n_retries
seconds_to_sleep_between_retries
Hi Alan,
Nice to factor that out.
> diff --git a/wui/scripts/ovirt-wui-install b/wui/scripts/ovirt-wui-install
> index 6eb7d8e..fc8f8bf 100755
> --- a/wui/scripts/ovirt-wui-install
> +++ b/wui/scripts/ovirt-wui-install
> @@ -71,6 +71,30 @@ find_ldap_base() {
> fi
> }
>
> +wait_for_service() {
> + local testcmd=$1; shift
> + declare -i timeout=$1; shift
> + declare -i seconds=$1; shift
Using a name like "sleep_seconds" would help readability.
Maybe put your SYNOPSIS comment in the code, too.
> + declare -i total
> + let total=timeout*seconds
> +
> + while [[ timeout -gt 0 ]]
> + do
> + $testcmd > /dev/null 2>&1
> + rc=$?
> + if [[ rc -eq 0 ]]
> + then
> + break
> + else
> + echo service not ready yet, retrying...
> + fi
> + let timeout--
> + sleep $seconds
> + done
> + [[ timeout -eq 0 ]] && printf 'service not ready after %d
seconds, giving up\n' $total && exit 1
> +}
[[ timeout -eq 0 ]] && {
printf 'service not ready after %d seconds, giving up\n' $total
1>&2
exit 1
}
I did a double-take at the lack of syntax: no "$" on variable
names, the uses of "let" and "declare". On one hand, I like
the reduced syntax, but I do find it slightly surprising.
If we might ever care about portability to a system without
/bin/bash, it'd be better to switch from "[[ var" to "[
$var", etc.
Using timeout=$((timeout-1)) is POSIX and nearly as readable
as the "let timeout--" bashism.
FYI, here's a portable version, checked with dash:
wait_for_service() {
local testcmd=$1; shift
local n_retries=$1; shift
local sleep_seconds=$1; shift
local total=$((n_retries*sleep_seconds))
while [ $n_retries -gt 0 ]
do
eval "$testcmd" > /dev/null 2>&1 && return 0
echo service not ready yet, retrying...
n_retries=$((n_retries-1))
sleep $sleep_seconds
done
printf 'service not ready after %d seconds, giving up\n' $total
1>&2
return 1
}
Note that while it still prints a diagnostic upon failure (now to stderr),
it no longer exits, so you'd use it like this:
wait_for_service 'virsh connect' 10 2 || exit 1