On 03/13/2013 02:58 PM, Martin Kosek wrote:
> On 03/11/2013 12:40 PM, Martin Kosek wrote:
>> On 03/07/2013 03:07 PM, Petr Viktorin wrote:
>>> On 03/07/2013 02:00 PM, Martin Kosek wrote:
>>>> When multiple servers are passed via --server option, ipadiscovery
>>>> module changed its order. Make sure that we preserve it.
>>>>
>>>> Also make sure that user is always warned when a tested server is
>>>> not available as then the server will be excluded from the fixed
>>>> server list.
>>>
>>> The message doesn't actually say that the server will be removed. It would 
>>> be
>>> nice if it did.
>>>
>>> Otherwise, ACK.
>>
>> Sending a patch with improved logging. User should now be more clear what
>> server is failing to verify (and why).
>>
>>>
>>>> ------
>>>>
>>>> When working on this ticket I was thinking - do we make the right thing we
>>>> deliberately remove a server from user-provided server list just because we
>>>> cannot connect to it at the moment if discovery? It may just be temporarily
>>>> down or something.
>>>>
>>>> Maybe we should preserve the original --server list in this case and use 
>>>> this
>>>> list when writing krb5.conf or sssd.conf. Of course, for ipa-join or other
>>>> active configuration commands we would have to use only the valid servers 
>>>> so
>>>> that the we do not hit the server that is currently down.
>>>>
>>>> Martin
>>>
>>> Good point, this deserves a ticket.
>>>
>>
>> Rob, do you think this deserves to be changed? Or is this behavior indeed
>> intended?
>>
>> Martin
>>
> 
> Sending a rebased patch 381, logging was also improved. Client discovery
> logging now looks like that:
> 
> # ipa-client-install
> Skip vm-024.idm.lab.bos.redhat.com: not an IPA server
> Skip doesnotexist.test: cannot verify if this is an IPA server
> Discovery was successful!
> Hostname: vm-148.idm.lab.bos.redhat.com
> Realm: IDM.LAB.BOS.REDHAT.COM
> DNS Domain: idm.lab.bos.redhat.com
> IPA Server: vm-037.idm.lab.bos.redhat.com
> BaseDN: dc=idm,dc=lab,dc=bos,dc=redhat,dc=com
> 
> Continue to configure the system with these values? [no]:
> ...
> 
> I also attached a related fix for redundant discoveries with fixed server list
> (found that when testing logging), details are in the patch description.
> 
> Martin
> 

I just creating a conflict in my patch numbering, renaming 386 to 387...

Martin
From a7a6d66e5f85f3a7bce4acdd78a1a144451c717a Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Wed, 13 Mar 2013 14:42:12 +0100
Subject: [PATCH 1/2] Preserve order of servers in ipa-client-install

When multiple servers are passed via --server option, ipadiscovery
module changed its order. Make sure that we preserve it.

Also make sure that user is always warned when a tested server is
not available as then the server will be excluded from the fixed
server list. Log messages were made more informative so that user
knows which server is actually failing to be verified.

https://fedorahosted.org/freeipa/ticket/3418
---
 ipa-client/ipaclient/ipadiscovery.py | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/ipa-client/ipaclient/ipadiscovery.py b/ipa-client/ipaclient/ipadiscovery.py
index 6ae9616def818642c6fdf88eb93b860befa19afd..9a58b967857a374809b2a85294010eb2f7f688a5 100644
--- a/ipa-client/ipaclient/ipadiscovery.py
+++ b/ipa-client/ipaclient/ipadiscovery.py
@@ -246,7 +246,7 @@ class IPADiscovery(object):
                 self.realm = ldapret[2]
                 self.server_source = self.realm_source = (
                     'Discovered from LDAP DNS records in %s' % self.server)
-                valid_servers.insert(0, server)
+                valid_servers.append(server)
                 # verified, we actually talked to the remote server and it
                 # is definetely an IPA server
                 verified_servers = True
@@ -256,7 +256,7 @@ class IPADiscovery(object):
                     break
             elif ldapret[0] == NO_ACCESS_TO_LDAP or ldapret[0] == NO_TLS_LDAP:
                 ldapaccess = False
-                valid_servers.insert(0, server)
+                valid_servers.append(server)
                 # we may set verified_servers below, we don't have it yet
                 if autodiscovered:
                     # No need to keep verifying servers if we discovered them
@@ -264,11 +264,14 @@ class IPADiscovery(object):
                     break
             elif ldapret[0] == NOT_IPA_SERVER:
                 root_logger.warn(
-                    '%s (realm %s) is not an IPA server', server, self.realm)
+                   'Skip %s: not an IPA server', server)
             elif ldapret[0] == NO_LDAP_SERVER:
-                root_logger.debug(
-                    'Unable to verify that %s (realm %s) is an IPA server',
-                                    server, self.realm)
+                root_logger.warn(
+                   'Skip %s: LDAP server is not responding, unable to verify if '
+                   'this is an IPA server', server)
+            else:
+                root_logger.warn(
+                   'Skip %s: cannot verify if this is an IPA server', server)
 
         # If one of LDAP servers checked rejects access (maybe anonymous
         # bind is disabled), assume realm and basedn generated off domain.
@@ -396,7 +399,7 @@ class IPADiscovery(object):
             return [UNKNOWN_ERROR]
 
         except errors.DatabaseTimeout:
-            root_logger.error("LDAP Error: timeout")
+            root_logger.debug("LDAP Error: timeout")
             return [NO_LDAP_SERVER]
         except errors.NetworkError, err:
             root_logger.debug("LDAP Error: %s" % err.strerror)
@@ -405,10 +408,10 @@ class IPADiscovery(object):
             root_logger.debug("LDAP Error: Anonymous access not allowed")
             return [NO_ACCESS_TO_LDAP]
         except errors.DatabaseError, err:
-            root_logger.error("Error checking LDAP: %s" % err.strerror)
+            root_logger.debug("Error checking LDAP: %s" % err.strerror)
             return [UNKNOWN_ERROR]
         except Exception, err:
-            root_logger.error("Error checking LDAP: %s" % err)
+            root_logger.debug("Error checking LDAP: %s" % err)
 
             return [UNKNOWN_ERROR]
 
-- 
1.8.1.4

From 4a772552c41a24361b75d556e2b914a6c887dc2e Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Wed, 13 Mar 2013 14:44:20 +0100
Subject: [PATCH 2/2] Avoid multiple client discovery with fixed server list

In client discovery module, we used to run up to three discovery
processes even though we received a fixed list of servers to connect
to. This could result in up to 3 identical "not an IPA server" error
messages when the passed server is not an IPA server.

Error out immediately when we are discovering against a fixed set
of servers.

Related to fixes in https://fedorahosted.org/freeipa/ticket/3418
---
 ipa-client/ipa-install/ipa-client-install | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 4433fc7175a7d565538fa7f3a681a36c0f91dc09..5910c7414ded85138c3d02002095299307b7e37c 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -1722,6 +1722,17 @@ def install(options, env, fstore, statestore):
 
     ret = ds.search(domain=options.domain, servers=options.server, hostname=hostname, ca_cert_path=get_cert_path(options.ca_cert_file))
 
+    if options.server and ret != 0:
+        # There is no point to continue with installation as server list was
+        # passed as a fixed list of server and thus we cannot discover any
+        # better result
+        root_logger.error("Failed to verify that %s is an IPA Server.",
+                ', '.join(options.server))
+        root_logger.error("This may mean that the remote server is not up "
+            "or is not reachable due to network or firewall settings.")
+        print_port_conf_info()
+        return CLIENT_INSTALL_ERROR
+
     if ret == ipadiscovery.BAD_HOST_CONFIG:
         root_logger.error("Can't get the fully qualified name of this host")
         root_logger.info("Check that the client is properly configured")
-- 
1.8.1.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to