[Ovirt-devel] [PATCH] generalized wait_for_service

[Jim Meyering]

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