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.



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.

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to