Andrew Bogott has submitted this change and it was merged.
Change subject: Make the nova_ldap notification handler thread-safe
......................................................................
Make the nova_ldap notification handler thread-safe
It turns out that this is getting called by eventlet,
and was re-entering all over the place.
Change-Id: I58ca5f93d0f6e481e0467038585d0f8947d787df
---
M modules/openstack/files/kilo/designate/nova_ldap/base.py
M modules/openstack/files/liberty/designate/nova_ldap/base.py
M modules/openstack/files/mitaka/designate/nova_ldap/base.py
3 files changed, 126 insertions(+), 111 deletions(-)
Approvals:
Andrew Bogott: Looks good to me, approved
Alex Monk: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/modules/openstack/files/kilo/designate/nova_ldap/base.py
b/modules/openstack/files/kilo/designate/nova_ldap/base.py
index 036fd5a..7ae9a46 100644
--- a/modules/openstack/files/kilo/designate/nova_ldap/base.py
+++ b/modules/openstack/files/kilo/designate/nova_ldap/base.py
@@ -33,7 +33,8 @@
class BaseAddressLdapHandler(BaseAddressHandler):
- def _get_ip_data(self, addr_dict):
+ @staticmethod
+ def _get_ip_data(addr_dict):
ip = addr_dict['address']
version = addr_dict['version']
@@ -49,7 +50,8 @@
data["octet%s" % i] = ip_data[i]
return data
- def _getLdapInfo(self, attr, conffile="/etc/ldap.conf"):
+ @staticmethod
+ def _getLdapInfo(attr, conffile="/etc/ldap.conf"):
try:
f = open(conffile)
except IOError:
@@ -64,39 +66,35 @@
return line.split(None, 1)[1].strip()
break
- def _initLdap(self):
- self.base = self._getLdapInfo("base")
- self.ldapHost = self._getLdapInfo("uri")
- self.sslType = self._getLdapInfo("ssl")
-
- self.binddn = cfg.CONF[self.name].get('ldapusername')
- self.bindpw = cfg.CONF[self.name].get('ldappassword')
-
def _openLdap(self):
- self.ds = ldap.initialize(self.ldapHost)
- self.ds.protocol_version = ldap.VERSION3
- if self.sslType == "start_tls":
- self.ds.start_tls_s()
+ ldapHost = self._getLdapInfo("uri")
+ sslType = self._getLdapInfo("ssl")
+
+ binddn = cfg.CONF[self.name].get('ldapusername')
+ bindpw = cfg.CONF[self.name].get('ldappassword')
+ ds = ldap.initialize(ldapHost)
+ ds.protocol_version = ldap.VERSION3
+ if sslType == "start_tls":
+ ds.start_tls_s()
try:
- self.ds.simple_bind_s(self.binddn, self.bindpw)
- return self.ds
+ ds.simple_bind_s(binddn, bindpw)
+ return ds
except ldap.CONSTRAINT_VIOLATION:
LOG.debug("LDAP bind failure: Too many failed attempts.\n")
except ldap.INVALID_DN_SYNTAX:
LOG.debug("LDAP bind failure: The bind DN is incorrect... \n")
except ldap.NO_SUCH_OBJECT:
- LOG.debug("LDAP bind failure: Unable to locate the bind DN
account.\n")
+ LOG.debug("LDAP bind failure: "
+ "Unable to locate the bind DN account.\n")
except ldap.UNWILLING_TO_PERFORM as msg:
- LOG.debug("LDAP bind failure: The LDAP server was unwilling to
perform the action"
+ LOG.debug("LDAP bind failure: "
+ "The LDAP server was unwilling to perform the action"
" requested.\nError was: %s\n" % msg[0]["info"])
except ldap.INVALID_CREDENTIALS:
LOG.debug("LDAP bind failure: Password incorrect.\n")
return None
-
- def _closeLdap(self):
- self.ds.unbind()
def _create(self, addresses, extra, managed=True,
resource_type=None, resource_id=None):
@@ -110,8 +108,8 @@
:param resource_type: The managed resource type
:param resource_id: The managed resource ID
"""
- self._initLdap()
- if not self._openLdap():
+ ds = self._openLdap()
+ if not ds:
return
LOG.debug('Using DomainID: %s' % cfg.CONF[self.name].domain_id)
@@ -153,23 +151,26 @@
hostEntry['puppetVar'].append(var)
hostEntry['associatedDomain'] = []
hostEntry['puppetVar'].append('instanceproject=%s' %
-
event_data['project_name'].encode('utf8'))
+ event_data['project_name'].encode(
+ 'utf8'))
hostEntry['puppetVar'].append('instancename=%s' %
- event_data['hostname'].encode('utf8'))
+ event_data['hostname'].encode(
+ 'utf8'))
for fmt in cfg.CONF[self.name].get('format'):
- hostEntry['associatedDomain'].append((fmt %
event_data).rstrip('.').encode('utf8'))
+ hostEntry['associatedDomain'].append(
+ (fmt % event_data).rstrip('.').encode('utf8'))
if managed:
LOG.debug('Creating ldap record')
modlist = ldap.modlist.addModlist(hostEntry)
try:
- self.ds.add_s(dn, modlist)
+ ds.add_s(dn, modlist)
except ldap.LDAPError as e:
LOG.debug('Ldap exception %s' % e)
- self._closeLdap()
+ ds.unbind()
def _delete(self, extra, managed=True, resource_id=None,
resource_type='instance', criterion={}):
@@ -178,9 +179,8 @@
:param criterion: Criterion to search and destroy records
"""
- LOG.debug('Initializing ldap')
- self._initLdap()
- if not self._openLdap():
+ ds = self._openLdap()
+ if not ds:
return
LOG.debug('Delete using DomainID: %s' % cfg.CONF[self.name].domain_id)
@@ -202,11 +202,11 @@
LOG.debug('Deleting ldap record: %s' % dn)
try:
- self.ds.delete_s(dn)
+ ds.delete_s(dn)
except ldap.NO_SUCH_OBJECT:
LOG.debug('Warning: %s not found in ldap. Not deleted.' % dn)
- self._closeLdap()
+ ds.unbind()
# WMF-specific add-on: Clean salt and puppet keys for deleted
# instance
@@ -217,7 +217,8 @@
LOG.debug('Cleaning puppet key %s' % puppetkey)
self._run_remote_command(cfg.CONF[self.name].puppet_master_host,
cfg.CONF[self.name].certmanager_user,
- 'sudo puppet cert clean %s' %
pipes.quote(puppetkey))
+ 'sudo puppet cert clean %s' %
+ pipes.quote(puppetkey))
if (cfg.CONF[self.name].salt_key_format and
cfg.CONF[self.name].salt_master_host):
@@ -226,12 +227,16 @@
LOG.debug('Cleaning salt key %s' % saltkey)
self._run_remote_command(cfg.CONF[self.name].salt_master_host,
cfg.CONF[self.name].certmanager_user,
- 'sudo salt-key -y -d %s' %
pipes.quote(saltkey))
+ 'sudo salt-key -y -d %s' %
+ pipes.quote(saltkey))
- def _run_remote_command(self, server, username, command):
+ @staticmethod
+ def _run_remote_command(server, username, command):
ssh_command = ['/usr/bin/ssh', '-l%s' % username, server, command]
- p = subprocess.Popen(ssh_command, stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
+ p = subprocess.Popen(ssh_command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
(out, error) = p.communicate()
rcode = p.wait()
return out, error, rcode
diff --git a/modules/openstack/files/liberty/designate/nova_ldap/base.py
b/modules/openstack/files/liberty/designate/nova_ldap/base.py
index 036fd5a..7ae9a46 100644
--- a/modules/openstack/files/liberty/designate/nova_ldap/base.py
+++ b/modules/openstack/files/liberty/designate/nova_ldap/base.py
@@ -33,7 +33,8 @@
class BaseAddressLdapHandler(BaseAddressHandler):
- def _get_ip_data(self, addr_dict):
+ @staticmethod
+ def _get_ip_data(addr_dict):
ip = addr_dict['address']
version = addr_dict['version']
@@ -49,7 +50,8 @@
data["octet%s" % i] = ip_data[i]
return data
- def _getLdapInfo(self, attr, conffile="/etc/ldap.conf"):
+ @staticmethod
+ def _getLdapInfo(attr, conffile="/etc/ldap.conf"):
try:
f = open(conffile)
except IOError:
@@ -64,39 +66,35 @@
return line.split(None, 1)[1].strip()
break
- def _initLdap(self):
- self.base = self._getLdapInfo("base")
- self.ldapHost = self._getLdapInfo("uri")
- self.sslType = self._getLdapInfo("ssl")
-
- self.binddn = cfg.CONF[self.name].get('ldapusername')
- self.bindpw = cfg.CONF[self.name].get('ldappassword')
-
def _openLdap(self):
- self.ds = ldap.initialize(self.ldapHost)
- self.ds.protocol_version = ldap.VERSION3
- if self.sslType == "start_tls":
- self.ds.start_tls_s()
+ ldapHost = self._getLdapInfo("uri")
+ sslType = self._getLdapInfo("ssl")
+
+ binddn = cfg.CONF[self.name].get('ldapusername')
+ bindpw = cfg.CONF[self.name].get('ldappassword')
+ ds = ldap.initialize(ldapHost)
+ ds.protocol_version = ldap.VERSION3
+ if sslType == "start_tls":
+ ds.start_tls_s()
try:
- self.ds.simple_bind_s(self.binddn, self.bindpw)
- return self.ds
+ ds.simple_bind_s(binddn, bindpw)
+ return ds
except ldap.CONSTRAINT_VIOLATION:
LOG.debug("LDAP bind failure: Too many failed attempts.\n")
except ldap.INVALID_DN_SYNTAX:
LOG.debug("LDAP bind failure: The bind DN is incorrect... \n")
except ldap.NO_SUCH_OBJECT:
- LOG.debug("LDAP bind failure: Unable to locate the bind DN
account.\n")
+ LOG.debug("LDAP bind failure: "
+ "Unable to locate the bind DN account.\n")
except ldap.UNWILLING_TO_PERFORM as msg:
- LOG.debug("LDAP bind failure: The LDAP server was unwilling to
perform the action"
+ LOG.debug("LDAP bind failure: "
+ "The LDAP server was unwilling to perform the action"
" requested.\nError was: %s\n" % msg[0]["info"])
except ldap.INVALID_CREDENTIALS:
LOG.debug("LDAP bind failure: Password incorrect.\n")
return None
-
- def _closeLdap(self):
- self.ds.unbind()
def _create(self, addresses, extra, managed=True,
resource_type=None, resource_id=None):
@@ -110,8 +108,8 @@
:param resource_type: The managed resource type
:param resource_id: The managed resource ID
"""
- self._initLdap()
- if not self._openLdap():
+ ds = self._openLdap()
+ if not ds:
return
LOG.debug('Using DomainID: %s' % cfg.CONF[self.name].domain_id)
@@ -153,23 +151,26 @@
hostEntry['puppetVar'].append(var)
hostEntry['associatedDomain'] = []
hostEntry['puppetVar'].append('instanceproject=%s' %
-
event_data['project_name'].encode('utf8'))
+ event_data['project_name'].encode(
+ 'utf8'))
hostEntry['puppetVar'].append('instancename=%s' %
- event_data['hostname'].encode('utf8'))
+ event_data['hostname'].encode(
+ 'utf8'))
for fmt in cfg.CONF[self.name].get('format'):
- hostEntry['associatedDomain'].append((fmt %
event_data).rstrip('.').encode('utf8'))
+ hostEntry['associatedDomain'].append(
+ (fmt % event_data).rstrip('.').encode('utf8'))
if managed:
LOG.debug('Creating ldap record')
modlist = ldap.modlist.addModlist(hostEntry)
try:
- self.ds.add_s(dn, modlist)
+ ds.add_s(dn, modlist)
except ldap.LDAPError as e:
LOG.debug('Ldap exception %s' % e)
- self._closeLdap()
+ ds.unbind()
def _delete(self, extra, managed=True, resource_id=None,
resource_type='instance', criterion={}):
@@ -178,9 +179,8 @@
:param criterion: Criterion to search and destroy records
"""
- LOG.debug('Initializing ldap')
- self._initLdap()
- if not self._openLdap():
+ ds = self._openLdap()
+ if not ds:
return
LOG.debug('Delete using DomainID: %s' % cfg.CONF[self.name].domain_id)
@@ -202,11 +202,11 @@
LOG.debug('Deleting ldap record: %s' % dn)
try:
- self.ds.delete_s(dn)
+ ds.delete_s(dn)
except ldap.NO_SUCH_OBJECT:
LOG.debug('Warning: %s not found in ldap. Not deleted.' % dn)
- self._closeLdap()
+ ds.unbind()
# WMF-specific add-on: Clean salt and puppet keys for deleted
# instance
@@ -217,7 +217,8 @@
LOG.debug('Cleaning puppet key %s' % puppetkey)
self._run_remote_command(cfg.CONF[self.name].puppet_master_host,
cfg.CONF[self.name].certmanager_user,
- 'sudo puppet cert clean %s' %
pipes.quote(puppetkey))
+ 'sudo puppet cert clean %s' %
+ pipes.quote(puppetkey))
if (cfg.CONF[self.name].salt_key_format and
cfg.CONF[self.name].salt_master_host):
@@ -226,12 +227,16 @@
LOG.debug('Cleaning salt key %s' % saltkey)
self._run_remote_command(cfg.CONF[self.name].salt_master_host,
cfg.CONF[self.name].certmanager_user,
- 'sudo salt-key -y -d %s' %
pipes.quote(saltkey))
+ 'sudo salt-key -y -d %s' %
+ pipes.quote(saltkey))
- def _run_remote_command(self, server, username, command):
+ @staticmethod
+ def _run_remote_command(server, username, command):
ssh_command = ['/usr/bin/ssh', '-l%s' % username, server, command]
- p = subprocess.Popen(ssh_command, stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
+ p = subprocess.Popen(ssh_command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
(out, error) = p.communicate()
rcode = p.wait()
return out, error, rcode
diff --git a/modules/openstack/files/mitaka/designate/nova_ldap/base.py
b/modules/openstack/files/mitaka/designate/nova_ldap/base.py
index 036fd5a..7ae9a46 100644
--- a/modules/openstack/files/mitaka/designate/nova_ldap/base.py
+++ b/modules/openstack/files/mitaka/designate/nova_ldap/base.py
@@ -33,7 +33,8 @@
class BaseAddressLdapHandler(BaseAddressHandler):
- def _get_ip_data(self, addr_dict):
+ @staticmethod
+ def _get_ip_data(addr_dict):
ip = addr_dict['address']
version = addr_dict['version']
@@ -49,7 +50,8 @@
data["octet%s" % i] = ip_data[i]
return data
- def _getLdapInfo(self, attr, conffile="/etc/ldap.conf"):
+ @staticmethod
+ def _getLdapInfo(attr, conffile="/etc/ldap.conf"):
try:
f = open(conffile)
except IOError:
@@ -64,39 +66,35 @@
return line.split(None, 1)[1].strip()
break
- def _initLdap(self):
- self.base = self._getLdapInfo("base")
- self.ldapHost = self._getLdapInfo("uri")
- self.sslType = self._getLdapInfo("ssl")
-
- self.binddn = cfg.CONF[self.name].get('ldapusername')
- self.bindpw = cfg.CONF[self.name].get('ldappassword')
-
def _openLdap(self):
- self.ds = ldap.initialize(self.ldapHost)
- self.ds.protocol_version = ldap.VERSION3
- if self.sslType == "start_tls":
- self.ds.start_tls_s()
+ ldapHost = self._getLdapInfo("uri")
+ sslType = self._getLdapInfo("ssl")
+
+ binddn = cfg.CONF[self.name].get('ldapusername')
+ bindpw = cfg.CONF[self.name].get('ldappassword')
+ ds = ldap.initialize(ldapHost)
+ ds.protocol_version = ldap.VERSION3
+ if sslType == "start_tls":
+ ds.start_tls_s()
try:
- self.ds.simple_bind_s(self.binddn, self.bindpw)
- return self.ds
+ ds.simple_bind_s(binddn, bindpw)
+ return ds
except ldap.CONSTRAINT_VIOLATION:
LOG.debug("LDAP bind failure: Too many failed attempts.\n")
except ldap.INVALID_DN_SYNTAX:
LOG.debug("LDAP bind failure: The bind DN is incorrect... \n")
except ldap.NO_SUCH_OBJECT:
- LOG.debug("LDAP bind failure: Unable to locate the bind DN
account.\n")
+ LOG.debug("LDAP bind failure: "
+ "Unable to locate the bind DN account.\n")
except ldap.UNWILLING_TO_PERFORM as msg:
- LOG.debug("LDAP bind failure: The LDAP server was unwilling to
perform the action"
+ LOG.debug("LDAP bind failure: "
+ "The LDAP server was unwilling to perform the action"
" requested.\nError was: %s\n" % msg[0]["info"])
except ldap.INVALID_CREDENTIALS:
LOG.debug("LDAP bind failure: Password incorrect.\n")
return None
-
- def _closeLdap(self):
- self.ds.unbind()
def _create(self, addresses, extra, managed=True,
resource_type=None, resource_id=None):
@@ -110,8 +108,8 @@
:param resource_type: The managed resource type
:param resource_id: The managed resource ID
"""
- self._initLdap()
- if not self._openLdap():
+ ds = self._openLdap()
+ if not ds:
return
LOG.debug('Using DomainID: %s' % cfg.CONF[self.name].domain_id)
@@ -153,23 +151,26 @@
hostEntry['puppetVar'].append(var)
hostEntry['associatedDomain'] = []
hostEntry['puppetVar'].append('instanceproject=%s' %
-
event_data['project_name'].encode('utf8'))
+ event_data['project_name'].encode(
+ 'utf8'))
hostEntry['puppetVar'].append('instancename=%s' %
- event_data['hostname'].encode('utf8'))
+ event_data['hostname'].encode(
+ 'utf8'))
for fmt in cfg.CONF[self.name].get('format'):
- hostEntry['associatedDomain'].append((fmt %
event_data).rstrip('.').encode('utf8'))
+ hostEntry['associatedDomain'].append(
+ (fmt % event_data).rstrip('.').encode('utf8'))
if managed:
LOG.debug('Creating ldap record')
modlist = ldap.modlist.addModlist(hostEntry)
try:
- self.ds.add_s(dn, modlist)
+ ds.add_s(dn, modlist)
except ldap.LDAPError as e:
LOG.debug('Ldap exception %s' % e)
- self._closeLdap()
+ ds.unbind()
def _delete(self, extra, managed=True, resource_id=None,
resource_type='instance', criterion={}):
@@ -178,9 +179,8 @@
:param criterion: Criterion to search and destroy records
"""
- LOG.debug('Initializing ldap')
- self._initLdap()
- if not self._openLdap():
+ ds = self._openLdap()
+ if not ds:
return
LOG.debug('Delete using DomainID: %s' % cfg.CONF[self.name].domain_id)
@@ -202,11 +202,11 @@
LOG.debug('Deleting ldap record: %s' % dn)
try:
- self.ds.delete_s(dn)
+ ds.delete_s(dn)
except ldap.NO_SUCH_OBJECT:
LOG.debug('Warning: %s not found in ldap. Not deleted.' % dn)
- self._closeLdap()
+ ds.unbind()
# WMF-specific add-on: Clean salt and puppet keys for deleted
# instance
@@ -217,7 +217,8 @@
LOG.debug('Cleaning puppet key %s' % puppetkey)
self._run_remote_command(cfg.CONF[self.name].puppet_master_host,
cfg.CONF[self.name].certmanager_user,
- 'sudo puppet cert clean %s' %
pipes.quote(puppetkey))
+ 'sudo puppet cert clean %s' %
+ pipes.quote(puppetkey))
if (cfg.CONF[self.name].salt_key_format and
cfg.CONF[self.name].salt_master_host):
@@ -226,12 +227,16 @@
LOG.debug('Cleaning salt key %s' % saltkey)
self._run_remote_command(cfg.CONF[self.name].salt_master_host,
cfg.CONF[self.name].certmanager_user,
- 'sudo salt-key -y -d %s' %
pipes.quote(saltkey))
+ 'sudo salt-key -y -d %s' %
+ pipes.quote(saltkey))
- def _run_remote_command(self, server, username, command):
+ @staticmethod
+ def _run_remote_command(server, username, command):
ssh_command = ['/usr/bin/ssh', '-l%s' % username, server, command]
- p = subprocess.Popen(ssh_command, stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
+ p = subprocess.Popen(ssh_command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
(out, error) = p.communicate()
rcode = p.wait()
return out, error, rcode
--
To view, visit https://gerrit.wikimedia.org/r/312127
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I58ca5f93d0f6e481e0467038585d0f8947d787df
Gerrit-PatchSet: 1
Gerrit-Project: operations/puppet
Gerrit-Branch: production
Gerrit-Owner: Andrew Bogott <abog...@wikimedia.org>
Gerrit-Reviewer: Alex Monk <a...@wikimedia.org>
Gerrit-Reviewer: Andrew Bogott <abog...@wikimedia.org>
Gerrit-Reviewer: Volans <rcocci...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
