On 11/20/19 5:43 PM, Stefan Reiter wrote:
> Same as we do in assert_joinable, a cluster with unconfigured IPs will
> fail start after creation anyway.
>
> Make "check_ip" a standalone sub ("check_ip_configured") and improve
> error messages all around.
>
> Also move call to create_conf up, so if it fails we exit before SSH and
> auth-key setup.
>
> Signed-off-by: Stefan Reiter <s.rei...@proxmox.com>
> ---
> data/PVE/API2/ClusterConfig.pm | 10 ++++-----
> data/PVE/Cluster/Setup.pm | 40 ++++++++++------------------------
> data/PVE/Corosync.pm | 29 ++++++++++++++++++++++++
> 3 files changed, 45 insertions(+), 34 deletions(-)
>
> diff --git a/data/PVE/API2/ClusterConfig.pm b/data/PVE/API2/ClusterConfig.pm
> index bff0c48..0454947 100644
> --- a/data/PVE/API2/ClusterConfig.pm
> +++ b/data/PVE/API2/ClusterConfig.pm
> @@ -115,6 +115,11 @@ __PACKAGE__->register_method ({
> my $authuser = $rpcenv->get_user();
>
> my $code = sub {
> + my $nodename = PVE::INotify::nodename();
> +
> + # get the corosync basis config for the new cluster
> + my $config = PVE::Corosync::create_conf($nodename, $param);
> +
> STDOUT->autoflush();
> PVE::Cluster::Setup::setup_sshd_config(1);
> PVE::Cluster::Setup::setup_rootsshconfig();
> @@ -124,11 +129,6 @@ __PACKAGE__->register_method ({
> if !-f $authfile;
> die "no authentication key available\n" if -f !$authfile;
>
> - my $nodename = PVE::INotify::nodename();
> -
> - # get the corosync basis config for the new cluster
> - my $config = PVE::Corosync::create_conf($nodename, $param);
> -
> print "Writing corosync config to /etc/pve/corosync.conf\n";
> PVE::Corosync::atomic_write_conf($config);
>
> diff --git a/data/PVE/Cluster/Setup.pm b/data/PVE/Cluster/Setup.pm
> index c6f6c9e..77d3664 100644
> --- a/data/PVE/Cluster/Setup.pm
> +++ b/data/PVE/Cluster/Setup.pm
> @@ -566,55 +566,37 @@ sub assert_joinable {
> my ($local_addr, $links, $force) = @_;
>
> my $errors = '';
> - my $error = sub { $errors .= "* $_[0]\n"; };
> + my $error = sub { $errors .= "* $_[0]"; };
>
> if (-f $authfile) {
> - $error->("authentication key '$authfile' already exists");
> + $error->("authentication key '$authfile' already exists\n");
> }
>
> if (-f $clusterconf) {
> - $error->("cluster config '$clusterconf' already exists");
> + $error->("cluster config '$clusterconf' already exists\n");
> }
>
> my $vmlist = PVE::Cluster::get_vmlist();
> if ($vmlist && $vmlist->{ids} && scalar(keys %{$vmlist->{ids}})) {
> - $error->("this host already contains virtual guests");
> + $error->("this host already contains virtual guests\n");
> }
please avoid that manual \n, I did that with reason.. maype use chomp
on the string for your $@ case below (or in the errors sub?
>
> if (PVE::Tools::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
> - my $check_ip = sub {
> - my $ip = shift // return;
> - my $logid = shift;
> - if (!PVE::JSONSchema::pve_verify_ip($ip, 1)) {
> - my $host = $ip;
> - eval { $ip = PVE::Network::get_ip_from_hostname($host); };
> - if ($@) {
> - $error->("$logid: cannot use '$host': $@\n") ;
> - return;
> - }
> - }
> -
> - my $cidr = (Net::IP::ip_is_ipv6($ip)) ? "$ip/128" : "$ip/32";
> - my $configured_ips = PVE::Network::get_local_ip_from_cidr($cidr);
> -
> - $error->("$logid: cannot use IP '$ip', it must be configured exactly
> once on local node!\n")
> - if (scalar(@$configured_ips) != 1);
> - };
> + $error->("corosync is currently running, is this node already in a
> cluster?\n");
> + }
>
> - $check_ip->($local_addr, 'local node address');
> + eval { PVE::Corosync::check_ip_configured($local_addr, 'local node
> address'); };
> + $error->($@) if $@;
>
> if ($links) {
> foreach my $link (keys %$links) {
> - $check_ip->($links->{$link}->{address}, "link$link");
> + eval {
> PVE::Corosync::check_ip_configured($links->{$link}->{address}, "link$link");
> };
> + $error->($@) if $@;
> }
> }
>
> if ($errors) {
> - warn "detected the following error(s):\n$errors";
> + warn "Detected the following error(s):\n$errors";
> die "Check if node may join a cluster failed!\n" if !$force;
> }
> }
> diff --git a/data/PVE/Corosync.pm b/data/PVE/Corosync.pm
> index 69927ff..19c8976 100644
> --- a/data/PVE/Corosync.pm
> +++ b/data/PVE/Corosync.pm
> @@ -11,6 +11,7 @@ use Socket qw(AF_INET AF_INET6 inet_ntop);
>
> use PVE::Cluster;
> use PVE::JSONSchema;
> +use PVE::Network;
> use PVE::Tools;
> use PVE::Tools qw($IPV4RE $IPV6RE);
>
> @@ -267,6 +268,32 @@ sub atomic_write_conf {
> || die "activating corosync.conf.new failed - $!\n";
> }
>
> +# check if an IP address is configured exactly once on the local node, so
> that
> +# corosync can safely use it as a link
> +sub check_ip_configured {
> + my ($ip, $logid) = @_;
> +
> + return if !defined($ip);
> + $logid //= "error";
> +
> + if (!PVE::JSONSchema::pve_verify_ip($ip, 1)) {
> + my $host = $ip;
> + eval { $ip = PVE::Network::get_ip_from_hostname($host); };
> + if ($@) {
> + die "$logid: cannot use '$host': $@\n";
> + }
> + }
> +
> + my $cidr = (Net::IP::ip_is_ipv6($ip)) ? "$ip/128" : "$ip/32";
> + my $configured_ips = PVE::Network::get_local_ip_from_cidr($cidr);
> +
> + die "$logid: cannot use IP '$ip', it is not configured on local node
> (must exist exactly once)!\n"
> + if (scalar(@$configured_ips) < 1);
> +
> + die "$logid: cannot use IP '$ip', it is configured multiple times on
> local node (must exist exactly once)!\n"
> + if (scalar(@$configured_ips) > 1);
> +};
> +
> # for creating a new cluster with the current node
> # params are those from the API/CLI cluster create call
> sub create_conf {
> @@ -317,6 +344,8 @@ sub create_conf {
> foreach my $lnum (keys %$links) {
> my $link = $links->{$lnum};
>
> + check_ip_configured($link->{address}, "link$lnum");
> +
> $totem->{interface}->{$lnum} = { linknumber => $lnum };
>
> my $prio = $link->{priority};
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
