On 1/18/18 2:52 PM, Fabian Grünbichler wrote: > 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?
I agree, that all sounds like a good maintainability improvement. I'll try to assemble v5 with this and 13/15 problems addressed at the beginning of next week, maybe some other problems/improvements pop up until then. I wanted to suggest to apply the first 5 patches already, as they are either code movements or non-invasive changes but I saw just now that the second hunk of 4/15 needs to get squashed into 3/15, so maybe its better to wait for v5 until we start to apply (parts of) this series. Thanks for the comments thus far! _______________________________________________ pve-devel mailing list [email protected] https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
