[Freeipa-devel] [PATCH 003] Use 'remove-ds.pl' to remove DS instance during server uninstall

[Martin Basti]

On 26/01/15 14:05, Martin Babinsky wrote:
> On 01/26/2015 01:59 PM, Martin Babinsky wrote:
>> On 01/23/2015 05:56 PM, Martin Basti wrote:
>>> On 22/01/15 15:03, Martin Babinsky wrote:
>>>> On 01/22/2015 12:38 PM, Martin Babinsky wrote:
>>>>> On 01/22/2015 12:19 PM, Martin Kosek wrote:
>>>>>> On 01/22/2015 11:58 AM, Martin Babinsky wrote:
>>>>>>> On 01/22/2015 11:04 AM, Martin Babinsky wrote:
>>>>>>>> The attached patch addresses
>>>>>>>> https://fedorahosted.org/freeipa/ticket/4487.
>>>>>>>>
>>>>>>>> Using 'remove-ds.pl' script from 389-ds during server/replica
>>>>>>>> uninstallation should allow for cleaner removal of DS instance
>>>>>>>> with no
>>>>>>>> leftovers (opened ports etc).
>>>>>>>>
>>>>>>>> Martin^3
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Freeipa-devel mailing list
>>>>>>>> Freeipa-devel at redhat.com
>>>>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>>>>>
>>>>>>> Thanks to Martin^2 for explaining the rules governing the placement
>>>>>>> of new
>>>>>>> attributes into BasePathNamespace class.
>>>>>>>
>>>>>>> Attaching updated patch.
>>>>>>>
>>>>>>> Martin^3
>>>>>>
>>>>>> I see you kept erase_ds_instance_data still in the dsinstance. 
>>>>>> What is
>>>>>> the
>>>>>> motivation? I thought that just like with PKI, with DS we will also
>>>>>> have
>>>>>> uninstall based solely only on the DS uninstall script and remove 
>>>>>> our
>>>>>> custom
>>>>>> removal.
>>>>>>
>>>>>> We can keep the manual removal somewhere in wiki, but I would
>>>>>> personally not
>>>>>> keep/maintain it in FreeIPA code.
>>>>>>
>>>>> I originally thought that I will keep erase_ds_instance-data as a
>>>>> failsafe method to clean up the dirsrv data in th case that
>>>>> remove-ds.pl
>>>>> fails.
>>>>>
>>>>> But I will remove the method altogether and change the code so 
>>>>> that it
>>>>> prints out some warning about manual removal of DS data when
>>>>> remove-ds.pl fails.
>>>>>
>>>>> Martin^2
>>>>>
>>>>> _______________________________________________
>>>>> Freeipa-devel mailing list
>>>>> Freeipa-devel at redhat.com
>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>> Updated patch attached
>>>>
>>>> Martin^3
>>>>
>>>
>>> Hi,
>>>
>>> thank you for patch, but I have a few nitpicks:
>>>
>>> 1) Extra parenthesis
>>> +                root_logger.error(("Failed to remove CA DS instance.
>>> You may "
>>> +                                   "need to remove instance data
>>> manually"))
>>>
>>> and (twice)
>>>
>>> +                root_logger.error(("Failed to remove DS instance. You
>>> may "
>>> +                                   "need to remove instance data
>>> manually"))
>>>
>>> and
>>>
>>> +        root_logger.debug(("'%s' failed. "
>>> +                          "Attempting to force removal" %
>>> paths.REMOVE_DS_PL))
>>>
>>> 2) Remove unused paths:
>>> USR_LIB_DIRSRV_SLAPD_INSTANCE_DIR_TEMPLATE
>>> SLAPD_INSTANCE_LOCK_TEMPLATE
>>> USR_LIB_SLAPD_INSTANCE_TEMPLATE
>>>
>>> Please do double check.
>>>
>>> 3) IMO we should avoid hardcoded value 'slapd'
>>> instance_name = '-'.join(['slapd', serverid])
>>>
>>> you may create module variable DS_INSTANCE_PREFIX and use it. Dont
>>> forget to change it in following place as well.
>>> ipaserver/install/dsinstance.py:117:    instance_prefix = 'slapd-'
>>>
>>> 4)
>>> DS keytab hasn't been removed, it is okay?
>>> It is created by IPA (krbinstance), not by DS install script.
>>>
>>>
>>> Martin^2
>>>
>> Thanks for suggestions, attaching updated patch
>>
>> Martin^3
>>
>>
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>
> Missed one little thing. Attaching updated patch.
>
> Martin^3
Thank you, ACK.

-- 
Martin Basti