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
From 2f33c8113e79311a28858bc55d3f7a534dfb920d Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Thu, 7 Mar 2013 13:54:11 +0100
Subject: [PATCH] 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 | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/ipa-client/ipaclient/ipadiscovery.py b/ipa-client/ipaclient/ipadiscovery.py
index 7fc6aae88672e15a6bf3068ef8769e4cc80184a4..c4ec7323f54ae3d013c9e175c313f86b6f38ffc9 100644
--- a/ipa-client/ipaclient/ipadiscovery.py
+++ b/ipa-client/ipaclient/ipadiscovery.py
@@ -248,7 +248,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
@@ -258,7 +258,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
@@ -266,11 +266,17 @@ class IPADiscovery(object):
break
elif ldapret[0] == NOT_IPA_SERVER:
root_logger.warn(
- '%s (realm %s) is not an IPA server', server, self.realm)
+ '%s: not an IPA server. Removing from list of servers '
+ 'to connect to', 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(
+ '%s: LDAP server is not responding, unable to verify if '
+ 'this is an IPA server. Removing from list of servers '
+ 'to connect to', server)
+ else:
+ root_logger.warn(
+ '%s: cannot verify if this is an IPA server. Removing '
+ 'from list of servers to connect to', server)
# If one of LDAP servers checked rejects access (maybe anonymous
# bind is disabled), assume realm and basedn generated off domain.
@@ -344,7 +350,7 @@ class IPADiscovery(object):
basedn = get_ipa_basedn(lh)
if basedn is None:
- root_logger.debug("The server is not an IPA server")
+ root_logger.debug("%s: The server is not an IPA server", thost)
return [NOT_IPA_SERVER]
self.basedn = basedn
@@ -384,25 +390,26 @@ class IPADiscovery(object):
except LDAPError, err:
if isinstance(err, ldap.TIMEOUT):
- root_logger.debug("LDAP Error: timeout")
+ root_logger.debug("%s: LDAP Error: timeout", thost)
return [NO_LDAP_SERVER]
if isinstance(err, ldap.SERVER_DOWN):
- root_logger.debug("LDAP Error: server down")
+ root_logger.debug("%s: LDAP Error: server down", thost)
return [NO_LDAP_SERVER]
if isinstance(err, ldap.INAPPROPRIATE_AUTH):
- root_logger.debug("LDAP Error: Anonymous access not allowed")
+ root_logger.debug("%s: LDAP Error: Anonymous access not allowed", thost)
return [NO_ACCESS_TO_LDAP]
# We should only get UNWILLING_TO_PERFORM if the remote LDAP server
# has minssf > 0 and we have attempted a non-TLS connection.
if ca_cert_path is None and isinstance(err, ldap.UNWILLING_TO_PERFORM):
- root_logger.debug("LDAP server returned UNWILLING_TO_PERFORM. This likely means that minssf is enabled")
+ root_logger.debug("%s: LDAP server returned UNWILLING_TO_PERFORM. "
+ "This likely means that minssf is enabled", thost)
return [NO_TLS_LDAP]
- root_logger.error("LDAP Error: %s: %s" %
- (err.args[0]['desc'], err.args[0].get('info', '')))
+ root_logger.debug("%s: LDAP Error: %s: %s", thost,
+ err.args[0]['desc'],err.args[0].get('info', ''))
return [UNKNOWN_ERROR]
--
1.8.1.4
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
