[pve-devel] [RFC cluster] node add: replace force with fine grained paramters

[Thomas Lamprecht]

force was an 'overwrite every possible problem' flag, which can
have bad results.
For example an user could be OK that the local machine has already
some VMIDs configured but not that corosync is running, with the
'-force' parameter he has no choice than overwrite both or none.
He even gets no warning from the checks if he uses force.

Enhance usability regarding this and remove the force parameter
entirely. Replace it with three 'allow_X' parameter which do:
* allow_cluster_config: allow that corosync config file exists.
* allow_cluster_authkey: allow that corosync authkey file exists.
* allow_running_corosync: allow that corosync already runs.
* allow_vmids: allow that virtual guest configs alreay exist

Further output also a warning if the check fails but was overwritten
with an 'allow_X'. This is also useful if a user posts the output of
a failing 'pvecm add' as the one helping him have more details to
work with.

Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
---

While giving each check his own allow param may seem a bit overkill, it's far
better than the current situation, imo.

Optionally we could keep the force flag as an 'ignore all problems' flag,
but imo it can generate more problems and the user can also use all allow_X
flags to get the same behaviour.


 data/PVE/CLI/pvecm.pm | 53 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/data/PVE/CLI/pvecm.pm b/data/PVE/CLI/pvecm.pm
index 04c7462..2100e7b 100755
--- a/data/PVE/CLI/pvecm.pm
+++ b/data/PVE/CLI/pvecm.pm
@@ -462,9 +462,24 @@ __PACKAGE__->register_method ({
 		minimum => 0,
 		optional => 1,
 	    },
-	    force => {
+	    allow_vmids => {
 		type => 'boolean',
-		description => "Do not throw error if node already exists.",
+		description => "Do not throw error if VMIDs are already configured.",
+		optional => 1,
+	    },
+	    allow_cluster_config => {
+		type => 'boolean',
+		description => "Do not throw error if cluster config already exists.",
+		optional => 1,
+	    },
+	    allow_cluster_authkey => {
+		type => 'boolean',
+		description => "Do not throw error if cluster authkey already exists.",
+		optional => 1,
+	    },
+	    allow_running_corosync => {
+		type => 'boolean',
+		description => "Do not throw error if corosync already runs.",
 		optional => 1,
 	    },
 	    ring0_addr => {
@@ -494,24 +509,28 @@ __PACKAGE__->register_method ({
 
 	my $host = $param->{hostname};
 
-	if (!$param->{force}) {
-	    
-	    if (-f $authfile) {
-		die "authentication key already exists\n";
-	    }
+	my $error = sub {
+	    my ($msg, $allow) = @_;
 
-	    if (-f $clusterconf)  {
-		die "cluster config '$clusterconf' already exists\n";
-	    }
+	    die "$msg\n" if !$allow;
+	    warn "warning, ignore that $msg\n";
+	};
 
-	    my $vmlist = PVE::Cluster::get_vmlist();
-	    if ($vmlist && $vmlist->{ids} && scalar(keys %{$vmlist->{ids}})) {
-		die "this host already contains virtual machines - please remove them first\n";
-	    }
+	if (-f $authfile) {
+	    &$error("authentication key already exists", $param->{allow_cluster_authkey});
+	}
 
-	    if (system("corosync-quorumtool >/dev/null 2>&1") == 0) {
-		die "corosync is already running\n";
-	    }
+	if (-f $clusterconf)  {
+	    &$error("cluster config '$clusterconf' already exists", $param->{allow_cluster_config});
+	}
+
+	my $vmlist = PVE::Cluster::get_vmlist();
+	if ($vmlist && $vmlist->{ids} && scalar(keys %{$vmlist->{ids}})) {
+	    &$error("this host already contains virtual guests", $param->{allow_vmids});
+	}
+
+	if (system("corosync-quorumtool >/dev/null 2>&1") == 0) {
+	    &$error("corosync is already running", $param->{allow_running_corosync});
 	}
 
 	# make sure known_hosts is on local filesystem
-- 
2.1.4