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

[Jan Cholasta]

On 29.9.2015 15:15, Martin Basti wrote:
>
>
> On 09/29/2015 03:11 PM, Milan Kubík wrote:
>> 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
> Pushed to ipa-4-2: ad285897f54190fd0113209f32fce7f37fb0ce77
> Pushed to master: 74da4f5870edda85039b3bba52fb0a578676fb44

Martin found an additional issued, see the attached patch for a fix.

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-498-install-fix-ipa-server-install-fail-on-missing-forwa.patch
Type: text/x-patch
Size: 2873 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151001/f5440d0b/attachment.bin>