[Freeipa-devel] [PATCH 0024] ipa-replica-manage: added --suffix option for certain commands

Jan Cholasta jcholast at redhat.com
Tue Mar 15 06:25:32 UTC 2016


On 14.3.2016 17:18, Petr Vobornik wrote:
> On 03/14/2016 04:55 PM, Jan Cholasta wrote:
>> On 14.3.2016 16:26, Petr Vobornik wrote:
>>> On 03/14/2016 12:57 PM, Jan Cholasta wrote:
>>>> On 14.3.2016 12:50, Martin Basti wrote:
>>>>>
>>>>>
>>>>> On 14.03.2016 12:05, Jan Cholasta wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 11.3.2016 10:39, Stanislav Laznicka wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Please see the patch attached. Contrary to the discussion at
>>>>>>> https://fedorahosted.org/freeipa/ticket/4987 I also added the suffix
>>>>>>> option for clean_ruv command. If this command is available for
>>>>>>> normal
>>>>>>> RUVs, it should probably be available for CS-RUVs as well (or
>>>>>>> deprecated
>>>>>>> for both with advised use of clean_dangling_ruv).
>>>>>>
>>>>>> ipa-csreplica-manage is used to manage the CA suffix, so
>>>>>> ipa-csreplica-manage should be extended instead of adding --suffix
>>>>>> option to ipa-replica-manage. Having half of the CA suffix managed by
>>>>>> ipa-replica-manage and the other half by ipa-replica-manage is
>>>>>> confusing.
>>>>>>
>>>>>> Honza
>>>>>>
>>>>> There is a design document about deprecating ipa-csreplica-manage and
>>>>> move part of its responsibilities to ipa-replica-manage.
>>>>>
>>>>> http://www.freeipa.org/page/V4/Manage_replication_topology_4_4#ipa.28cs.29replica_manange_changes
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> So patch is compatible with design.
>>>>
>>>> The design is wrong then.
>>>
>>> I don't agree.
>>>
>>>>
>>>> Either do it in ipa-csreplica-manage, or make *all* ipa-replica-manage
>>>> sub-commands respect the --suffix option. Anything else is inconsistent
>>>> mess.
>>>
>>> That's the idea for domain level 1. There is little value in extending
>>> behavior(managing replication agreements) in domain level 0.
>>
>> Domain level 0 is still relevant, it won't go away anytime soon.
>>
>>>
>>> Main idea is to not care about suffixes and work with all suffixes right
>>> away. This is reflected in clean-dangling-ruv command and these
>>> extensions are its counterpart - to enable disabling the run. We mostly
>>> care about replica IDs not suffixes they belong to. IMO --suffix option
>>> is not necessary and is mostly for debugging.
>>>
>>> One of the reasons why we have all the RUV commands is a mess after
>>> uninstallation when somebody forgets/ignores to run
>>> `ipa-csreplica-manage del $server` or also `ipa-replica-manage del
>>> $server` before uninstallation of replica. Users then usually run
>>> `ipa-replica-manage del $server` --force --clean` but
>>> `ipa-csreplica-manage del $server` can't be run after it.  Changes in
>>> 4.3 and 4.4 tries to prevent this situation (e.g. by calling equivalent
>>> of `ipa-cs+replica-manage del` from `ipa-server-install  --uninstall`).
>>> But until then mess is cleaned on all servers, we should deal with it
>>> with the most convenient way - hiding implementation details.
>>>
>>
>> This is actually exposing implementation details by forcing the user to
>> use a different command based on the domain level.
>
> What different commands?

ipa-replica-manage vs ipa-csreplica-manage cs API commands.

>
>> Please explain to me how any of the above requires us to introduce
>> additional inconsistencies and bad UX to IPA.
>
> What bad UX?

This is how replicas are managed in domain level 0 without the patch:

suffix                  both          domain           ca

list                     -            i-r-m          i-c-m

list-ruv                 -            i-r-m            -

connect                  -            i-r-m          i-c-m

diconnect                -            i-r-m          i-c-m

del                      -            i-r-m          i-c-m

re-initialize            -            i-r-m          i-c-m

force-sync               -            i-r-m          i-c-m

clean-ruv                -            i-r-m            -

abort-clean-ruv          -            i-r-m            -

list-clean-ruv         i-r-m            -              -

clean-dangling-ruv     i-r-m            -              -

(i-r-m == ipa-replica-manage, etc.)


This is how replicas are managed in domain level 1 with the patch:

suffix                  both          domain           ca

list                     -            i-r-m          i-c-m
                         s-f         s-f -ts=d      s-f -ts=c

list-ruv               i-r-m        i-r-m -s=d     i-r-m -s=c

connect                  -            ts-a d         ts-a c

diconnect                -            ts-d d         ts-d c

del                    i-r-m            -              -
                         s-d             -              -

re-initialize            -            i-r-m          i-c-m
                          -            ts-r d         ts-r c

force-sync               -            i-r-m          i-c-m

clean-ruv              i-r-m        i-r-m -s=d     i-r-m -s=c

abort-clean-ruv        i-r-m        i-r-m -s=d     i-r-m -s=c

list-clean-ruv         i-r-m            -              -

clean-dangling-ruv     i-r-m            -              -

(s-f -ts=d == server-find --topologysuffixes=domain, etc.)


Maybe it's just me, but I fail to see the pattern here and find this 
very confusing. (Note that I'm not trying to blame this particular patch 
for this, I'm just frustrated from the overall state.)

>
> It is supposed to be used in following way:
>    ipa-replica-manage clean-dangling-ruvs
>
> If from whatever reason some clean ruv task is not finished then:
>    ipa-replica-manage list-clean-ruv
>      [all running task for all suffixes]
>    ipa-replica-manage abort-clean-ruv REPLICATION_ID
>
> Nothing else. Works for both domain levels and suffixes from a single
> tool. Again, --suffix option is not important.

This changes the default behavior in domain level 0. I though we are not 
extending domain level 0 anymore, you said it yourself in a comment above.

>
> Note: clean-ruv subcommand could be probably marked as deprecated or be
> discouraged to use.

If the commands are deprecated, why further extend them?

>
> If the patch doesn't implement it, then it's wrong.

The patch changes the default behavior of the sub-commands and extends 
them even in domain level 0. I would think at least that should be fixed.

-- 
Jan Cholasta




More information about the Freeipa-devel mailing list