[Freeipa-devel] [PATCH 0058] ipa-restore: check whether DS is running before attempting connection

[Jan Cholasta]

On 18.8.2015 14:24, Martin Babinsky wrote:
> On 08/18/2015 02:05 PM, Jan Cholasta wrote:
>> On 18.8.2015 13:41, Martin Basti wrote:
>>>
>>>
>>> On 08/18/2015 01:16 PM, Alexander Bokovoy wrote:
>>>> On Tue, 18 Aug 2015, Martin Babinsky wrote:
>>>>> https://fedorahosted.org/freeipa/ticket/4838
>>>>>
>>>>> --
>>>>> Martin^3 Babinsky
>>>>
>>>>> From d86aae6c3fef4dea1afbbdbacbc978afbbfa5fcf Mon Sep 17 00:00:00 2001
>>>>> From: Martin Babinsky <mbabinsk at redhat.com>
>>>>> Date: Tue, 18 Aug 2015 12:47:46 +0200
>>>>> Subject: [PATCH] ipa-restore: check whether DS is running before
>>>>> attempting
>>>>> connection
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/4838
>>>>> ---
>>>>> ipaserver/install/ipa_restore.py | 7 +++++++
>>>>> 1 file changed, 7 insertions(+)
>>>>>
>>>>> diff --git a/ipaserver/install/ipa_restore.py
>>>>> b/ipaserver/install/ipa_restore.py
>>>>> index
>>>>> 65cb49355a0567446debe9c367aa3c1bc5a12e1c..b69ea90001a6fd03d0fddde8844870d1aa9c3704
>>>>>
>>>>>
>>>>> 100644
>>>>> --- a/ipaserver/install/ipa_restore.py
>>>>> +++ b/ipaserver/install/ipa_restore.py
>>>>> @@ -410,6 +410,13 @@ class Restore(admintool.AdminTool):
>>>>>         '''
>>>>>         Create an ldapi connection and bind to it using autobind as
>>>>> root.
>>>>>         '''
>>>>> +        instance_name = installutils.realm_to_serverid(api.env.realm)
>>>>> +
>>>>> +        if not
>>>>> services.knownservices.dirsrv.is_running(instance_name):
>>>>> +            raise admintool.ScriptError(
>>>>> +                "directory server instance is not running/configured"
>>>>> +            )
>>>>> +
>>>>>         if self._conn is not None:
>>>>>             return self._conn
>>>>>
>>>> ACK.
>>>>
>>> Pushed to:
>>> master: 31776755b47f44f894e4f2bb256aca1c5262b8a8
>>> ipa-4-2: e4b8cffdb4e017874bb7f12a7ca362b927ead67a
>>>
>>
>> I didn't try, but I'm pretty sure this patch breaks ipa-restore on
>> systems without IPA installed, which was not at all the point of the
>> ticket - the point was to replace the "Unable to get connection,
>> skipping disabling agreements: Unable to bind to LDAP server: [Errno 2]
>> No such file or directory" error message with something meaningful in
>> such a case.
>>
>
> I have just now tested ipa-restore without installed IPA master and it
> works just fine.
>
> The point of my patch was no not even try to connect to DS if the
> instance is not running/configured and raise an error telling this fact
> to the user, instead of timing out on connection and then raising a
> generic exception.
>
> So unless I missed something it should be ok.
>

I stand corrected.

It was the ScriptError that made me suspicious.

Sorry for the noise.

-- 
Jan Cholasta