[Freeipa-devel] [PATCH] 857 topology: ipa management commands

[Petr Vobornik]

On 06/03/2015 10:59 AM, Martin Babinsky wrote:
> On 06/03/2015 10:52 AM, Martin Babinsky wrote:
>> On 05/26/2015 03:31 PM, Petr Vobornik wrote:
>>> On 05/26/2015 12:19 PM, Petr Vobornik wrote:
>>>> this patch is based on top of my patch #856 and tbabej'
>>>> s 325-9.
>>>>
>>>> Obsoletes Ludwig's 0006.
>>>>
>>>> ipalib part of topology management
>>>>
>>>> Design:
>>>> - http://www.freeipa.org/page/V4/Manage_replication_topology
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/4302
>>>>
>>>>
>>>
>>> New version attached:
>>> - domainlevel_show usage changed to domainlevel_get
>>> - updated VERSION
>>> - added more attrs to default_attributes
>>>
>>>
>>
>> Hi Petr,
>>
>> the commands themselves seem to work just fine. I had encountered some
>> quirks in the underlying topology plugin, but I will address them in a
>> different thread in order to keep the discussion relevant to the
>> reviewed patch.
>>
>> I have some minor coomments below:
>>
>> 1.)
>>   IPA_API_VERSION_MAJOR=2
>> -IPA_API_VERSION_MINOR=121
>> -# Last change: pvoborni - added server-find and server-show
>> +IPA_API_VERSION_MINOR=122
>> +# Last change: pvoborni - added topology management commands
>>
>> Several people were touching API in the meantime so please double-check
>> that you have correct VERSION and regenerate API.txt

Patch rebased.

>>
>> 2.)
>>
>> +        Str(
>> +            'nsds5replicatedattributelist?',
>> +            cli_name='replattrs',
>> +            label='Attributes to replicate',
>> +            doc=_('Attributes that are not replicated to a consumer
>> server '
>> +                  'during a fractional update. E.g., `(objectclass=*) '
>> +                  '$ EXCLUDE accountlockout memberof'),
>> +        ),
>> +        Str(
>> +            'nsds5replicatedattributelisttotal?',
>> +            cli_name='replattrstotal',
>> +            label=_('Attributes for total update'),
>> +            doc=_('Attributes that are not replicated to a consumer
>> server '
>> +                  'during a total update. E.g. (objectclass=*) $
>> EXCLUDE '
>> +                  'accountlockout'),
>>
>> The descriptions of these two options confused me greatly, are these
>> attributes supposed to be replicated or not, or is there some more
>> complex logic behind them that I failed to grasp? I am cc'ing Ludwig, he
>> can probably explain them to us and then we can decide whether we may
>> alter the descriptions to be less confusing.
>>
>> 3.)
>>
>> +    takes_params = (
>> +        Str(
>> +            'cn',
>> +            cli_name='name',
>> +            primary_key=True,
>> +            label=_('Suffix name'),
>> +        ),
>> +        Str(
>> +            'iparepltopoconfroot',
>> +            maxlength=255,
>> +            cli_name='suffix',
>> +            label=_('Suffix to be managed'),
>> +            normalizer=lambda value: value.lower(),
>> +        ),
>> +    )
>>
>> This also confused me at first, I suggest to change the label of
>> 'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or
>> 'LDAP subtree to be managed'.

Changed to 'LDAP suffix to be managed'

>>
>> 4.)
>>
>> There is currently no way to rename existing topology segments/suffixes.
>> In the case of hosts with funky FQDN's (pointing at you, ABC lab), the
>> segment cn's created during replica installs are mearly impossible to
>> remember and it would be nice to rename them to something more
>> manageable. However, this is not related to core functionality and can
>> be a subject of a separate patch once this gets pushed.
>>
>> That's all from my side.
>>
>
> I also forgot to ask what is the expected policy when deleting a
> non-empty topology suffix. If this is not supported and you have to
> first remove all segments and then the suffix itself, the
> 'topologysuffix-del' command should issue an error pointing the user to
> correct procedure.
>

Do we have a use case for creation or deletion of topology suffix?
-- 
Petr Vobornik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0857-2-topology-ipa-management-commands.patch
Type: text/x-patch
Size: 27572 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150603/a776139e/attachment.bin>