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

[Martin Basti]

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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0316-Replica-inst.-fix-do-not-require-r-a-p-options-in-un.patch
Type: text/x-patch
Size: 2412 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150923/d2d997cc/attachment.bin>