[pve-devel] [PATCH cluster v3 05/14] assert_joinable: simplify error and warning handling

[Fabian Grünbichler]

On Tue, Dec 19, 2017 at 12:52:30PM +0100, Thomas Lamprecht wrote:
> remove the if check for force, as we handle this on another level and
> want to record errors independent of the value of force.
> ---
> 
> new in v3
> 
>  data/PVE/Cluster.pm | 47 +++++++++++++++++++----------------------------
>  1 file changed, 19 insertions(+), 28 deletions(-)
> 
> diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
> index fc173af..ff9234b 100644
> --- a/data/PVE/Cluster.pm
> +++ b/data/PVE/Cluster.pm
> @@ -1684,35 +1684,24 @@ sub ssh_info_to_command {
>  sub assert_joinable {
>      my ($ring0_addr, $ring1_addr, $force) = @_;
>  
> -    my ($errors, $warnings) = ('', '');
> -    my $error = sub {
> -	my ($msg, $suppress) = @_;
> +    my $errors = '';
> +    my $error = sub { $errors .= "* $_[0]\n"; };

why not simply use a list of error messages and iterate over it when
printing?

>  
> -	if ($suppress) {
> -	    $warnings .= "* $msg\n";
> -	} else {
> -	    $errors .= "* $msg\n";
> -	}
> -    };
> +    if (-f $authfile) {
> +	$error->("authentication key '$authfile' already exists");
> +    }
>  
> -    if (!$force) {
> +    if (-f $clusterconf)  {
> +	$error->("cluster config '$clusterconf' already exists");
> +    }
>  
> -	if (-f $authfile) {
> -	    $error->("authentication key '$authfile' already exists", $force);
> -	}
> +    my $vmlist = PVE::Cluster::get_vmlist();
> +    if ($vmlist && $vmlist->{ids} && scalar(keys %{$vmlist->{ids}})) {
> +	$error->("this host already contains virtual guests");
> +    }
>  
> -	if (-f $clusterconf)  {
> -	    $error->("cluster config '$clusterconf' already exists", $force);
> -	}
> -
> -	my $vmlist = PVE::Cluster::get_vmlist();
> -	if ($vmlist && $vmlist->{ids} && scalar(keys %{$vmlist->{ids}})) {
> -	    $error->("this host already contains virtual guests", $force);
> -	}
> -
> -	if (run_command(['corosync-quorumtool', '-l'], noerr => 1, quiet => 1) == 0) {
> -	    $error->("corosync is already running, is this node already in a cluster?!", $force);
> -	}
> +    if (run_command(['corosync-quorumtool', '-l'], noerr => 1, quiet => 1) == 0) {
> +	$error->("corosync is already running, is this node already in a cluster?!");
>      }
>  
>      # check if corosync ring IPs are configured on the current nodes interfaces
> @@ -1722,7 +1711,7 @@ sub assert_joinable {
>  	    my $host = $ip;
>  	    eval { $ip = PVE::Network::get_ip_from_hostname($host); };
>  	    if ($@) {
> -		$error->("cannot use '$host': $@\n", 1) ;
> +		$error->("cannot use '$host': $@\n") ;
>  		return;
>  	    }
>  	}
> @@ -1737,8 +1726,10 @@ sub assert_joinable {
>      $check_ip->($ring0_addr);
>      $check_ip->($ring1_addr);
>  
> -    warn "warning, ignore the following errors:\n$warnings" if $warnings;
> -    die "detected the following error(s):\n$errors" if $errors;
> +    if ($errors) {
> +	warn "detected the following error(s):\n$errors";
> +	die "Check if node may join a cluster failed!\n" if !$force;
> +    }
>  }
>  
>  my $backup_cfs_database = sub {
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel