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

[Thomas Lamprecht]

On 11/11/2016 08:25 AM, Fabian Grünbichler wrote:
> On Fri, Nov 11, 2016 at 08:18:08AM +0100, Thomas Lamprecht wrote:
>> On 11/11/2016 06:39 AM, Dietmar Maurer wrote:
>>>> 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.
>>> I don't really understand why that makes sense? When (and how often) does
>>> a user run into that problem?
>> rebuilding a cluster (after upgrading to new major PVE version) for example,
>> there he has to use the --force param, if something else is broken (i.e.
>> corosync still runs) this can be problematic, because he gets not even
>> notified by the failed check.
>>
>> We currently say to the users the should use `pvecm add --force`  on cluster
>> rebuild, now we could say use `pvecm add --allow_vmids` and we could ensure
>> that other problems trigger an error.
>>
>> Also from forum post I saw that the -force option is an 'one cure for all'
>> solution, if adding not works a lot just add the force param, now the have
>> more fine grained control.
>>
>> I guess it's not seldom the case that a user has to single PVE nodes with
>> VMs already running, now -force is an overkill, if the user checked that he
>> has no VMID conflicts he can safely use 'allow_vmids' and has not to worry
>> that he missed something and corsync ran already on both.
>>
>> I had to deal with a few broken clusters in support and there I often wished
>> this, I mean I think through twice what I do but still I wished that I can
>> do cluster adds safer in such situations.
>>
>> maybe it irked just me, so other opinions would be welcomed.
>> Also we can let the force option if wished, which overwrites all, but at
>> least gives warnings of the checks it overwrote.
> alternative proposal: add a new parameter with a flag-list that allows
> "partial forcing" with warnings for stuff that is actually forced, and
> "-force" simply sets all of the "force flags"?

sounds good to me, I do not care if its four separate boolean flag 
parameters
or one parameter which accepts those 4 boolean flags.
And the behavior you describe is already the one I implemented (minus 
keeping the force flag)

Although the bash CLI completion works better with single boolean 
parameters, IIRC.
- I know you use ZSH ;)

>
> that way only one new parameter is introduced, the fine-grained approach
> is possible, the old -force is not broken (but actually improved) and
> the code gets more explicit as well.

I have to note broken is here not that bad as in other cases, as its a 
manual
triggered command and should not be used often (at least in its current 
form).
So I saw that as an active chance to break and remove it, for good, imo :)