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

[Martin Basti]

On 10/01/2015 12:55 PM, Jan Cholasta wrote:
> 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.
>
ACK

Pushed to:
ipa-4-2: 75a8454caeeaf293c0b6be48f2b8476e7707447f
master: 6067824be494745926204a7ba3709c3c0f054326