[Freeipa-devel] [PATCH 0024] ipa-replica-manage: added --suffix option for certain commands
Stanislav Laznicka
slaznick at redhat.com
Wed Mar 16 13:45:31 UTC 2016
On 03/16/2016 10:22 AM, Jan Cholasta wrote:
> On 16.3.2016 08:33, Stanislav Laznicka wrote:
>> On 03/15/2016 12:47 PM, Petr Vobornik wrote:
>>> On 03/15/2016 07:25 AM, Jan Cholasta wrote:
>>>> 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 - -
>>>
>>> isnt' it?:
>>> - i-r-m -
>>>
>> It is AFAIK.
>
> It's not, the command searches all 'cleanallruv' and 'abort
> cleanallruv' tasks without filtering by suffix.
My bad, misread it and thought it was list-ruv.
>
>>>
>>>>
>>>> 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.)
>>>
>>> Yes, backwards compatibility(bc) makes a mess there. But look at the
>>> state in following way (bc hidden):
>>>
>>> suffix both domain ca
>>>
>>> == Normal operations (i.e. all in API) ==
>>>
>>> list s-f s-f -ts=d s-f -ts=c
>>>
>>>
>>>
>>> connect - ts-a d ts-a c
>>>
>>> diconnect - ts-d d ts-d c
>>>
>>> del s-d - -
>>>
>>> == Debugging & Fixing ==
>>>
>>> re-initialize ts-r d ts-r c
>>> - i-r-m i-c-m
>>>
>>> force-sync - i-r-m i-c-m
>>>
>>>
>>> list-ruv i-r-m
>>>
>>> clean-ruv i-r-m
>>>
>>> abort-clean-ruv i-r-m
>>>
>>> list-clean-ruv i-r-m - -
>>>
>>> clean-dangling-ruv i-r-m - -
>>>
>>>
>>> Then we can see that only issue is force-sync operations which use
>>> case I don't really understand and with re-initialize which should be
>>> improved in API to be more usable (currently there is no progress
>>> status).
>>>
>>> Note: "debugging and fixing" is basically the same on both domain
>>> levels.
>>>
>>>
>>>>
>>>>>
>>>>> 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.
>>>
>>> I meant that we don't need to invest into new features in domain level
>>> 0 but RUV commands doesn't need to behave differently on various
>>> domain levels. There is no reason.
>>>
>>>>
>>>>>
>>>>> Note: clean-ruv subcommand could be probably marked as deprecated
>>>>> or be
>>>>> discouraged to use.
>>>>
>>>> If the commands are deprecated, why further extend them?
>>>
>>> No reason, clean-ruv subcommand doesn't need to be extended. Maybe to
>>> have similar behavior as rest of ruv commands.
>>>
>> It was exactly for that reason. If there's abort-clean-ruv which allows
>> aborting the clean operation for both suffixes, it seems rather natural
>> to have its counterpart to be able to do the same (as long as it's not
>> deprecated, which we might do right now if it seems like a good thing to
>> do).
>>>>
>>>>>
>>>>> 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.
>>>
>>> Why?
>>>
>>>
>> Given the question of deprecating clean-ruv is answered by now, I should
>> also ask why.
>
> We discussed this with Petr offline. We agreed that it's actually
> desirable to make all clean-ruv commands behave the same on all domain
> levels. We also agreed that it's desirable to make the normal
> operation commands behave the same on all domain levels, which is
> currently not true for the connect and disconnect commands, but that's
> unrelated to this patch.
>
> Therefore, I'm OK with the approach, as long as you either remove the
> --suffix option altogether, or add it to the remaining clean-ruv
> commands (list-clean-ruv and clean-dangling-ruv). I would personally
> just remove it, because as Petr pointed out, it's not actually
> necessary for anything.
>
Modified the patch (removed the --suffix option) and added password
check for clean_dangling_ruv command to be in the same spot as for the
other commands.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-stlaz-0024-1-abort-clean-list-clean-ruv-now-work-for-both-suffixe.patch
Type: text/x-patch
Size: 11306 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160316/95fa32e2/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-stlaz-0025-Moved-password-check-from-clean_dangling_ruv.patch
Type: text/x-patch
Size: 2478 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160316/95fa32e2/attachment-0001.bin>
More information about the Freeipa-devel
mailing list