Ovirt devel - Sep 2008 - [PATCH node] Check to make sure a config file was applied, if not revert to default config

ovirt-early tries to use the managed_node controller on the server
to get the remote config.  If there is none available a file is returned
but there is presently no way to determine if that file is 'empty' or
not

This check looks to see if any ifup-eth* files were created.  If none were, then
we assume the config was blank and use the default bridge config of one bridge
per physical interface.

Signed-off-by: Perry Myers <pmyers at redhat.com>
---
 scripts/ovirt-early |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/ovirt-early b/scripts/ovirt-early
index 5843fed..95e8bf7 100755
--- a/scripts/ovirt-early
+++ b/scripts/ovirt-early
@@ -49,7 +49,7 @@ configure_from_network() {
                         if [ -f /var/tmp/node-augtool ]; then
                             augtool < /var/tmp/node-augtool
                         fi
-                        if [ $? -eq 0 ]; then
+                        if [[ $? -eq 0 && -f
/etc/sysconfig/network-scripts/ifcfg-eth* ]]; then
                             printf "remote config applied."
                             return
                         fi
-- 
1.5.5.1

Perry Myers <pmyers at redhat.com> wrote:
> ovirt-early tries to use the managed_node controller on the server
> to get the remote config.  If there is none available a file is returned
> but there is presently no way to determine if that file is 'empty'
or not
>
> This check looks to see if any ifup-eth* files were created.  If none were,
then
> we assume the config was blank and use the default bridge config of one
bridge
> per physical interface.
>
> Signed-off-by: Perry Myers <pmyers at redhat.com>
> ---
>  scripts/ovirt-early |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/ovirt-early b/scripts/ovirt-early
> index 5843fed..95e8bf7 100755
> --- a/scripts/ovirt-early
> +++ b/scripts/ovirt-early
> @@ -49,7 +49,7 @@ configure_from_network() {
>                          if [ -f /var/tmp/node-augtool ]; then
>                              augtool < /var/tmp/node-augtool
>                          fi
> -                        if [ $? -eq 0 ]; then
> +                        if [[ $? -eq 0 && -f
/etc/sysconfig/network-scripts/ifcfg-eth* ]]; then
That fails with a syntax error if there are two or more matching files.

    $ touch ab1 ab2; bash -c 'test -f ab* && echo ok'
    bash: line 0: test: ab1: binary operator expected

How about this instead?  Then, we use the exit status of
ls to tell us whether there were matches:

  if test $? = 0 \
    && ls /etc/sysconfig/network-scripts/ifcfg-eth* > /dev/null
2>&1; then

There's also a problem independent of your change that I missed in
an earlier review:

That test of $? was supposed to refer to augtool's exit status.
But now, it can also refer to the exit status of the just-preceding
"if" statement; i.e., $? can be 0 even when /var/tmp/node-augtool
doesn't exist.

This is a good argument for avoiding the following paradigm altogether:

   run_command
   if test $? = 0; then      # BAD: separate line/stmt, might be separated

Instead, write is like this:

   run_command && stmt-if-successful # GOOD: harder to separate
accidentally

or if the body is longer:

   run_command && {          # GOOD
     stmts-if-successful
     ...
     }

These latter two forms make it harder to accidentally separate
the command and the test for its success.

Alternatively, save the exit status in a variable
and test *that*, so it's ok to separate them:

   run_command; st=$?
   if test $st = 0; then # GOOD: now it's ok to separate set and test stmts

ovirt-early tries to use the managed_node controller on the server
to get the remote config.  If there is none available a file is returned
but there is presently no way to determine if that file is 'empty' or
not

This check looks to see if any ifup-eth* files were created.  If none were, then
we assume the config was blank and use the default bridge config of one bridge
per physical interface.

Also did some refactoring of log output for the configure network interfaces
function so that it's easier to see what it going on for debugging purposes.

Signed-off-by: Perry Myers <pmyers at redhat.com>
---
 scripts/ovirt-early     |   34 +++++++++++++++++++++++-----------
 scripts/ovirt-functions |    1 +
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/scripts/ovirt-early b/scripts/ovirt-early
index 5843fed..3d2d0ca 100755
--- a/scripts/ovirt-early
+++ b/scripts/ovirt-early
@@ -21,38 +21,49 @@ get_mac_addresses() {
 configure_from_network() {
     DEVICE=$1
     if [ -n "$DEVICE" ]; then
-        printf .
+        echo "Configuring network Using interface $DEVICE"
         # setup temporary interface to retrieve configuration
         echo "network --device $DEVICE --bootproto dhcp" | nash
         if [ $? -eq 0 ]; then
-            printf .
             # from network-scripts/ifup-post
             IPADDR=$(LC_ALL=C ip -o -4 addr ls dev ${DEVICE} | awk '{ print
$4 ; exit }')
+            echo "Interface brought up with $IPADDR"
             eval $(ipcalc --silent --hostname ${IPADDR} ; echo
"status=$?")
             if [ "$status" = "0" ]; then
                 hostname $HOSTNAME
+                echo "Hostname resolved to $HOSTNAME"
                 # retrieve remote config
                 find_srv ovirt tcp
                 if [ -n "$SRV_HOST" -a -n "$SRV_PORT" ];
then
-                    printf .
+                    echo "oVirt Server found at: $SRV_HOST:$SRV_PORT"
                     cfgdb=$(mktemp)
                     get_mac_addresses
-                    wget -q -O $cfgdb \
+                    echo "MACs to use: $macs"
+                    wget -O $cfgdb \
                      
"http://$SRV_HOST:$SRV_PORT/ovirt/managed_node/config?host=$(hostname)&macs=$macs"
                     if [ $? -eq 0 ]; then
-                        printf .
+                        echo "Remote configuration bundle retrieved to
$cfgdb"
                         bash $cfgdb
                         if [ -f /var/tmp/pre-config-script ]; then
-                            printf "loading kernel modules"
-                            bash /var/tmp/pre-config-script
+                            echo "Loading kernel modules"
+                            bash /var/tmp/pre-config-script \
+                                && echo "Kernel modules
loaded" \
+                                || echo "Failed loading kernel
modules"
                         fi
                         if [ -f /var/tmp/node-augtool ]; then
-                            augtool < /var/tmp/node-augtool
+                            echo "Loading remote config"
+                            augtool < /var/tmp/node-augtool \
+                                && echo "Remote config
applied" \
+                                || echo "Failed applying remote
config"
                         fi
-                        if [ $? -eq 0 ]; then
-                            printf "remote config applied."
+                        if ls /etc/sysconfig/network-scripts/ifcfg-eth* >
/dev/null 2>&1; then
+                            echo "Network interfaces created from remote
config"
                             return
+                        else
+                            echo "Remote config contained no network
interfaces"
                         fi
+                    else
+                        echo "Failed to retrieve configuration
bundle"
                     fi
                 fi
             fi
@@ -63,13 +74,14 @@ configure_from_network() {
     ETHDEVS=$(cd /sys/class/net && ls -d eth*)
     for eth in $ETHDEVS; do
         BRIDGE=ovirtbr`echo $eth | cut -b4-`
+        echo "Applying default configuration to $eth and $BRIDGE"
         printf '%s\n' "DEVICE=$eth" ONBOOT=yes
"BRIDGE=$BRIDGE" \
             > /etc/sysconfig/network-scripts/ifcfg-$eth
         printf '%s\n' "DEVICE=$BRIDGE" BOOTPROTO=dhcp \
             ONBOOT=yes TYPE=Bridge PEERNTP=yes DELAY=0 \
             > /etc/sysconfig/network-scripts/ifcfg-$BRIDGE
     done
-    printf "default config applied."
+    echo "Default config applied"
 }
 
 # find_disk $bus $serial $live_disk
diff --git a/scripts/ovirt-functions b/scripts/ovirt-functions
index 792113a..68e9aa3 100644
--- a/scripts/ovirt-functions
+++ b/scripts/ovirt-functions
@@ -83,5 +83,6 @@ ovirt_store_config() {
            fi
        fi
     done
+    echo
     umount $ovirt && rmdir $ovirt
 }
-- 
1.5.5.1