[Freeipa-devel] [PATCHES 466-468, 0316] install: Add common base class for server and replica install

[Milan Kubík]

On 09/23/2015 05:01 PM, Martin Basti wrote:
>
>
> On 09/22/2015 12:10 PM, Jan Cholasta wrote:
>> On 22.9.2015 10:29, Martin Babinsky wrote:
>>> On 09/16/2015 10:44 AM, Jan Cholasta wrote:
>>>> On 16.9.2015 08:11, Jan Cholasta wrote:
>>>>> On 15.9.2015 07:22, Jan Cholasta wrote:
>>>>>> On 10.8.2015 16:58, Martin Babinsky wrote:
>>>>>>> On 08/06/2015 08:22 AM, Jan Cholasta wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> the attached patch fixes part of
>>>>>>>> <https://fedorahosted.org/freeipa/ticket/4517>.
>>>>>>>>
>>>>>>>> See also Martin Babinsky's patch 51:
>>>>>>>> <https://www.redhat.com/archives/freeipa-devel/2015-August/msg00012.html>. 
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Honza
>>>>>>>>
>>>>>>>
>>>>>>> Sorry but NACK, see below:
>>>>>>>
>>>>>>> 1.) it seems that passing kwargs to Server components doesn't 
>>>>>>> work as
>>>>>>> expected. See these logs (install on fresh F22 VM):
>>>>>>>
>>>>>>> http://fpaste.org/253416/21363814/
>>>>>>> http://fpaste.org/253419/43921374/
>>>>>>
>>>>>> Fixed.
>>>>>>
>>>>>>>
>>>>>>> 2.) the following code blows up in BaseServers' __init__:
>>>>>>> (http://fpaste.org/253400/21225314/)
>>>>>>>
>>>>>>> 392         if not self.dns.setup_dns:
>>>>>>> 393             if self.dns.forwarders:
>>>>>>> 394                 raise RuntimeError(
>>>>>>> 395                     "You cannot specify a --forwarder option
>>>>>>> without
>>>>>>> the "
>>>>>>> 396                     "--setup-dns option")
>>>>>>>
>>>>>>>
>>>>>>> I think that the check should be:
>>>>>>>
>>>>>>> 392         if not self.setup_dns:
>>>>>>> 393             if self.dns.forwarders:
>>>>>>
>>>>>> Fixed.
>>>>>>
>>>>>>>
>>>>>>> IMHO BaseServerDNS class shouldn't have setup_dns knob, that 
>>>>>>> should be
>>>>>>> set in the parent class (BaseServer)
>>>>>>
>>>>>> Fixed.
>>>>>>
>>>>>>>
>>>>>>> 3.) Is there any reason why BaseServer doesn't have 
>>>>>>> 'master_password',
>>>>>>> 'idmax' and 'idstart' knobs? I know that these are then brought 
>>>>>>> in by
>>>>>>> the derived Server class, but the check for them is in parent's
>>>>>>> __init__() method and it is IMHO a bit confusing
>>>>>>
>>>>>> The check should be in Server, fixed.
>>>>>>
>>>>>>>
>>>>>>> 4.) please add license header to the beginning of
>>>>>>> 'ipaserver/install/server/common.py' file
>>>>>>
>>>>>> Added.
>>>>>>
>>>>>> Updated patches attached.
>>>>>
>>>>> Self-NACK, I broke ipa-server-install --uninstall.
>>>>
>>>> Fixed.
>>>>
>>>
>>> ACK to all three patches.
>>>
>>
>> Thanks.
>>
>> Pushed to:
>> master: 86edd6abeb9749e159a529b83cfce6443fff4ba5
>> ipa-4-2: 42d16b02cd153ac89ebd8ae07c98611dc3b6e471
>>
> These patches introduced regression.
> ipa-replica-install in unattended mode requires to specify -a, -p and 
> -r options.
>
> Attached patch fixes it.
>
>
>
Works for me, ACK.

Milan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150929/a388f357/attachment.htm>