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

2012-10-31 Thread Rob Crittenden

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-manage2012-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 

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

2012-10-25 Thread Tomas Babej

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-manage2012-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 

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

2012-10-25 Thread Tomas Babej

On 10/25/2012 12:40 PM, 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-manage2012-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:
 

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

2012-10-23 Thread Rob Crittenden

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-manage2012-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)

@@ 

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

2012-10-22 Thread Tomas Babej

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
From 228840f76b94194ec1c38bbb55bdabbfe6e0d9ae Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 2 Oct 2012 09:15:33 -0400
Subject: [PATCH] IPA Server check in ipa-replica-manage

When executing ipa-replica-manage connect to an master that raises
NotFound error we now check if the master is at least IPA server.
If so, we inform the user that it is probably foreign or previously
deleted master. If not, we inform the user that the master is not
an IPA server at all.

https://fedorahosted.org/freeipa/ticket/3105
---
 install/tools/ipa-replica-manage | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 897d117681d3e1559d5710366101b50540b705c8..82740baff79048f3a18bfd3cbb78ddd2b52c0802 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -33,6 +33,7 @@ from ipalib import api, errors, util
 from ipapython.ipa_log_manager import *
 from ipapython.dn import DN
 from ipapython.config import IPAOptionParser
+from ipaclient import ipadiscovery
 
 CACERT = /etc/ipa/ca.crt
 
@@ -709,7 +710,21 @@ def add_link(realm, replica1, replica2, dirman_passwd, options):
 repl2.conn.getEntry(master2_dn, ldap.SCOPE_BASE)
 
 except errors.NotFound:
-sys.exit(You cannot connect to a previously deleted master)
+standard_logging_setup(console_format='%(message)s')
+
+ds = ipadiscovery.IPADiscovery()
+ret = ds.search(server=replica2)
+
+if ret == ipadiscovery.NOT_IPA_SERVER:
+sys.exit(Connection unsuccessful: %s is not an IPA Server. %
+replica2)
+elif ret == 0:  # success
+sys.exit(Connection unsuccessful: %s is an IPA Server, 
+but it might be unknown, foreign or previously deleted 
+one. % replica2)
+else:
+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)
 
-- 
1.7.11.7

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

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

2012-10-19 Thread Petr Viktorin

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.


I'd also replace the test for -4 with the constant
ipadiscovery.NOT_IPA_SERVER

rob



--
PetrĀ³

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


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

2012-10-18 Thread Rob Crittenden

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.


I'd also replace the test for -4 with the constant 
ipadiscovery.NOT_IPA_SERVER


rob

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


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

2012-10-05 Thread Tomas Babej

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


From 8bcd599a4fb2bb33f0592170df4a0f6b76d8aa2f Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 2 Oct 2012 09:15:33 -0400
Subject: [PATCH] IPA Server check in ipa-replica-manage

When executing ipa-replica-manage connect to an master that raises
NotFound error we now check if the master is at least IPA server.
If so, we inform the user that it is probably foreign or previously
deleted master. If not, we inform the user that the master is not
an IPA server at all.

https://fedorahosted.org/freeipa/ticket/3105
---
 install/tools/ipa-replica-manage | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 897d117681d3e1559d5710366101b50540b705c8..19c9fea3092be0da4c9fed2640183a1939141a59 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -33,6 +33,7 @@ from ipalib import api, errors, util
 from ipapython.ipa_log_manager import *
 from ipapython.dn import DN
 from ipapython.config import IPAOptionParser
+from ipaclient import ipadiscovery
 
 CACERT = /etc/ipa/ca.crt
 
@@ -709,7 +710,19 @@ def add_link(realm, replica1, replica2, dirman_passwd, options):
 repl2.conn.getEntry(master2_dn, ldap.SCOPE_BASE)
 
 except errors.NotFound:
-sys.exit(You cannot connect to a previously deleted master)
+ds = ipadiscovery.IPADiscovery()
+ret = ds.search(server=replica2)
+
+if ret == -4:  # not an IPA Server
+sys.exit(Connection unsuccessful: %s is not an IPA Server. %
+replica2)
+elif ret == 0:  # success
+sys.exit(Connection unsuccessful: %s is an IPA Server, 
+but it might be unknown, foreign or previously deleted 
+one. % replica2)
+else:
+sys.exit(Connection unsuccessful.)
+
 repl1.setup_gssapi_replication(replica2, DN(('cn', 'Directory Manager')), dirman_passwd)
 print Connected '%s' to '%s' % (replica1, replica2)
 
-- 
1.7.11.4

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

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

2012-10-02 Thread Rob Crittenden

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

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