[pve-devel] [PATCH cluster v4 00/15] Allow adding/deleting nodes and cluster creation over API

[Fabian Grünbichler]

On Tue, Jan 09, 2018 at 03:52:48PM +0100, Thomas Lamprecht wrote:
> Fourth iteration of this series, see [1] for the v3 cover letter.
> 
> This series depends on the apiclient exception series and the "add
> fingerprint-sha256 standard option" patch for common, which are both
> not yet applied. Further latest git of pve-cluster should be used as
> base, it deals with a restarting pve-cluster.
> 
> Higher level changes:
> 
> * Allow to get the node specific join information (IP and SSL FP) from
>   any node via a parameter - here I'm still a bit unsure, maybe
>   Fabians request to add them from all nodes would be nicer from a API
>   design standpoint of view. But, I then would like to have a short
>   cut to the information of the current (connected) node at all cost,
>   it makes front end design way easier and should be enough in 99% of
>   use cases.
> * local lock for create and join, added as new patch at end of series
> * addnode and delnode have now the node parameter in the path (thanks
>   for the suggestion Fabian).
> * own format for fingerprint
> * log to clusterlog on addition and deletion
> 
> Besides that very minor changes happened, thus no extra
> changelog-per-patch
> 
> Tested with CLI tool pvecm and through API client.


meta-nits:

it might make sense to factor out the (mostly shared) parameters of the
'join' API path and the 'add' CLI cmd, if just to prevent future changes
to be only applied to one copy.

right now, the description of the fingerprint parameter is already
different for example ;)

also, we have 4 slightly varying definitions of ring0/1_addr:
- API->create
- API->addnode
- API->join
- CLI->add

what about the something like this on top of the whole series?

man pvecm would then look like this:

       --ring0_addr <string> (default = Hostname of the node)
           Hostname (or IP) of the corosync ring0 address of this node.

       --ring1_addr <string>
           Hostname (or IP) of the corosync ring1 address of this node. Requires a valid configured
           ring 1 (bindnet1_addr) in the cluster.

--------8<--------

From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= <f.gruenbichler at proxmox.com>
Date: Thu, 18 Jan 2018 13:56:56 +0100
Subject: [PATCH cluster] factor out ring0/ring1 descs

and reword them a bit

Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
---
 data/PVE/API2/ClusterConfig.pm | 61 +++++++++++++++++-------------------------
 data/PVE/CLI/pvecm.pm          | 18 +++----------
 2 files changed, 29 insertions(+), 50 deletions(-)

diff --git a/data/PVE/API2/ClusterConfig.pm b/data/PVE/API2/ClusterConfig.pm
index 688dfcf..c48e1af 100644
--- a/data/PVE/API2/ClusterConfig.pm
+++ b/data/PVE/API2/ClusterConfig.pm
@@ -18,6 +18,23 @@ my $clusterconf = "/etc/pve/corosync.conf";
 my $authfile = "/etc/corosync/authkey";
 my $local_cluster_change_lock = "/var/lock/pvecm.lock";
 
+my $ring0_desc = {
+    type => 'string', format => 'address',
+    description => "Hostname (or IP) of the corosync ring0 address of this node.",
+    default => "Hostname of the node",
+    optional => 1,
+};
+PVE::JSONSchema::register_standard_option("corosync-ring0-addr", $ring0_desc);
+
+my $ring1_desc = {
+    type => 'string', format => 'address',
+    description => "Hostname (or IP) of the corosync ring1 address of this node.".
+	" Requires a valid configured ring 1 (bindnet1_addr) in the cluster.",
+    optional => 1,
+};
+
+PVE::JSONSchema::register_standard_option("corosync-ring1-addr", $ring1_desc);
+
 __PACKAGE__->register_method({
     name => 'index',
     path => '',
@@ -81,24 +98,14 @@ __PACKAGE__->register_method ({
 		    " executive should bind to and defaults to the local IP address of the node.",
 		optional => 1,
 	    },
-	    ring0_addr => {
-		type => 'string', format => 'address',
-		description => "Hostname (or IP) of the corosync ring0 address of this node.".
-		    " Defaults to the hostname of the node.",
-		optional => 1,
-	    },
+	    ring0_addr => get_standard_option('corosync-ring0-addr'),
 	    bindnet1_addr => {
 		type => 'string', format => 'ip',
 		description => "This specifies the network address the corosync ring 1".
 		    " executive should bind to and is optional.",
 		optional => 1,
 	    },
-	    ring1_addr => {
-		type => 'string', format => 'address',
-		description => "Hostname (or IP) of the corosync ring1 address, this".
-		    " needs an valid bindnet1_addr.",
-		optional => 1,
-	    },
+	    ring1_addr => get_standard_option('corosync-ring1-addr'),
 	},
     },
     returns => { type => 'string' },
@@ -219,18 +226,8 @@ __PACKAGE__->register_method ({
 		description => "Do not throw error if node already exists.",
 		optional => 1,
 	    },
-	    ring0_addr => {
-		type => 'string', format => 'address',
-		description => "Hostname (or IP) of the corosync ring0 address of this node.".
-		    " Defaults to nodes hostname.",
-		optional => 1,
-	    },
-	    ring1_addr => {
-		type => 'string', format => 'address',
-		description => "Hostname (or IP) of the corosync ring1 address, this".
-		    " needs an valid bindnet1_addr.",
-		optional => 1,
-	    },
+	    ring0_addr => get_standard_option('corosync-ring0-addr'),
+	    ring1_addr => get_standard_option('corosync-ring1-addr'),
 	},
     },
     returns => {
@@ -497,18 +494,10 @@ __PACKAGE__->register_method ({
 		description => "Do not throw error if node already exists.",
 		optional => 1,
 	    },
-	    ring0_addr => {
-		type => 'string', format => 'address',
-		description => "Hostname (or IP) of the corosync ring0 address of this node.".
-		    " Defaults IP resolved by nodes hostname.",
-		optional => 1,
-	    },
-	    ring1_addr => {
-		type => 'string', format => 'address',
-		description => "Hostname (or IP) of the corosync ring1 address, this".
-		    " needs an valid configured ring 1 interface in the cluster.",
-		optional => 1,
-	    },
+	    ring0_addr => get_standard_option('corosync-ring0-addr', {
+		default => 'IP this node\'s hostname gets resolved to',
+	    }),
+	    ring1_addr => get_standard_option('corosync-ring1-addr'),
 	    fingerprint => get_standard_option('fingerprint-sha256', {
 		description => "SSL certificate fingerprint. Optional in CLI environment.",
 		optional => 1,
diff --git a/data/PVE/CLI/pvecm.pm b/data/PVE/CLI/pvecm.pm
index 4620f61..59be453 100755
--- a/data/PVE/CLI/pvecm.pm
+++ b/data/PVE/CLI/pvecm.pm
@@ -8,7 +8,7 @@ use File::Basename;
 use PVE::Tools qw(run_command);
 use PVE::Cluster;
 use PVE::INotify;
-use PVE::JSONSchema;
+use PVE::JSONSchema qw(get_standard_option);
 use PVE::RPCEnvironment;
 use PVE::CLIHandler;
 use PVE::PTY;
@@ -92,19 +92,9 @@ __PACKAGE__->register_method ({
 		description => "Do not throw error if node already exists.",
 		optional => 1,
 	    },
-	    ring0_addr => {
-		type => 'string', format => 'address',
-		description => "Hostname (or IP) of the corosync ring0 address of this node.".
-		    " Defaults to nodes hostname.",
-		optional => 1,
-	    },
-	    ring1_addr => {
-		type => 'string', format => 'address',
-		description => "Hostname (or IP) of the corosync ring1 address, this".
-		    " needs an valid configured ring 1 interface in the cluster.",
-		optional => 1,
-	    },
-	    fingerprint => PVE::JSONSchema::get_standard_option('fingerprint-sha256', {
+	    ring0_addr => get_standard_option('corosync-ring0-addr'),
+	    ring1_addr => get_standard_option('corosync-ring1-addr'),
+	    fingerprint => get_standard_option('fingerprint-sha256', {
 		optional => 1,
 	    }),
 	    'use_ssh' => {
-- 
2.14.2