Fix 4190: vrrp may fail to start daemon if previous commit of vrrp partially failed

1 files changed, 62 insertions, 37 deletions

diff --git a/scripts/keepalived/vyatta-keepalived.pl b/scripts/keepalived/vyatta-keepalived.pl
index 7d5e6ed7..f7d3a652 100755
--- a/scripts/keepalived/vyatta-keepalived.pl
+++ b/scripts/keepalived/vyatta-keepalived.pl
@@ -13,7 +13,7 @@
 # General Public License for more details.
 #
 # This code was originally developed by Vyatta, Inc.
-# Portions created by Vyatta are Copyright (C) 2007 Vyatta, Inc.
+# Portions created by Vyatta are Copyright (C) 2007-2009 Vyatta, Inc.
 # All Rights Reserved.
 # 
 # Author: Stig Thormodsrud
@@ -34,6 +34,7 @@ use Getopt::Long;
 use strict;
 use warnings;
 
+my ($action, $vrrp_intf, $vrrp_group, $vrrp_vip);
 my ($conf_file, $changes_file);
 my %HoA_sync_groups;
 
@@ -50,9 +51,9 @@ sub validate_source_addr {
 	    }
 	}
 	if (!defined $config_ipaddrs{$source_addr}) {
-	    print "hello-source-address [$source_addr] must be configured on" .
-		" some interface\n";
-	    exit 1;
+	    vrrp_log("no hello-source");
+	    return "hello-source-address [$source_addr] must be " . 
+		"configured on the interface\n";
 	}
 	return;
     }
@@ -61,9 +62,9 @@ sub validate_source_addr {
     my $intf = new Vyatta::Interface($ifname);
     @ipaddrs = $intf->address(4);
     if (scalar(@ipaddrs) < 1) {
-	print "must configure either a primary address on [$ifname] or" .
+	vrrp_log("no primary or hello-source");
+	return "must configure either a primary address on [$ifname] or" .
 	    " a hello-source-address\n";
-	exit 1;
     }
     return;
 }
@@ -71,11 +72,13 @@ sub validate_source_addr {
 sub keepalived_get_values {
     my ($intf, $path) = @_;
 
+    my @errs   = ();
     my $output = '';
     my $config = new Vyatta::Config;
 
     my $state_transition_script = get_state_script();
     
+    vrrp_log("keepalived_get_values [$intf][$path]");
     $config->setLevel("$path vrrp vrrp-group");
     my @groups = $config->listNodes();
     foreach my $group (@groups) {
@@ -90,12 +93,12 @@ sub keepalived_get_values {
 	my @vips = $config->returnValues("virtual-address");
 	my $num_vips = scalar(@vips);
 	if ($num_vips == 0) {
-	    print "must define a virtual-address for vrrp-group $group\n";
-	    exit 1;
+	    push @errs, "must define a virtual-address for vrrp-group $group\n";
+	    next;
 	}
 	if ($num_vips > 20) {
-	    print "can not set more than 20 VIPs per group\n";
-	    exit 1;
+	    push @errs, "can not set more than 20 VIPs per group\n";
+	    next
 	}
 	my $priority = $config->returnValue("priority");
 	if (!defined $priority) {
@@ -118,7 +121,11 @@ sub keepalived_get_values {
 	    push @{ $HoA_sync_groups{$sync_group} }, $vrrp_instance;
 	}
 	my $hello_source_addr = $config->returnValue("hello-source-address");
-	validate_source_addr($intf, $hello_source_addr);
+	my $err = validate_source_addr($intf, $hello_source_addr);
+	if (defined $err) {
+	    push @errs, $err;
+	    next;
+	}
 
 	$config->setLevel("$path vrrp vrrp-group $group authentication");
 	my $auth_type = $config->returnValue("type");
@@ -128,8 +135,8 @@ sub keepalived_get_values {
 	    $auth_type = uc($auth_type);
 	    $auth_pass = $config->returnValue("password");
 	    if (! defined $auth_pass) {
-		print "vrrp authentication password not set";
-		exit 1;
+		push @errs, "vrrp authentication password not set\n";
+		next;
 	    }
 	}
 
@@ -147,10 +154,12 @@ sub keepalived_get_values {
            $run_master_script = "null";
         }
 
+	# We now have the values and have validated them, so 
+	# generate the config.
+
 	$output  .= "vrrp_instance $vrrp_instance \{\n";
 	my $init_state;
-	$init_state = vrrp_get_init_state($intf, $group, 
-							    $vips[0], $preempt);
+	$init_state = vrrp_get_init_state($intf, $group, $vips[0], $preempt);
 	$output .= "\tstate $init_state\n";
 	$output .= "\tinterface $intf\n";
 	$output .= "\tvirtual_router_id $group\n";
@@ -184,7 +193,7 @@ sub keepalived_get_values {
 	$output .= "\}\n\n";
     }
 
-    return $output;
+    return ($output, @errs);
 }
 
 sub vrrp_get_sync_groups {
@@ -198,7 +207,6 @@ sub vrrp_get_sync_groups {
 	}
 	$output .= "\t\}\n\}\n";
     }
-    
     return $output;
 }
 
@@ -301,6 +309,7 @@ sub remove_from_changes {
 	#
 	# we shouldn't get to this point, but try to handle it if we do
 	#
+	vrrp_log("unexpected remove_from_changes()");
 	system("rm -f $changes_file");
 	return 0;
     }
@@ -325,7 +334,8 @@ sub remove_from_changes {
 sub vrrp_update_config {
     my ($intf) = @_;
 
-    my $date = localtime();
+    my @errs   = ();
+    my $date   = localtime();
     my $output = "#\n# autogenerated by $0 on $date\n#\n\n";
 
     my $config = new Vyatta::Config;
@@ -337,29 +347,40 @@ sub vrrp_update_config {
 	my $path = "interfaces ethernet $eth";
 	$config->setLevel($path);
 	if ($config->exists("vrrp")) {
-	    $output .= keepalived_get_values($eth, $path);
-	    $vrrp_instances++;
+	    my ($inst_output, @inst_errs) = keepalived_get_values($eth, $path);
+	    if (scalar(@inst_errs)) {
+		push @errs, @inst_errs;
+	    } else {
+		$output .= $inst_output;
+		$vrrp_instances++;
+	    }
 	}
 	if ($config->exists("vif")) {
 	    my $path = "interfaces ethernet $eth vif";
 	    $config->setLevel($path);
 	    my @vifs = $config->listNodes();
 	    foreach my $vif (@vifs) {
-		#
-		# keepalived gets real grumpy with interfaces that don't 
-		# exist, so skip vlans that haven't been instantiated 
-		# yet (typically occurs at boot up).
-		#
-		my $vif_intf = $eth . "." . $vif;
-		if (!(-d "/sys/class/net/$vif_intf")) {
-		    vrrp_log("skipping $vif_intf");
-		    next;
-		}
 		my $vif_path = "$path $vif";
 		$config->setLevel($vif_path);
 		if ($config->exists("vrrp")) {
-		    $output .= keepalived_get_values($vif_intf, $vif_path);
-		    $vrrp_instances++;
+		    #
+		    # keepalived gets real grumpy with interfaces that don't 
+		    # exist, so skip vlans that haven't been instantiated 
+		    # yet (typically occurs at boot up).
+		    #
+		    my $vif_intf = $eth . "." . $vif;
+		    if (!(-d "/sys/class/net/$vif_intf")) {
+			push @errs, "vlan doesn't exist $vif_intf";
+			next;
+		    }
+		    my ($inst_output, @inst_errs) = 
+			keepalived_get_values($vif_intf, $vif_path);
+		    if (scalar(@inst_errs)) {
+			push @errs, @inst_errs;
+		    } else {
+			$output .= $inst_output;
+			$vrrp_instances++;
+		    }
 		}
 	    }
 	}
@@ -372,7 +393,7 @@ sub vrrp_update_config {
 	}
 	keepalived_write_file($conf_file, $output);
     } 
-    return $vrrp_instances;
+    return ($vrrp_instances, @errs);
 }
 
 sub keepalived_write_file {
@@ -426,8 +447,6 @@ sub list_vrrp_group {
 #
 # main
 #
-my ($action, $vrrp_intf, $vrrp_group, $vrrp_vip);
-
 GetOptions("vrrp-action=s" => \$action,
 	   "intf=s"        => \$vrrp_intf,
 	   "group=s"       => \$vrrp_group,
@@ -440,7 +459,7 @@ if (! defined $action) {
 
 if ($action eq "update") {
     $changes_file = get_changes_file();
-    $conf_file = get_conf_file();
+    $conf_file    = get_conf_file();
     vrrp_log("vrrp update $vrrp_intf");
     if ( ! -e $changes_file) {
 	my $num_changes = vrrp_find_changes();
@@ -451,7 +470,7 @@ if ($action eq "update") {
 	    vrrp_log("unexpected 0 changes");	    
 	}
     }
-    my $vrrp_instances = vrrp_update_config($vrrp_intf);
+    my ($vrrp_instances, @errs) = vrrp_update_config($vrrp_intf);
     my $more_changes = remove_from_changes($vrrp_intf);
     vrrp_log(" instances $vrrp_instances, $more_changes");
     if ($vrrp_instances > 0 and $more_changes == 0) {
@@ -461,6 +480,12 @@ if ($action eq "update") {
 	stop_daemon();
 	system("rm -f $conf_file");
     }
+    if (scalar(@errs)) {
+	print join("\n", @errs);
+	vrrp_log(join("\n", @errs));
+	exit 1
+    }
+    exit 0;
 }
 
 if ($action eq "delete") {