Ovirt devel - Dec 2008 - [PATCH node] Usability fixes to the logging configuration.

Users can now abort the configuration. At any point the user can enter
either "abort" or an "a" depending on the input and quit the
logging
configuration.

Also smoothes out the overall flow of configuration.

Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
---
 scripts/ovirt-config-logging |  144 ++++++++++++++++++++++++++----------------
 1 files changed, 90 insertions(+), 54 deletions(-)

diff --git a/scripts/ovirt-config-logging b/scripts/ovirt-config-logging
index f79597f..8e6bc70 100755
--- a/scripts/ovirt-config-logging
+++ b/scripts/ovirt-config-logging
@@ -71,62 +71,98 @@ function is_numeric {
 }
 
 function prompt_user {
+    max_log_size="10"
+    syslog_server_ip=""
+    syslog_server_port=""
+
     while true ; do
-        max_log_size="10k"
-        printf "\nWhat is the max size for log files on this machine
[10k]? "
-        read -e
-        if [ -n "$REPLY" ]; then
-            max_log_size=$REPLY
-        fi
-        printf "\nWhat is the IP address or server name for the syslog
server? "
-        read -e
-        syslog_server_ip=$REPLY
-        printf "\nWhat port does the syslog daemon run on? "
-        read -e
-        if is_numeric "$REPLY"; then
-            syslog_server_port=$REPLY
-
-            PROTOCOLS="udp tcp"
-            PS3="Please select the protocol to use: "
-            select syslog_server_protocol in $PROTOCOLS;
-            do
-                case $syslog_server_protocol in
-                    "tcp")
-                        break ;;
-                    "udp")
-                        break;;
-                esac
-            done
-
-            printf "\n"
-            printf "\nLogging will be configured as follows:"
-            printf "\n======================================"
-            printf "\n Max Logfile Size: $max_log_size"
-            printf "\n    Remote Server: $syslog_server_ip"
-            printf "\n      Remote Port: $syslog_server_port"
-            printf "\n Logging Protocol: $syslog_server_protocol"
-            printf "\n"
-            printf "\nPlease confirm these changes (Y/N)"
-            read -e
-            case $REPLY in
-                Y|y)
-                    ovirt_rsyslog $syslog_server_ip \
-                                  $syslog_server_port \
-                                  $syslog_server_protocol
-                    sed -i -e "s/^size=.*/size=$max_log_size/" \
-                              /etc/logrotate.d/ovirt-logrotate.conf
-                    break
-                    ;;
-                N|n)
-                    printf "Discarding settings\n"
-                    break
-                    ;;
-            esac
-        else
-            printf "Invalid port number\n"
-        fi
+	while true; do
+	    printf "\n"
+	    read -p "What is the max size for log files on this machine (def.
$max_log_size)? "
+
+	    if [ -n "$REPLY" ]; then
+		r=$(echo $REPLY|tr '[[:lower:]]' '[[:upper:]]')
+		if [[ $r =~ ^[0-9]+$ ]] && [[ $r -gt 0 ]]; then
+		    max_log_size=$r
+		    printf "\nMaximum logging size will be ${max_log_size}k.\n"
+		    break
+		elif [ "ABORT" == "$r" ]; then
+		    printf "\nAborting...\n"
+		    return
+		else
+		    printf "\nInvalid input.\n"
+		fi
+	    else
+		printf "\nLeaving log size as $max_log_size.\n"
+		break
+	    fi
+	done
+
+	printf "\n"
+	read -p "What is the IP address or hostname for the syslog server? "
+	if [ -n "$REPLY" ]; then
+	    syslog_server_ip=$REPLY
+	    while true; do
+		read -p "What port does the syslog daemon use? "
+		r=$(echo $REPLY|tr '[[:lower:]]' '[[:upper:]]')
+		if [ -n "$r" ]; then
+		    if [[ $r =~ ^[0-9]+$ ]] && [[ $r -gt 0 ]]; then
+			syslog_server_port=$REPLY
+			break
+		    elif [ "ABORT" == "$r" ]; then
+			printf "\nAbort...\n"
+			return
+		    else
+			printf "Invalid port.\n"
+		    fi
+		fi
+	    done
+
+	    printf "\n"
+	    while true; do
+		read -p "Remote logging uses [t]cp or [u]dp, or [a]bort? "
+		r=$(echo $REPLY|tr '[[:lower:]]' '[[:upper:]]')
+		if [ "$r" == "T" ]; then
syslog_server_protocol="tcp"; break; fi
+		if [ "$r" == "U" ]; then
syslog_server_protocol="udp"; break; fi
+		if [ "$r" == "A" ]; then return; fi
+		# else
+		printf "Invalid input.\n"
+	    done
+	else
+	    printf "\nDisabling remote logging.\n"
+	fi
+
+	printf "\n"
+	printf "\nLogging will be configured as follows:"
+	printf "\n======================================"
+	printf "\n Max Logfile Size: $max_log_size"
+	if [ -n "$syslog_server_ip" ]; then
+	    printf "\n    Remote Server: $syslog_server_ip"
+	    printf "\n      Remote Port: $syslog_server_port"
+	    printf "\n Logging Protocol: $syslog_server_protocol"
+	fi
+	printf "\n"
+	printf "\n"
+	while true; do
+	    read -p "Is this correct (Y/N/A)? "
+	    r=$(echo $REPLY|tr '[[:lower:]]' '[[:upper:]]')
+	    if [ "$r" == "Y" ]; then
+		printf "\nSaving configuration.\n"
+		ovirt_rsyslog $syslog_server_ip \
+		    $syslog_server_port \
+		    $syslog_server_protocol
+		sed -i -e "s/^size=.*/size=${max_log_size}k/" \
+		    /etc/logrotate.d/ovirt-logrotate.conf
+		return
+	    elif [ "$r" == "N" ]; then
+		printf "\nRestarting logging configuration.\n"
+		break
+	    elif [ "$r" == "A" ]; then
+		printf "\nAborting logging configuration.\n"
+		return
+	    fi
+	done
     done
-
 }
 
 # AUTO for auto-install
-- 
1.6.0.4

Some comments below

Darryl L. Pierce wrote:
> Users can now abort the configuration. At any point the user can enter
> either "abort" or an "a" depending on the input and
quit the logging
> configuration.
> 
> Also smoothes out the overall flow of configuration.
> 
> Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
> ---
>  scripts/ovirt-config-logging |  144
++++++++++++++++++++++++++----------------
>  1 files changed, 90 insertions(+), 54 deletions(-)
> 
> diff --git a/scripts/ovirt-config-logging b/scripts/ovirt-config-logging
> index f79597f..8e6bc70 100755
> --- a/scripts/ovirt-config-logging
> +++ b/scripts/ovirt-config-logging
> @@ -71,62 +71,98 @@ function is_numeric {
>  }
>  
>  function prompt_user {
> +    max_log_size="10"
> +    syslog_server_ip=""
> +    syslog_server_port=""
> +
>      while true ; do
> -        max_log_size="10k"
> -        printf "\nWhat is the max size for log files on this machine
[10k]? "
> -        read -e
> -        if [ -n "$REPLY" ]; then
> -            max_log_size=$REPLY
> -        fi
> -        printf "\nWhat is the IP address or server name for the
syslog server? "
> -        read -e
> -        syslog_server_ip=$REPLY
> -        printf "\nWhat port does the syslog daemon run on? "
> -        read -e
> -        if is_numeric "$REPLY"; then
> -            syslog_server_port=$REPLY
> -
> -            PROTOCOLS="udp tcp"
> -            PS3="Please select the protocol to use: "
> -            select syslog_server_protocol in $PROTOCOLS;
> -            do
> -                case $syslog_server_protocol in
> -                    "tcp")
> -                        break ;;
> -                    "udp")
> -                        break;;
> -                esac
> -            done
> -
> -            printf "\n"
> -            printf "\nLogging will be configured as follows:"
> -            printf "\n======================================"
> -            printf "\n Max Logfile Size: $max_log_size"
> -            printf "\n    Remote Server: $syslog_server_ip"
> -            printf "\n      Remote Port: $syslog_server_port"
> -            printf "\n Logging Protocol:
$syslog_server_protocol"
> -            printf "\n"
> -            printf "\nPlease confirm these changes (Y/N)"
> -            read -e
> -            case $REPLY in
> -                Y|y)
> -                    ovirt_rsyslog $syslog_server_ip \
> -                                  $syslog_server_port \
> -                                  $syslog_server_protocol
> -                    sed -i -e "s/^size=.*/size=$max_log_size/" \
> -                              /etc/logrotate.d/ovirt-logrotate.conf
> -                    break
> -                    ;;
> -                N|n)
> -                    printf "Discarding settings\n"
> -                    break
> -                    ;;
> -            esac
> -        else
> -            printf "Invalid port number\n"
> -        fi
> +	while true; do
> +	    printf "\n"
> +	    read -p "What is the max size for log files on this machine
(def. $max_log_size)? "
I would let folks know that this is in kilibytes
> +
> +	    if [ -n "$REPLY" ]; then
> +		r=$(echo $REPLY|tr '[[:lower:]]' '[[:upper:]]')
> +		if [[ $r =~ ^[0-9]+$ ]] && [[ $r -gt 0 ]]; then
> +		    max_log_size=$r
> +		    printf "\nMaximum logging size will be
${max_log_size}k.\n"
> +		    break
> +		elif [ "ABORT" == "$r" ]; then
> +		    printf "\nAborting...\n"
> +		    return
this is undocumented. Will every input line allow me to ABORT?
> +		else
> +		    printf "\nInvalid input.\n"
> +		fi
> +	    else
> +		printf "\nLeaving log size as $max_log_size.\n"
> +		break
> +	    fi
> +	done
> +
> +	printf "\n"
> +	read -p "What is the IP address or hostname for the syslog server?
"
> +	if [ -n "$REPLY" ]; then
> +	    syslog_server_ip=$REPLY
> +	    while true; do
> +		read -p "What port does the syslog daemon use? "
> +		r=$(echo $REPLY|tr '[[:lower:]]' '[[:upper:]]')
> +		if [ -n "$r" ]; then
> +		    if [[ $r =~ ^[0-9]+$ ]] && [[ $r -gt 0 ]]; then
> +			syslog_server_port=$REPLY
> +			break
> +		    elif [ "ABORT" == "$r" ]; then
> +			printf "\nAbort...\n"
> +			return
> +		    else
> +			printf "Invalid port.\n"
> +		    fi
> +		fi
> +	    done
> +
> +	    printf "\n"
> +	    while true; do
> +		read -p "Remote logging uses [t]cp or [u]dp, or [a]bort? "
> +		r=$(echo $REPLY|tr '[[:lower:]]' '[[:upper:]]')
> +		if [ "$r" == "T" ]; then
syslog_server_protocol="tcp"; break; fi
> +		if [ "$r" == "U" ]; then
syslog_server_protocol="udp"; break; fi
> +		if [ "$r" == "A" ]; then return; fi
> +		# else
> +		printf "Invalid input.\n"
> +	    done
> +	else
> +	    printf "\nDisabling remote logging.\n"
> +	fi
I found it odd that if I hit return I could stilll commit the logging 
change, but if I hit the abort then I lost both chanages. I would 
suggest the following:
- Keep the abort at the end of the process.
- Allow me to "skip" over the rsyslog
- Loose the ABORT Logic

> +
> +	printf "\n"
> +	printf "\nLogging will be configured as follows:"
> +	printf "\n======================================"
> +	printf "\n Max Logfile Size: $max_log_size"
> +	if [ -n "$syslog_server_ip" ]; then
> +	    printf "\n    Remote Server: $syslog_server_ip"
> +	    printf "\n      Remote Port: $syslog_server_port"
> +	    printf "\n Logging Protocol: $syslog_server_protocol"
> +	fi
> +	printf "\n"
> +	printf "\n"
> +	while true; do
> +	    read -p "Is this correct (Y/N/A)? "
> +	    r=$(echo $REPLY|tr '[[:lower:]]' '[[:upper:]]')
> +	    if [ "$r" == "Y" ]; then
> +		printf "\nSaving configuration.\n"
> +		ovirt_rsyslog $syslog_server_ip \
> +		    $syslog_server_port \
> +		    $syslog_server_protocol
> +		sed -i -e "s/^size=.*/size=${max_log_size}k/" \
> +		    /etc/logrotate.d/ovirt-logrotate.conf
> +		return
> +	    elif [ "$r" == "N" ]; then
> +		printf "\nRestarting logging configuration.\n"
> +		break
> +	    elif [ "$r" == "A" ]; then
> +		printf "\nAborting logging configuration.\n"
> +		return
> +	    fi
> +	done
>      done
> -
>  }
>  
>  # AUTO for auto-install

NOTE: This patch includes fixes per Bryan K's feedback.

Users can now abort the configuration. At any point the user can enter
either "abort" or an "a" depending on the input and quit the
logging
configuration.

Also smoothes out the overall flow of configuration.

Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
---
 scripts/ovirt-config-logging |  138 +++++++++++++++++++++++++-----------------
 1 files changed, 83 insertions(+), 55 deletions(-)

diff --git a/scripts/ovirt-config-logging b/scripts/ovirt-config-logging
index f79597f..358a37c 100755
--- a/scripts/ovirt-config-logging
+++ b/scripts/ovirt-config-logging
@@ -5,7 +5,6 @@
 . /etc/init.d/functions
 . /etc/init.d/ovirt-functions
 
-
 RSYSLOG_FILE="/etc/rsyslog.conf"
 
 # Creates the rsyslog file based on the following inputs
@@ -72,61 +71,90 @@ function is_numeric {
 
 function prompt_user {
     while true ; do
-        max_log_size="10k"
-        printf "\nWhat is the max size for log files on this machine
[10k]? "
-        read -e
-        if [ -n "$REPLY" ]; then
-            max_log_size=$REPLY
-        fi
-        printf "\nWhat is the IP address or server name for the syslog
server? "
-        read -e
-        syslog_server_ip=$REPLY
-        printf "\nWhat port does the syslog daemon run on? "
-        read -e
-        if is_numeric "$REPLY"; then
-            syslog_server_port=$REPLY
-
-            PROTOCOLS="udp tcp"
-            PS3="Please select the protocol to use: "
-            select syslog_server_protocol in $PROTOCOLS;
-            do
-                case $syslog_server_protocol in
-                    "tcp")
-                        break ;;
-                    "udp")
-                        break;;
-                esac
-            done
-
-            printf "\n"
-            printf "\nLogging will be configured as follows:"
-            printf "\n======================================"
-            printf "\n Max Logfile Size: $max_log_size"
-            printf "\n    Remote Server: $syslog_server_ip"
-            printf "\n      Remote Port: $syslog_server_port"
-            printf "\n Logging Protocol: $syslog_server_protocol"
-            printf "\n"
-            printf "\nPlease confirm these changes (Y/N)"
-            read -e
-            case $REPLY in
-                Y|y)
-                    ovirt_rsyslog $syslog_server_ip \
-                                  $syslog_server_port \
-                                  $syslog_server_protocol
-                    sed -i -e "s/^size=.*/size=$max_log_size/" \
-                              /etc/logrotate.d/ovirt-logrotate.conf
-                    break
-                    ;;
-                N|n)
-                    printf "Discarding settings\n"
-                    break
-                    ;;
-            esac
-        else
-            printf "Invalid port number\n"
-        fi
+	max_log_size="10"
+	syslog_server_ip=""
+	syslog_server_port=""
+
+	while true; do
+	    printf "\n"
+	    read -p "What is the max size, in kilobytes, for local log files
(def. ${max_log_size}k)? "
+
+	    if [ -n "$REPLY" ]; then
+		r=$REPLY
+		if [[ $r =~ ^[0-9]+$ ]] && [[ $r -gt 0 ]]; then
+		    max_log_size=$r
+		    printf "\nMaximum logging size will be ${max_log_size}k.\n"
+		    break
+		else
+		    printf "\nInvalid input.\n"
+		fi
+	    else
+		printf "\nLeaving log size as ${max_log_size}k.\n"
+		break
+	    fi
+	done
+
+	printf "\n"
+	read -p "What is the IP address for the syslog server, hit Enter to skip?
"
+	if [ -n "$REPLY" ]; then
+	    syslog_server_ip=$REPLY
+	    while true; do
+		read -p "Please enter the remote logging port used:  "
+		r=$REPLY
+		if [ -n "$r" ]; then
+		    if [[ $r =~ ^[0-9]+$ ]] && [[ $r -gt 0 ]]; then
+			syslog_server_port=$REPLY
+			break
+		    else
+			printf "Invalid port.\n"
+		    fi
+		fi
+	    done
+
+	    printf "\n"
+	    while true; do
+		read -p "Remote logging uses [t]cp or [u]dp? "
+		r=$(echo $REPLY|tr '[[:lower:]]' '[[:upper:]]')
+		if [ "$r" == "T" ]; then
syslog_server_protocol="tcp"; break; fi
+		if [ "$r" == "U" ]; then
syslog_server_protocol="udp"; break; fi
+		# else
+		printf "Invalid input.\n"
+	    done
+	else
+	    printf "\nDisabling remote logging.\n"
+	fi
+
+	printf "\n"
+	printf "\nLogging will be configured as follows:"
+	printf "\n======================================"
+	printf "\n Max Logfile Size: $max_log_size"
+	if [ -n "$syslog_server_ip" ]; then
+	    printf "\n    Remote Server: $syslog_server_ip"
+	    printf "\n      Remote Port: $syslog_server_port"
+	    printf "\n Logging Protocol: $syslog_server_protocol"
+	fi
+	printf "\n"
+	printf "\n"
+	while true; do
+	    read -p "Is this correct (Y/N/A)? "
+	    r=$(echo $REPLY|tr '[[:lower:]]' '[[:upper:]]')
+	    if [ "$r" == "Y" ]; then
+		printf "\nSaving configuration.\n"
+		ovirt_rsyslog $syslog_server_ip \
+		    $syslog_server_port \
+		    $syslog_server_protocol
+		sed -i -e "s/^size=.*/size=${max_log_size}k/" \
+		    /etc/logrotate.d/ovirt-logrotate.conf
+		return
+	    elif [ "$r" == "N" ]; then
+		printf "\nRestarting logging configuration.\n"
+		break
+	    elif [ "$r" == "A" ]; then
+		printf "\nAborting logging configuration.\n"
+		return
+	    fi
+	done
     done
-
 }
 
 # AUTO for auto-install
-- 
1.6.0.4

CHANGED CALL TO ovirt_rsyslog to require the ip, port and protocol to have
been set first.

Users can now abort the configuration. At any point the user can enter
either "abort" or an "a" depending on the input and quit the
logging
configuration.

Also smoothes out the overall flow of configuration.

Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
---
 scripts/ovirt-config-logging |  142 ++++++++++++++++++++++++++----------------
 1 files changed, 87 insertions(+), 55 deletions(-)

diff --git a/scripts/ovirt-config-logging b/scripts/ovirt-config-logging
index f79597f..f1daf29 100755
--- a/scripts/ovirt-config-logging
+++ b/scripts/ovirt-config-logging
@@ -5,7 +5,6 @@
 . /etc/init.d/functions
 . /etc/init.d/ovirt-functions
 
-
 RSYSLOG_FILE="/etc/rsyslog.conf"
 
 # Creates the rsyslog file based on the following inputs
@@ -72,61 +71,94 @@ function is_numeric {
 
 function prompt_user {
     while true ; do
-        max_log_size="10k"
-        printf "\nWhat is the max size for log files on this machine
[10k]? "
-        read -e
-        if [ -n "$REPLY" ]; then
-            max_log_size=$REPLY
-        fi
-        printf "\nWhat is the IP address or server name for the syslog
server? "
-        read -e
-        syslog_server_ip=$REPLY
-        printf "\nWhat port does the syslog daemon run on? "
-        read -e
-        if is_numeric "$REPLY"; then
-            syslog_server_port=$REPLY
-
-            PROTOCOLS="udp tcp"
-            PS3="Please select the protocol to use: "
-            select syslog_server_protocol in $PROTOCOLS;
-            do
-                case $syslog_server_protocol in
-                    "tcp")
-                        break ;;
-                    "udp")
-                        break;;
-                esac
-            done
-
-            printf "\n"
-            printf "\nLogging will be configured as follows:"
-            printf "\n======================================"
-            printf "\n Max Logfile Size: $max_log_size"
-            printf "\n    Remote Server: $syslog_server_ip"
-            printf "\n      Remote Port: $syslog_server_port"
-            printf "\n Logging Protocol: $syslog_server_protocol"
-            printf "\n"
-            printf "\nPlease confirm these changes (Y/N)"
-            read -e
-            case $REPLY in
-                Y|y)
-                    ovirt_rsyslog $syslog_server_ip \
-                                  $syslog_server_port \
-                                  $syslog_server_protocol
-                    sed -i -e "s/^size=.*/size=$max_log_size/" \
-                              /etc/logrotate.d/ovirt-logrotate.conf
-                    break
-                    ;;
-                N|n)
-                    printf "Discarding settings\n"
-                    break
-                    ;;
-            esac
-        else
-            printf "Invalid port number\n"
-        fi
+	max_log_size="10"
+	syslog_server_ip=""
+	syslog_server_port=""
+
+	while true; do
+	    printf "\n"
+	    read -p "What is the max size, in kilobytes, for local log files
(def. ${max_log_size}k)? "
+
+	    if [ -n "$REPLY" ]; then
+		r=$REPLY
+		if [[ $r =~ ^[0-9]+$ ]] && [[ $r -gt 0 ]]; then
+		    max_log_size=$r
+		    printf "\nMaximum logging size will be ${max_log_size}k.\n"
+		    break
+		else
+		    printf "\nInvalid input.\n"
+		fi
+	    else
+		printf "\nLeaving log size as ${max_log_size}k.\n"
+		break
+	    fi
+	done
+
+	printf "\n"
+	read -p "What is the IP address for the syslog server, hit Enter to skip?
"
+	if [ -n "$REPLY" ]; then
+	    syslog_server_ip=$REPLY
+	    while true; do
+		read -p "Please enter the remote logging port used:  "
+		r=$REPLY
+		if [ -n "$r" ]; then
+		    if [[ $r =~ ^[0-9]+$ ]] && [[ $r -gt 0 ]]; then
+			syslog_server_port=$REPLY
+			break
+		    else
+			printf "Invalid port.\n"
+		    fi
+		fi
+	    done
+
+	    printf "\n"
+	    while true; do
+		read -p "Remote logging uses [t]cp or [u]dp? "
+		r=$(echo $REPLY|tr '[[:lower:]]' '[[:upper:]]')
+		if [ "$r" == "T" ]; then
syslog_server_protocol="tcp"; break; fi
+		if [ "$r" == "U" ]; then
syslog_server_protocol="udp"; break; fi
+		# else
+		printf "Invalid input.\n"
+	    done
+	else
+	    printf "\nDisabling remote logging.\n"
+	fi
+
+	printf "\n"
+	printf "\nLogging will be configured as follows:"
+	printf "\n======================================"
+	printf "\n Max Logfile Size: $max_log_size"
+	if [ -n "$syslog_server_ip" ]; then
+	    printf "\n    Remote Server: $syslog_server_ip"
+	    printf "\n      Remote Port: $syslog_server_port"
+	    printf "\n Logging Protocol: $syslog_server_protocol"
+	fi
+	printf "\n"
+	printf "\n"
+	while true; do
+	    read -p "Is this correct (Y/N/A)? "
+	    r=$(echo $REPLY|tr '[[:lower:]]' '[[:upper:]]')
+	    if [ "$r" == "Y" ]; then
+		printf "\nSaving configuration.\n"
+		if [[ -n "$syslog_server_ip" ]] &&
+		    [[ -n "$syslog_server_port" ]] &&
+		    [[ -n "$syslog_server_protocol" ]]; then
+		    ovirt_rsyslog $syslog_server_ip \
+			$syslog_server_port \
+			$syslog_server_protocol
+		fi
+		sed -i -e "s/^size=.*/size=${max_log_size}k/" \
+		    /etc/logrotate.d/ovirt-logrotate.conf
+		return
+	    elif [ "$r" == "N" ]; then
+		printf "\nRestarting logging configuration.\n"
+		break
+	    elif [ "$r" == "A" ]; then
+		printf "\nAborting logging configuration.\n"
+		return
+	    fi
+	done
     done
-
 }
 
 # AUTO for auto-install
-- 
1.6.0.4