[Freeipa-devel] [PATCH 0073] Remove support for IPA deployments with no persistent search

[Martin Kosek]

On 07/15/2013 04:41 PM, Petr Spacek wrote:
> On 15.7.2013 16:15, Simo Sorce wrote:
>> On Mon, 2013-07-15 at 15:57 +0200, Martin Kosek wrote:
>>> On 07/15/2013 03:44 PM, Petr Spacek wrote:
>>>> On 15.7.2013 15:31, Martin Kosek wrote:
>>>>> On 07/11/2013 05:10 PM, Tomas Babej wrote:
>>>>>> On Thursday 11 of July 2013 16:10:33 Ana Krivokapic wrote:
>>>>>>
>>>>>>> On 07/11/2013 11:20 AM, Tomas Babej wrote:
>>>>>>
>>>>>>>> boolean_var = {}
>>>>>>
>>>>>>>> - for var in ('persistent_search', 'serial_autoincrement'):
>>>>>>
>>>>>>>> + for var in ('serial_autoincrement'):
>>>>>>
>>>>>>> This won't work - a one element tuple needs a comma at the end:
>>>>>>
>>>>>>> ('serial_autoincrement', )
>>>>>>
>>>>>>>> boolean_var[var] = "yes" if getattr(self, var, False) else "no"
>>>>>>
>>>>>>>>
>>>>>>
>>>>>>>> self.sub_dict = dict(FQDN=self.fqdn,
>>>>>>
>>>>>>>> @@ -607,9 +604,8 @@ class BindInstance(service.Service):
>>>>>>
>>>>>>>> SUFFIX=self.suffix,
>>>>>>
>>>>>>>> OPTIONAL_NTP=optional_ntp,
>>>>>>
>>>>>>>> ZONEMGR=self.zonemgr,
>>>>>>
>>>>>>>> - ZONE_REFRESH=self.zone_refresh,
>>>>>>
>>>>>>>> IPA_CA_RECORD=ipa_ca,
>>>>>>
>>>>>>>> - PERSISTENT_SEARCH=boolean_var['persistent_search'],
>>>>>>
>>>>>>>> + PERSISTENT_SEARCH="yes",
>>>>>>
>>>>>>>> SERIAL_AUTOINCREMENT=boolean_var['serial_autoincrement'],)
>>>>>>
>>>>>>>
>>>>>>
>>>>>>> But anyway, I think this piece of code is unnecessarily complicated, I
>>>>>>> don't see
>>>>>>
>>>>>>> a need for the 'boolean_var' dict here. I would suggest replacing it with
>>>>>>
>>>>>>> something like:
>>>>>>
>>>>>>>
>>>>>>
>>>>>>> serial_autoincrement = "yes" if self.serial_autoincrement else "no"
>>>>>>
>>>>>>>
>>>>>>
>>>>>>> and then pass serial_autoincrement to self.sub_dict = dict(...)
>>>>>>
>>>>>>>
>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Attached patch refactored the relevant part of the code.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Tomas
>>>>>>
>>>>>
>>>>> Thanks for patches! I am just thinking, should we also hide the respective
>>>>> option from ipa global DNS configuration? That's idnszonerefresh attribute.
>>>>>
>>>>> We may want to mark the attribute as invisible in CLI + remove it from Web
>>>>> UI.
>>>>> Petr - what is your take on this? Do you plan to remove idnszonerefresh
>>>>> attribute support in the future (Fedora 20) as persistent search will be
>>>>> mandatory in that time?
>>>>
>>>> Yes, you are right. We completely forgot to web UI. And yes - please remove
>>>> the
>>>> option from web UI.
>>>
>>> Ok, Tomas please do the changes as proposed above.
>>>
>>>>
>>>> The latest development shows that persistent search will be replaced by RFC
>>>> 4533 (known as 'syncrepl'), but from user's point of view it doesn't matter.
>>>> All options related to persistent search and zone_refresh will simply
>>>> disappear. Syncrepl itself doesn't require explicit configuration.
>>>
>>> Ah, so this means that "psearch" option will be also removed from
>>> bind-dyndb-ldap? In Fedora 19 we just plan to hard-code it to "yes", will that
>>> cause issues with Fedora 20? Should we already avoid using the "psearch" option
>>> and assume that bind-dyndb-ldap in Fedora 19 is using persistent search by
>>> default?
>>
>> Won't the new bind-dyndb-ldap simply ignore the psearch option when it
>> moves to syncrepl ?
> 
> I can do it, but I think that cleanest way is to remove the 'psearch' option in
> upgrade script.

Hm, right, this should make the upgrade script a lot simpler - it would just
remove all psearch or zone_refresh references.

> 
> Another option is to release new bind-dyndb-ldap to Fedora 19 and change
> default values to 'psearch yes' right now. Do you agree?

Looks OK to me and would let us avoid doing any additional upgrade process for
Fedora 20 - are you planning to do a Fedora 19 release any time soon? If yes,
we can do the changes we are talking about in next 3.2.x release.

Martin