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?= <[email protected]>
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 <[email protected]>
---
 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


_______________________________________________
pve-devel mailing list
[email protected]
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to