[Freeipa-devel] [PATCH] 0056 Fix broken replication

[Petr Viktorin]

On 08/20/2013 01:39 PM, Ana Krivokapic wrote:
> On 08/20/2013 01:17 PM, Petr Viktorin wrote:
>> On 08/19/2013 06:16 PM, Ana Krivokapic wrote:
>>> On 08/19/2013 06:01 PM, Petr Viktorin wrote:
>>>>> On 08/19/2013 05:50 PM, Ana Krivokapic wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> This patch addresses tickethttps://fedorahosted.org/freeipa/ticket/3868.
>>>>>>>
>>>>>>> -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red
>>>>>>> Hat Inc.
>>>>>>>
>>>>>>>
>>>>>>> freeipa-akrivoka-0056-Fix-broken-replication.patch
>>>>>>>
>>>>>>>
>>>>>>>   From cdcb28b9b3b8e45db1b7a61f0df6f41e7a61450a Mon Sep 17 00:00:00 2001
>>>>>>> From: Ana Krivokapic<akrivoka at redhat.com>
>>>>>>> Date: Mon, 19 Aug 2013 17:45:31 +0200
>>>>>>> Subject: [PATCH] Fix broken replication
>>>>>>>
>>>>>>> Make sure the subject base parameter is correctly passed and used during the
>>>>>>> creation of the DS instance on a replica.
>>>>>>>
>>>>>>> https://fedorahosted.org/freeipa/ticket/3868
>>>>>>> ---
>>>>> [...]
>>>>>>> --- a/ipaserver/install/dsinstance.py
>>>>>>> +++ b/ipaserver/install/dsinstance.py
>>>>>>> @@ -275,7 +275,7 @@ def create_instance(self, realm_name, fqdn, domain_name,
>>>>>>>
>>>>>>>        def create_replica(self, realm_name, master_fqdn, fqdn,
>>>>>>>                           domain_name, dm_password, pkcs12_info=None,
>>>>>>> -                       ca_file=None):
>>>>>>> +                       ca_file=None, subject_base=None):
>>>>>
>>>>> Does it ever make sense to have subject_base=None here?
>>>>>
>>>>>
>>> I don't think so. Fixed.
>>>
>>> Also changed the commit message and ticket summary, as suggested by Rob.
>>>
>>> Updated patch is attached.
>>>
>>> -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red
>>> Hat Inc.
>>>
>>>
>>> freeipa-akrivoka-0056-02-Fix-broken-replica-installation.patch
>>>
>>>
>>>   From 0730de02f665da080956175e78c263a011416dc2 Mon Sep 17 00:00:00 2001
>>> From: Ana Krivokapic<akrivoka at redhat.com>
>>> Date: Mon, 19 Aug 2013 17:45:31 +0200
>>> Subject: [PATCH] Fix broken replica installation
>>>
>>> Make sure the subject base parameter is correctly passed and used during the
>>> creation of the DS instance on a replica.
>>>
>>> https://fedorahosted.org/freeipa/ticket/3868
>>> ---
>>>    install/tools/ipa-replica-install | 14 ++++++++++----
>>>    ipaserver/install/dsinstance.py   |  6 +++---
>>>    2 files changed, 13 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/install/tools/ipa-replica-install
>>> b/install/tools/ipa-replica-install
>>> index
>>> 79f8a7ab48f75ac2d9cd5149df6eda4784b3854a..8be57bf7d6f5ed956f3d666b6518ea18055d9df6
>>> 100755
>>> --- a/install/tools/ipa-replica-install
>>> +++ b/install/tools/ipa-replica-install
>>> @@ -162,10 +162,16 @@ def install_replica_ds(config):
>>>                           config.dir + "/dirsrv_pin.txt")
>>>
>>>        ds = dsinstance.DsInstance()
>>> -    ds.create_replica(config.realm_name,
>>> -                      config.master_host_name, config.host_name,
>>> -                      config.domain_name, config.dirman_password,
>>> -                      pkcs12_info, ca_file = config.dir + "/ca.crt")
>>> +    ds.create_replica(
>>> +        config.realm_name,
>>> +        config.master_host_name,
>>> +        config.host_name,
>>> +        config.domain_name,
>>> +        config.dirman_password,
>>> +        config.subject_base,
>>> +        pkcs12_info,
>>> +        ca_file=config.dir + "/ca.crt",
>>> +    )
>>
>> Here's small a nitpick; if you don't get to it by the time I test the patch
>> I'll aim to ack the current version.
>>
>> Since there's lots of arguments that apparently tend to get rearranged, I
>> think it's good style to name each one when calling the method (e.g.
>> `fqdn=config.master_host_name,`). That way if you for some reason end up with
>> a mismatched version of create_replica (common during development), it would
>> fail loudly rather than doing the wrong thing.
>>
>
> Agreed, updated patch attached.

Thanks, ACK, pushed to

master: c318213250af68c02cc72fa609b3b844f929166b[[BR]]
ipa-3-3: 646db4d90d5d31a36099ada749671e66a6fd78c4


-- 
Petr³