[Freeipa-devel] [PATCH 0017] Improve error message in ipa-replica-manage

Rob Crittenden rcritten at redhat.com
Wed Oct 31 15:55:35 UTC 2012


Tomas Babej wrote:
> On 10/24/2012 04:40 AM, Rob Crittenden wrote:
>> Tomas Babej wrote:
>>> On 10/19/2012 09:55 AM, Petr Viktorin wrote:
>>>> On 10/18/2012 08:01 PM, Rob Crittenden wrote:
>>>>> Tomas Babej wrote:
>>>>>> On 10/02/2012 03:55 PM, Rob Crittenden wrote:
>>>>>>> Tomas Babej wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> When executing ipa-replica-manage connect to an unknown or
>>>>>>>> irrelevant
>>>>>>>> master, we now print a sensible error message informing the user
>>>>>>>> about this possiblity as well.
>>>>>>>>
>>>>>>>> https://fedorahosted.org/freeipa/ticket/3105
>>>>>>>>
>>>>>>>> Tomas
>>>>>>>
>>>>>>> I put a whole bunch of code into a try/except and this may be
>>>>>>> catching
>>>>>>> errors in unexpected ways.
>>>>>>>
>>>>>>> I'm not entirely sure right now what we should do, but looking at
>>>>>>> the
>>>>>>> code in the try:
>>>>>>>
>>>>>>> repl1.conn.getEntry(master1_dn, ldap.SCOPE_BASE)
>>>>>>> repl1.conn.getEntry(master2_dn, ldap.SCOPE_BASE)
>>>>>>>
>>>>>>> We take in replica1 and replica1 as arguments (the default for
>>>>>>> replica1 is the current host).
>>>>>>>
>>>>>>> If either of these raise a NotFound it means there there is no
>>>>>>> master
>>>>>>> of that name. Does that mean that the master was deleted? Well,
>>>>>>> clearly not.
>>>>>>>
>>>>>>> A lot has changed since I did this, I may have been relying on a
>>>>>>> side-effect, or just hadn't tested well-enough.
>>>>>>>
>>>>>>> I wonder if we need that message at all. Is "foo" is not an IPA
>>>>>>> server
>>>>>>> good enough? It still might be confusing if someone didn't know that
>>>>>>> "foo" was deleted and it was still running. We could probably verify
>>>>>>> that it is at least an IPA server by doing similar checking in the
>>>>>>> client, it all depends on how far we want to take it.
>>>>>>>
>>>>>>> rob
>>>>>>
>>>>>> I modified the patch. Now if the NotFound error is encountered, we
>>>>>> try
>>>>>> to investigate whether we're trying to connect to an IPA server at
>>>>>> all.
>>>>>> Please see if you have any suggestions.
>>>>>>
>>>>>> Tomas
>>>>>
>>>>> Getting there, the errors are a lot better.
>>>>>
>>>>> Can you modify the 'Connection unsuccessful' message to include the
>>>>> server we failed to connect to?
>>>>>
>>>>> The logger isn't so nice either, I think I'd prefer it if we could
>>>>> exclude the severity:
>>>>>
>>>>> ipa: ERROR: LDAP Error: Connect error: TLS error -8172:Peer's
>>>>> certificate issuer has been marked as not trusted by the user.
>>>>> Connection unsuccessful.
>>>>>
>>>>> So drop the 'ipa: ERROR: ' part for consistency. TI don't believe we
>>>>> configure the root logger at all in this tool so it is using the
>>>>> defaults.
>>>>
>>>> It's not very easy to find the right call to configure the logger to
>>>> drop the "ipa: ERROR:" part:
>>>> standard_logging_setup(console_format='%(message)s')
>>>> The function is in ipapython.ipa_log_manager. Hopefully that helps.
>>>>
>>> Thanks!
>>>>> I'd also replace the test for -4 with the constant
>>>>> ipadiscovery.NOT_IPA_SERVER
>>>>>
>>>>> rob
>>>>>
>>>>
>>> I implemented your suggestions. Please have a look at the new patch
>>> version.
>>>
>>> Tomas
>>
>> Getting a traceback:
>>
>> # ipa-replica-manage connect otherinstall.example.com
>> LDAP Error: Connect error: TLS error -8054:You are attempting to
>> import a cert with the same issuer/serial as an existing cert, but
>> that is not the same cert.
>> Traceback (most recent call last):
>>   File "/usr/sbin/ipa-replica-manage", line 862, in <module>
>>     main()
>>   File "/usr/sbin/ipa-replica-manage", line 845, in main
>>     add_link(realm, replica1, replica2, dirman_passwd, options)
>>   File "/usr/sbin/ipa-replica-manage", line 732, in add_link
>>     sys.exit("Connection to %s unsuccessful.", replica2)
>> TypeError: exit expected at most 1 arguments, got 2
>>
>> I was just going to fix this and push and discovered another thing to
>> improve.
>>
>> Trying to connect to a host not in DNS doesn't return a very nice
>> message. It says that the CA wasn't found, which is true in an offbeat
>> sort of way I guess.
>>
>> Here is a quick mock-up to see if a host resolves. Note that we can't
>> use ipapython.ipautil.is_host_resolvable() because that only uses DNS.
>>
>> I whipped this up in 2 seconds just to see how it'd work. The name
>> stinks, it should probably go into ipapython/ipautil and the way I'm
>> checking in add_link would lead to a lot of duplication if tried in
>> the other commands. Probably best to have try to centralize the
>> message and sys.exit().
>>
>> --- ipa-replica-manage  2012-10-23 22:08:36.426435673 -0400
>> +++ /usr/sbin/ipa-replica-manage        2012-10-23 22:34:36.946371449
>> -0400
>> @@ -21,6 +21,7 @@
>>  import os
>>
>>  import ldap, re, krbV
>> +import socket
>>  import traceback
>>  from urllib2 import urlparse
>>
>> @@ -54,6 +55,19 @@
>>      "list-clean-ruv":(0, 0, "", ""),
>>  }
>>
>> +def host_exists(host):
>> +    """
>> +    Resolve the host to see if it exists.
>> +
>> +    Returns True/False
>> +    """
>> +    try:
>> +        socket.getaddrinfo(host, 80)
>> +    except socket.gaierror:
>> +        return False
>> +    else:
>> +        return True
>> +
>>  def convert_error(exc):
>>      """
>>      LDAP exceptions are a dictionary, make them prettier.
>> @@ -652,6 +666,11 @@
>>
>>  def add_link(realm, replica1, replica2, dirman_passwd, options):
>>
>> +    if not host_exists(replica1):
>> +        sys.exit("unknown host %s" % replica1)
>> +    if not host_exists(replica2):
>> +        sys.exit("unknown host %s" % replica2)
>> +
>>      if options.winsync:
>>          if not options.binddn or not options.bindpw or not
>> options.cacert or not options.passsync:
>>              root_logger.error("The arguments --binddn, --bindpw,
>> --passsync and --cacert are required to create a winsync agreement")
>> @@ -729,7 +748,7 @@
>>                      "but it might be unknown, foreign or previously
>> deleted "
>>                      "one." % replica2)
>>              else:
>> -                sys.exit("Connection to %s unsuccessful.", replica2)
>> +                sys.exit("Connection to %s unsuccessful." % replica2)
>>
>>          repl1.setup_gssapi_replication(replica2, DN(('cn', 'Directory
>> Manager')), dirman_passwd)
>>      print "Connected '%s' to '%s'" % (replica1, replica2)
>>
> I added the host_exists function to ipautil.py and added check for
> hosts in each relevant function in ipa-replica-manage. I retested
> the patch, seems to work as expected.
>
> Tomas

Looks good. Pushed to master and ipa-3-0

rob




More information about the Freeipa-devel mailing list