Re: [Freeipa-devel] [PATCH] 381 Preserve order of servers in ipa-client-install
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) +
[Freeipa-devel] [PATCH] 381 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. https://fedorahosted.org/freeipa/ticket/3418 -- 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 From 1ba31219970ca32cfc27850a0655fa5b0c0a9da8 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. https://fedorahosted.org/freeipa/ticket/3418 --- ipa-client/ipaclient/ipadiscovery.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ipa-client/ipaclient/ipadiscovery.py b/ipa-client/ipaclient/ipadiscovery.py index 7fc6aae88672e15a6bf3068ef8769e4cc80184a4..f58937e7b9e14712ca025af3236416560af4beaa 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 @@ -268,7 +268,7 @@ class IPADiscovery(object): root_logger.warn( '%s (realm %s) is not an IPA server', server, self.realm) elif ldapret[0] == NO_LDAP_SERVER: -root_logger.debug( +root_logger.warn( 'Unable to verify that %s (realm %s) is an IPA server', server, self.realm) -- 1.8.1.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 381 Preserve order of servers in ipa-client-install
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. -- 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. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
