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

[Martin Babinsky]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbabinsk-0003-5-use-remove-ds.pl-to-remove-DS-instance.patch
Type: text/x-patch
Size: 8863 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150126/80e786e9/attachment.bin>