[Freeipa-devel] [PATCH] manage replication topology in the shared tree

[Ludwig Krispenz]

Hi,

I did split the patches, for easier review and to share work on it.
The attachment contains 4 patches:
- the ds plugin part as submitted for review
- the changes to the ds plugin part done after review  (not complete yet)
- the ipa framework part (including Petr's improvements)
- the install related part

Regards,
Ludwig

On 04/21/2015 01:09 PM, Petr Vobornik wrote:
> On 04/21/2015 12:53 PM, Petr Vobornik wrote:
>> On 04/13/2015 10:56 AM, Ludwig Krispenz wrote:
>>> Hi,
>>>
>>> in the attachment you find the latest state of the "topology 
>>> plugin", it
>>> implements what is defined in the design page:
>>> http://www.freeipa.org/page/V4/Manage_replication_topology (which is
>>> also waiting for a reviewer)
>>>
>>> It contains the plugin itself and  a core of ipa commands to manage a
>>> topology. to be really applicable, some work outside is required, eg 
>>> the
>>> management of the domain level and a decision where the binddn group
>>> should be maintained.
>>>
>>> Thanks,
>>> Ludwig
>>>
>>>
>>
>> I've looked at the python part, mostly because I want to start with POC
>> of Web UI for topology.
>>
>> topology.py is clearly still a work in progress. I've reflected
>> following comments into a patch to speed things up.
>>
>> What's in the patch:
>>
>> 1. git am complains about trailing whitespaces
>>
>> 2. pep8 check produces quite a lot of issues. New code should be almost
>> with any (`E501 line too long` is not a hard rule)
>>     `git diff HEAD~1 -U0 | pep8 --diff`
>>
>> 3. some typos
>>
>> 4. A lot of unused imports
>>
>> 5. Option name --sname for 'Segment identifier' is not very friendly. I
>> don't see any examples of command options in the design notes.
>>
>> 6. NO_UPG_MAGIC - leftover from other plugin?
>>
>> 7. suffix object has labels from segment
>>
>> 8. IPA framework has a support for nested object. Key is setting
>> `parent_object = 'topologysuffix'` in topologysegment object.
>>
>> 9. repl_agmt_attrs could be in topologysegment takes_params.
>>
>> 10. missing various CRUD commands like topologysuffix-find and
>> topologysuffix-show commands.
>>
>> Whats missing, not fixed:
>> 1. last 2 lines of VERSION file are not updated
>>
>> 2. Mixed terminology. Somewhere is used suffix and somewhere replication
>> area or just area.
>>
>> 3. Validation
>> - suffix should check for dn
>> - existence of both ends of a segment
>>
>> 4. print of segments in suffix-show needs to be improved or removed
>>
>> To discuss:
>> a) Do params in topologysegment have to have a maxlength set?
>>
>> b) Terminology has to be united. Segments are nested in suffix but
>> sometimes are called areas and suffix is 'the suffix'. User might be
>> confused. E.g. shouldn't the object be named a topologyarea instead of
>> topologysuffix?
>>
>> c) I've added all missing CRUD commands. Are there any which we don't
>> want there, or want to restrict them. E.g. I can imagine that deleting a
>> suffix should be prevented if it contains any segments (or it has to be
>> forced (--force option))
>>
>> d) Do we want to print segments in suffix-show?
>>
>> e) Mainly for Honza: I've added --show-segments option to suffix-show
>> which defaults to True. I don't like the behavior of CLI, which asks to
>> confirm the value all the time. My intention was to have it there by
>> default, but also allow to disable it by --show-segments=False. I don't
>> want to add it as Flag (--hide-segments) since it restricts versatility.
>> I would like to see an optional flag which would be filled by default
>> value if not explicitly defined and CLI would not ask for the option 
>> value.
>>
>>
>
> Also it would be better to split the work into more patches. E.g. DS 
> plugin, installation, python plugin. So ds plugin review could be 
> separated from the python part.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-lkrispen-0003-plugin-part-of-topology-management.patch
Type: text/x-patch
Size: 144983 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/5543981c/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-lkrispen-0004-required-changes-from-review-1.patch
Type: text/x-patch
Size: 18826 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/5543981c/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-lkrispen-0006-topology-management-ipa-commands.patch
Type: text/x-patch
Size: 17089 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/5543981c/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-lkrispen-0005-install-part-of-topology-plugin.patch
Type: text/x-patch
Size: 10950 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/5543981c/attachment-0003.bin>