On 01/26/2015 05:46 PM, Martin Basti wrote:
> 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@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@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@redhat.com
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>
>> Missed one little thing. Attaching updated patch.
>>
>> Martin^3
> Thank you, ACK.
> 

Pushed to master: 55b7eed77e5f76c159ba157d020e93aa9d43bdc5

Martin

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to