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.


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
        if conn is not None and conn.isconnected():
        if tmpdir is not None:

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)


Jan Cholasta

Freeipa-devel mailing list

Reply via email to