Re: [Freeipa-devel] [PATCH] 1068 wait for LDAP when renewing the RA
Hi, On 24.10.2012 21:24, Rob Crittenden wrote: All the certs are pretty critical in certificate renewal but the agent cert has the distinction of having to be updated in multiple places. It needs to exist in both LDAP servers. It is possible that one or both of these servers may be down briefly during renewal so we need to be a bit more robust in our handling. This will wait up to 5 minutes per server to try to update things, and syslog when failures occur. It is now also safe to re-run this in case something catastrophic happens. One would just need to manually run this to load the required data into LDAP. rob I believe that there should be a break statement after the updated = True line: +updated = True +except errors.NetworkError: It would be nice if this message said 30 s instead of just 30: +syslog.syslog(syslog.LOG_ERR, 'Connection to %s failed, sleeping 30' % dogtag_uri) The continue statement is redundant: +attempts += 1 +continue except errors.EmptyModlist: The variables you access in both of the finally blocks (conn and tmpdir) may be unbound. This can be fixed like this: while attempts 10: conn = None tmpdir = None try: ... finally: if conn is not None and conn.isconnected(): conn.disconnect() if tmpdir is not None: shutil.rmtree(tmpdir) It would be nice if this message (both instances of it) included sys.argv[0], so that it is obvious to the user what script is this script: +syslog.syslog(syslog.LOG_ERR, 'Giving up. This script may be safely re-executed.') No spaces in kwarg assignment please: +tmpdir = tempfile.mkdtemp(prefix = tmp-) You might want to include sleeping 30 s in this message as well: +except Exception, e: +syslog.syslog(syslog.LOG_ERR, 'Updating renewal certificate failed: %s' % e) +time.sleep(30) Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1068 wait for LDAP when renewing the RA
Jan Cholasta wrote: Hi, On 24.10.2012 21:24, Rob Crittenden wrote: All the certs are pretty critical in certificate renewal but the agent cert has the distinction of having to be updated in multiple places. It needs to exist in both LDAP servers. It is possible that one or both of these servers may be down briefly during renewal so we need to be a bit more robust in our handling. This will wait up to 5 minutes per server to try to update things, and syslog when failures occur. It is now also safe to re-run this in case something catastrophic happens. One would just need to manually run this to load the required data into LDAP. rob I believe that there should be a break statement after the updated = True line: +updated = True +except errors.NetworkError: Sure is, added. It would be nice if this message said 30 s instead of just 30: +syslog.syslog(syslog.LOG_ERR, 'Connection to %s failed, sleeping 30' % dogtag_uri) Sure. The continue statement is redundant: +attempts += 1 +continue except errors.EmptyModlist: Yup. I used to have code executed after the try/except/finally. Removed. The variables you access in both of the finally blocks (conn and tmpdir) may be unbound. This can be fixed like this: while attempts 10: conn = None tmpdir = None try: ... finally: if conn is not None and conn.isconnected(): conn.disconnect() if tmpdir is not None: shutil.rmtree(tmpdir) Good catch, added. It would be nice if this message (both instances of it) included sys.argv[0], so that it is obvious to the user what script is this script: +syslog.syslog(syslog.LOG_ERR, 'Giving up. This script may be safely re-executed.') It is included by syslog: Nov 1 11:13:24 edsel renew_ra_cert: Connection to ldap://localhost:7389 failed, sleeping 30 No spaces in kwarg assignment please: +tmpdir = tempfile.mkdtemp(prefix = tmp-) OK. You might want to include sleeping 30 s in this message as well: +except Exception, e: +syslog.syslog(syslog.LOG_ERR, 'Updating renewal certificate failed: %s' % e) +time.sleep(30) Sure, added that. I also added a missing update. I added handling for ldap.SERVER_DOWN as a NetworkError instead of a DatabaseError. rob From 8e31742686f5b261e6f6a82f9d49882a0a2750a6 Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Tue, 23 Oct 2012 14:07:13 -0400 Subject: [PATCH] Wait for the directory server to come up when updating the agent certificate. It is possible that either or both of the LDAP instances are being restarted during the renewal process. Make the script retry if this is the case. It is also safe to re-run this script if it fails. It will take the current ipaCert certificate and attempt to update the agent information in LDAP. https://fedorahosted.org/freeipa/ticket/3179 --- install/restart_scripts/renew_ra_cert | 100 +++--- ipaserver/plugins/ldap2.py| 3 + 2 files changed, 72 insertions(+), 31 deletions(-) diff --git a/install/restart_scripts/renew_ra_cert b/install/restart_scripts/renew_ra_cert index 14cbc114ca9d38697cdb2a24b41ccf2ac2c66389..9d4a5e9c431c5445d4bc361630303015c83e53db 100644 --- a/install/restart_scripts/renew_ra_cert +++ b/install/restart_scripts/renew_ra_cert @@ -23,6 +23,7 @@ import sys import shutil import tempfile import syslog +import time from ipapython import services as ipaservices from ipapython.certmonger import get_pin from ipapython import ipautil @@ -33,6 +34,7 @@ from ipapython.dn import DN from ipalib import x509 from ipalib import errors from ipaserver.plugins.ldap2 import ldap2 +import ldap as _ldap api.bootstrap(context='restart') api.finalize() @@ -53,41 +55,77 @@ except IOError, e: syslog.syslog(syslog.LOG_ERR, 'Unable to determine PIN for CA instance: %s' % e) sys.exit(1) -try: -conn = ldap2(shared_instance=False, ldap_uri='ldap://localhost:%d' % DEFAULT_DSPORT) -conn.connect(bind_dn=DN(('cn', 'directory manager')), bind_pw=dm_password) -(entry_dn, entry_attrs) = conn.get_entry(dn, ['usercertificate'], normalize=False) -entry_attrs['usercertificate'].append(cert) -entry_attrs['description'] = '2;%d;%s;%s' % (serial_number, issuer, subject) -conn.update_entry(dn, entry_attrs, normalize=False) -conn.disconnect() -except Exception, e: -syslog.syslog(syslog.LOG_ERR, 'Updating agent entry failed: %s' % e) -sys.exit(1) +attempts = 0 +dogtag_uri='ldap://localhost:%d' % DEFAULT_DSPORT +updated = False -# Store it in the IPA LDAP server -tmpdir = tempfile.mkdtemp(prefix = tmp-) -try: -dn = DN(('cn','ipaCert'), ('cn', 'ca_renewal'), ('cn', 'ipa'), ('cn', 'etc'), api.env.basedn) -principal = str('host/%s@%s' % (api.env.host, api.env.realm)) -ccache = ipautil.kinit_hostprincipal('/etc/krb5.keytab', tmpdir, principal) -conn =
Re: [Freeipa-devel] [PATCH] 1068 wait for LDAP when renewing the RA
On 1.11.2012 16:32, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 24.10.2012 21:24, Rob Crittenden wrote: All the certs are pretty critical in certificate renewal but the agent cert has the distinction of having to be updated in multiple places. It needs to exist in both LDAP servers. It is possible that one or both of these servers may be down briefly during renewal so we need to be a bit more robust in our handling. This will wait up to 5 minutes per server to try to update things, and syslog when failures occur. It is now also safe to re-run this in case something catastrophic happens. One would just need to manually run this to load the required data into LDAP. rob I believe that there should be a break statement after the updated = True line: +updated = True +except errors.NetworkError: Sure is, added. It would be nice if this message said 30 s instead of just 30: +syslog.syslog(syslog.LOG_ERR, 'Connection to %s failed, sleeping 30' % dogtag_uri) Sure. The continue statement is redundant: +attempts += 1 +continue except errors.EmptyModlist: Yup. I used to have code executed after the try/except/finally. Removed. The variables you access in both of the finally blocks (conn and tmpdir) may be unbound. This can be fixed like this: while attempts 10: conn = None tmpdir = None try: ... finally: if conn is not None and conn.isconnected(): conn.disconnect() if tmpdir is not None: shutil.rmtree(tmpdir) Good catch, added. It would be nice if this message (both instances of it) included sys.argv[0], so that it is obvious to the user what script is this script: +syslog.syslog(syslog.LOG_ERR, 'Giving up. This script may be safely re-executed.') It is included by syslog: Nov 1 11:13:24 edsel renew_ra_cert: Connection to ldap://localhost:7389 failed, sleeping 30 Yep, but it might be nice to show the full path to the script, since it is not in $PATH. No spaces in kwarg assignment please: +tmpdir = tempfile.mkdtemp(prefix = tmp-) OK. You might want to include sleeping 30 s in this message as well: +except Exception, e: +syslog.syslog(syslog.LOG_ERR, 'Updating renewal certificate failed: %s' % e) +time.sleep(30) Sure, added that. I also added a missing update. I added handling for ldap.SERVER_DOWN as a NetworkError instead of a DatabaseError. rob Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1068 wait for LDAP when renewing the RA
Jan Cholasta wrote: On 1.11.2012 16:32, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 24.10.2012 21:24, Rob Crittenden wrote: All the certs are pretty critical in certificate renewal but the agent cert has the distinction of having to be updated in multiple places. It needs to exist in both LDAP servers. It is possible that one or both of these servers may be down briefly during renewal so we need to be a bit more robust in our handling. This will wait up to 5 minutes per server to try to update things, and syslog when failures occur. It is now also safe to re-run this in case something catastrophic happens. One would just need to manually run this to load the required data into LDAP. rob I believe that there should be a break statement after the updated = True line: +updated = True +except errors.NetworkError: Sure is, added. It would be nice if this message said 30 s instead of just 30: +syslog.syslog(syslog.LOG_ERR, 'Connection to %s failed, sleeping 30' % dogtag_uri) Sure. The continue statement is redundant: +attempts += 1 +continue except errors.EmptyModlist: Yup. I used to have code executed after the try/except/finally. Removed. The variables you access in both of the finally blocks (conn and tmpdir) may be unbound. This can be fixed like this: while attempts 10: conn = None tmpdir = None try: ... finally: if conn is not None and conn.isconnected(): conn.disconnect() if tmpdir is not None: shutil.rmtree(tmpdir) Good catch, added. It would be nice if this message (both instances of it) included sys.argv[0], so that it is obvious to the user what script is this script: +syslog.syslog(syslog.LOG_ERR, 'Giving up. This script may be safely re-executed.') It is included by syslog: Nov 1 11:13:24 edsel renew_ra_cert: Connection to ldap://localhost:7389 failed, sleeping 30 Yep, but it might be nice to show the full path to the script, since it is not in $PATH. Ok, added. rob From 9071e4b5c6ffb04929c1a68c6cd21359134660ad Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Tue, 23 Oct 2012 14:07:13 -0400 Subject: [PATCH] Wait for the directory server to come up when updating the agent certificate. It is possible that either or both of the LDAP instances are being restarted during the renewal process. Make the script retry if this is the case. It is also safe to re-run this script if it fails. It will take the current ipaCert certificate and attempt to update the agent information in LDAP. https://fedorahosted.org/freeipa/ticket/3179 --- install/restart_scripts/renew_ra_cert | 100 +++--- ipaserver/plugins/ldap2.py| 3 + 2 files changed, 72 insertions(+), 31 deletions(-) diff --git a/install/restart_scripts/renew_ra_cert b/install/restart_scripts/renew_ra_cert index 14cbc114ca9d38697cdb2a24b41ccf2ac2c66389..048a1ab18825c29d16d62e2a68e4b54dc79f032d 100644 --- a/install/restart_scripts/renew_ra_cert +++ b/install/restart_scripts/renew_ra_cert @@ -23,6 +23,7 @@ import sys import shutil import tempfile import syslog +import time from ipapython import services as ipaservices from ipapython.certmonger import get_pin from ipapython import ipautil @@ -33,6 +34,7 @@ from ipapython.dn import DN from ipalib import x509 from ipalib import errors from ipaserver.plugins.ldap2 import ldap2 +import ldap as _ldap api.bootstrap(context='restart') api.finalize() @@ -53,41 +55,77 @@ except IOError, e: syslog.syslog(syslog.LOG_ERR, 'Unable to determine PIN for CA instance: %s' % e) sys.exit(1) -try: -conn = ldap2(shared_instance=False, ldap_uri='ldap://localhost:%d' % DEFAULT_DSPORT) -conn.connect(bind_dn=DN(('cn', 'directory manager')), bind_pw=dm_password) -(entry_dn, entry_attrs) = conn.get_entry(dn, ['usercertificate'], normalize=False) -entry_attrs['usercertificate'].append(cert) -entry_attrs['description'] = '2;%d;%s;%s' % (serial_number, issuer, subject) -conn.update_entry(dn, entry_attrs, normalize=False) -conn.disconnect() -except Exception, e: -syslog.syslog(syslog.LOG_ERR, 'Updating agent entry failed: %s' % e) -sys.exit(1) +attempts = 0 +dogtag_uri='ldap://localhost:%d' % DEFAULT_DSPORT +updated = False -# Store it in the IPA LDAP server -tmpdir = tempfile.mkdtemp(prefix = tmp-) -try: -dn = DN(('cn','ipaCert'), ('cn', 'ca_renewal'), ('cn', 'ipa'), ('cn', 'etc'), api.env.basedn) -principal = str('host/%s@%s' % (api.env.host, api.env.realm)) -ccache = ipautil.kinit_hostprincipal('/etc/krb5.keytab', tmpdir, principal) -conn = ldap2(shared_instance=False, ldap_uri=api.env.ldap_uri) -conn.connect(ccache=ccache) +while attempts 10: try: -(entry_dn, entry_attrs) = conn.get_entry(dn, ['usercertificate']) -entry_attrs['usercertificate'] = cert +conn = ldap2(shared_instance=False,
Re: [Freeipa-devel] [PATCH] 1068 wait for LDAP when renewing the RA
Jan Cholasta wrote: On 1.11.2012 16:54, Rob Crittenden wrote: Jan Cholasta wrote: On 1.11.2012 16:32, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 24.10.2012 21:24, Rob Crittenden wrote: All the certs are pretty critical in certificate renewal but the agent cert has the distinction of having to be updated in multiple places. It needs to exist in both LDAP servers. It is possible that one or both of these servers may be down briefly during renewal so we need to be a bit more robust in our handling. This will wait up to 5 minutes per server to try to update things, and syslog when failures occur. It is now also safe to re-run this in case something catastrophic happens. One would just need to manually run this to load the required data into LDAP. rob I believe that there should be a break statement after the updated = True line: +updated = True +except errors.NetworkError: Sure is, added. It would be nice if this message said 30 s instead of just 30: +syslog.syslog(syslog.LOG_ERR, 'Connection to %s failed, sleeping 30' % dogtag_uri) Sure. The continue statement is redundant: +attempts += 1 +continue except errors.EmptyModlist: Yup. I used to have code executed after the try/except/finally. Removed. The variables you access in both of the finally blocks (conn and tmpdir) may be unbound. This can be fixed like this: while attempts 10: conn = None tmpdir = None try: ... finally: if conn is not None and conn.isconnected(): conn.disconnect() if tmpdir is not None: shutil.rmtree(tmpdir) Good catch, added. It would be nice if this message (both instances of it) included sys.argv[0], so that it is obvious to the user what script is this script: +syslog.syslog(syslog.LOG_ERR, 'Giving up. This script may be safely re-executed.') It is included by syslog: Nov 1 11:13:24 edsel renew_ra_cert: Connection to ldap://localhost:7389 failed, sleeping 30 Yep, but it might be nice to show the full path to the script, since it is not in $PATH. Ok, added. rob Thanks. Please also add the conn = None bit to the first loop: while attempts 10: conn = None try: ... finally: if conn is not None and conn.isconnected(): conn.disconnect() and it's ACK. Honza Done, pushed to master and ipa-3-0 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1068 wait for LDAP when renewing the RA
On 1.11.2012 16:54, Rob Crittenden wrote: Jan Cholasta wrote: On 1.11.2012 16:32, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 24.10.2012 21:24, Rob Crittenden wrote: All the certs are pretty critical in certificate renewal but the agent cert has the distinction of having to be updated in multiple places. It needs to exist in both LDAP servers. It is possible that one or both of these servers may be down briefly during renewal so we need to be a bit more robust in our handling. This will wait up to 5 minutes per server to try to update things, and syslog when failures occur. It is now also safe to re-run this in case something catastrophic happens. One would just need to manually run this to load the required data into LDAP. rob I believe that there should be a break statement after the updated = True line: +updated = True +except errors.NetworkError: Sure is, added. It would be nice if this message said 30 s instead of just 30: +syslog.syslog(syslog.LOG_ERR, 'Connection to %s failed, sleeping 30' % dogtag_uri) Sure. The continue statement is redundant: +attempts += 1 +continue except errors.EmptyModlist: Yup. I used to have code executed after the try/except/finally. Removed. The variables you access in both of the finally blocks (conn and tmpdir) may be unbound. This can be fixed like this: while attempts 10: conn = None tmpdir = None try: ... finally: if conn is not None and conn.isconnected(): conn.disconnect() if tmpdir is not None: shutil.rmtree(tmpdir) Good catch, added. It would be nice if this message (both instances of it) included sys.argv[0], so that it is obvious to the user what script is this script: +syslog.syslog(syslog.LOG_ERR, 'Giving up. This script may be safely re-executed.') It is included by syslog: Nov 1 11:13:24 edsel renew_ra_cert: Connection to ldap://localhost:7389 failed, sleeping 30 Yep, but it might be nice to show the full path to the script, since it is not in $PATH. Ok, added. rob Thanks. Please also add the conn = None bit to the first loop: while attempts 10: conn = None try: ... finally: if conn is not None and conn.isconnected(): conn.disconnect() and it's ACK. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel